From 852277b2ae1e69d969c03460a9cd0f214beb10e7 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Sat, 11 Apr 2020 12:12:45 -0300 Subject: [PATCH] Refactor notification manager and triggerUi to use extension platform (#8317) The notification manager has been refactored to use the extension platform module instead of using `extensionizer` directly. The extension platform API presents a more ergonomic API, and it correctly handles errors (which the old notification manager did not). Methods that the extension platform lacked have been added. It has been updated to use `async/await` instead of callbacks as well, for readability. The `triggerUI` function has also been updated to use the extension platform instead of `extensionizer`. --- app/scripts/background.js | 19 +++-- app/scripts/lib/notification-manager.js | 100 +++++++++--------------- app/scripts/platforms/extension.js | 60 +++++++++++++- 3 files changed, 102 insertions(+), 77 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 5da6d0774..abf2c8ce9 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -455,22 +455,21 @@ function setupController (initState, initLangCode) { /** * Opens the browser popup for user confirmation */ -function triggerUi () { - extension.tabs.query({ active: true }, (tabs) => { - const currentlyActiveMetamaskTab = Boolean(tabs.find((tab) => openMetamaskTabsIDs[tab.id])) - if (!popupIsOpen && !currentlyActiveMetamaskTab && !notificationIsOpen) { - notificationManager.showPopup() - } - }) +async function triggerUi () { + const tabs = await platform.getActiveTabs() + const currentlyActiveMetamaskTab = Boolean(tabs.find((tab) => openMetamaskTabsIDs[tab.id])) + if (!popupIsOpen && !currentlyActiveMetamaskTab && !notificationIsOpen) { + await notificationManager.showPopup() + } } /** * Opens the browser popup for user confirmation of watchAsset * then it waits until user interact with the UI */ -function openPopup () { - triggerUi() - return new Promise( +async function openPopup () { + await triggerUi() + await new Promise( (resolve) => { const interval = setInterval(() => { if (!notificationIsOpen) { diff --git a/app/scripts/lib/notification-manager.js b/app/scripts/lib/notification-manager.js index 12a0d3289..666c1f8c8 100644 --- a/app/scripts/lib/notification-manager.js +++ b/app/scripts/lib/notification-manager.js @@ -1,4 +1,4 @@ -import extension from 'extensionizer' +import ExtensionPlatform from '../platforms/extension' const NOTIFICATION_HEIGHT = 620 const NOTIFICATION_WIDTH = 360 @@ -12,57 +12,49 @@ class NotificationManager { * */ + constructor () { + this.platform = new ExtensionPlatform() + } + /** * Either brings an existing MetaMask notification window into focus, or creates a new notification window. New * notification windows are given a 'popup' type. * */ - showPopup () { - this._getPopup((err, popup) => { - if (err) { - throw err - } + async showPopup () { + const popup = await this._getPopup() - // Bring focus to chrome popup - if (popup) { - // bring focus to existing chrome popup - extension.windows.update(popup.id, { focused: true }) - } else { - const { screenX, screenY, outerWidth, outerHeight } = window - const notificationTop = Math.round(screenY + (outerHeight / 2) - (NOTIFICATION_HEIGHT / 2)) - const notificationLeft = Math.round(screenX + (outerWidth / 2) - (NOTIFICATION_WIDTH / 2)) - const cb = (currentPopup) => { - this._popupId = currentPopup.id - } - // create new notification popup - const creation = extension.windows.create({ - url: 'notification.html', - type: 'popup', - width: NOTIFICATION_WIDTH, - height: NOTIFICATION_HEIGHT, - top: Math.max(notificationTop, 0), - left: Math.max(notificationLeft, 0), - }, cb) - creation && creation.then && creation.then(cb) - } - }) + // Bring focus to chrome popup + if (popup) { + // bring focus to existing chrome popup + await this.platform.focusWindow(popup.id) + } else { + const { screenX, screenY, outerWidth, outerHeight } = window + const notificationTop = Math.round(screenY + (outerHeight / 2) - (NOTIFICATION_HEIGHT / 2)) + const notificationLeft = Math.round(screenX + (outerWidth / 2) - (NOTIFICATION_WIDTH / 2)) + // create new notification popup + const popupWindow = await this.platform.openWindow({ + url: 'notification.html', + type: 'popup', + width: NOTIFICATION_WIDTH, + height: NOTIFICATION_HEIGHT, + top: Math.max(notificationTop, 0), + left: Math.max(notificationLeft, 0), + }) + this._popupId = popupWindow.id + } } /** * Closes a MetaMask notification if it window exists. * */ - closePopup () { - // closes notification popup - this._getPopup((err, popup) => { - if (err) { - throw err - } - if (!popup) { - return - } - extension.windows.remove(popup.id, console.error) - }) + async closePopup () { + const popup = this._getPopup() + if (!popup) { + return + } + await this.platform.removeWindow(popup.id) } /** @@ -73,31 +65,9 @@ class NotificationManager { * @param {Function} cb - A node style callback that to whcih the found notification window will be passed. * */ - _getPopup (cb) { - this._getWindows((err, windows) => { - if (err) { - throw err - } - cb(null, this._getPopupIn(windows)) - }) - } - - /** - * Returns all open MetaMask windows. - * - * @private - * @param {Function} cb - A node style callback that to which the windows will be passed. - * - */ - _getWindows (cb) { - // Ignore in test environment - if (!extension.windows) { - return cb() - } - - extension.windows.getAll({}, (windows) => { - cb(null, windows) - }) + async _getPopup () { + const windows = await this.platform.getAllWindows() + return this._getPopupIn(windows) } /** diff --git a/app/scripts/platforms/extension.js b/app/scripts/platforms/extension.js index ddb0b551c..77ed4e451 100644 --- a/app/scripts/platforms/extension.js +++ b/app/scripts/platforms/extension.js @@ -12,8 +12,40 @@ class ExtensionPlatform { extension.runtime.reload() } - openWindow ({ url }) { - extension.tabs.create({ url }) + openWindow (options) { + return new Promise((resolve, reject) => { + extension.windows.create(options, (newWindow) => { + const error = checkForError() + if (error) { + return reject(error) + } + return resolve(newWindow) + }) + }) + } + + closeWindow (windowId) { + return new Promise((resolve, reject) => { + extension.windows.remove(windowId, () => { + const error = checkForError() + if (error) { + return reject(error) + } + return resolve() + }) + }) + } + + focusWindow (windowId) { + return new Promise((resolve, reject) => { + extension.windows.update(windowId, { focused: true }, () => { + const error = checkForError() + if (error) { + return reject(error) + } + return resolve() + }) + }) } closeCurrentWindow () { @@ -65,6 +97,30 @@ class ExtensionPlatform { } } + getAllWindows () { + return new Promise((resolve, reject) => { + extension.windows.getAll((windows) => { + const error = checkForError() + if (error) { + return reject(error) + } + return resolve(windows) + }) + }) + } + + getActiveTabs () { + return new Promise((resolve, reject) => { + extension.tabs.query({ active: true }, (tabs) => { + const error = checkForError() + if (error) { + return reject(error) + } + return resolve(tabs) + }) + }) + } + currentTab () { return new Promise((resolve, reject) => { extension.tabs.getCurrent((tab) => {