diff --git a/app/scripts/background.js b/app/scripts/background.js index 65ed6e147..816bf4f55 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -52,6 +52,7 @@ import getFirstPreferredLangCode from './lib/get-first-preferred-lang-code'; import getObjStructure from './lib/getObjStructure'; import setupEnsIpfsResolver from './lib/ens-ipfs/setup'; import { deferredPromise, getPlatform } from './lib/util'; + /* eslint-enable import/first */ /* eslint-disable import/order */ @@ -78,7 +79,6 @@ const metamaskBlockedPorts = ['trezor-connect']; log.setDefaultLevel(process.env.METAMASK_DEBUG ? 'debug' : 'info'); const platform = new ExtensionPlatform(); - const notificationManager = new NotificationManager(); global.METAMASK_NOTIFIER = notificationManager; @@ -848,7 +848,12 @@ async function triggerUi() { ) { uiIsTriggering = true; try { - await notificationManager.showPopup(); + const currentPopupId = controller.appStateController.getCurrentPopupId(); + await notificationManager.showPopup( + (newPopupId) => + controller.appStateController.setCurrentPopupId(newPopupId), + currentPopupId, + ); } finally { uiIsTriggering = false; } diff --git a/app/scripts/controllers/app-state.js b/app/scripts/controllers/app-state.js index ff12b85f3..7d1c83a4e 100644 --- a/app/scripts/controllers/app-state.js +++ b/app/scripts/controllers/app-state.js @@ -42,6 +42,7 @@ export default class AppStateController extends EventEmitter { showTestnetMessageInDropdown: true, showBetaHeader: isBeta(), trezorModel: null, + currentPopupId: undefined, ...initState, qrHardware: {}, nftsDropdownState: {}, @@ -343,4 +344,22 @@ export default class AppStateController extends EventEmitter { this.store.updateState({ usedNetworks }); } + + /** + * A setter for the currentPopupId which indicates the id of popup window that's currently active + * + * @param currentPopupId + */ + setCurrentPopupId(currentPopupId) { + this.store.updateState({ + currentPopupId, + }); + } + + /** + * A getter to retrieve currentPopupId saved in the appState + */ + getCurrentPopupId() { + return this.store.getState().currentPopupId; + } } diff --git a/app/scripts/lib/notification-manager.js b/app/scripts/lib/notification-manager.js index a03090ee9..6ee4e3c12 100644 --- a/app/scripts/lib/notification-manager.js +++ b/app/scripts/lib/notification-manager.js @@ -32,15 +32,19 @@ export default class NotificationManager extends EventEmitter { * Either brings an existing MetaMask notification window into focus, or creates a new notification window. New * notification windows are given a 'popup' type. * + * @param {Function} setCurrentPopupId - setter of current popup id from appStateController + * @param {number} currentPopupId - id of current opened metamask popup window */ - async showPopup() { - const popup = await this._getPopup(); - + async showPopup(setCurrentPopupId, currentPopupId) { + this._popupId = currentPopupId; + this._setCurrentPopupId = setCurrentPopupId; + const popup = await this._getPopup(currentPopupId); // Bring focus to chrome popup if (popup) { // bring focus to existing chrome popup await this.platform.focusWindow(popup.id); } else { + // create new notification popup let left = 0; let top = 0; try { @@ -57,7 +61,6 @@ export default class NotificationManager extends EventEmitter { left = Math.max(screenX + (outerWidth - NOTIFICATION_WIDTH), 0); } - // create new notification popup const popupWindow = await this.platform.openWindow({ url: 'notification.html', type: 'popup', @@ -71,12 +74,16 @@ export default class NotificationManager extends EventEmitter { if (popupWindow.left !== left && popupWindow.state !== 'fullscreen') { await this.platform.updateWindowPosition(popupWindow.id, left, top); } + // pass new created popup window id to appController setter + // and store the id to private variable this._popupId for future access + this._setCurrentPopupId(popupWindow.id); this._popupId = popupWindow.id; } } _onWindowClosed(windowId) { if (windowId === this._popupId) { + this._setCurrentPopupId(undefined); this._popupId = undefined; this.emit(NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED, { automaticallyClosed: this._popupAutomaticallyClosed, diff --git a/app/scripts/lib/notification-manager.test.ts b/app/scripts/lib/notification-manager.test.ts new file mode 100644 index 000000000..b3c44b249 --- /dev/null +++ b/app/scripts/lib/notification-manager.test.ts @@ -0,0 +1,71 @@ +import browser from 'webextension-polyfill'; +import NotificationManager from './notification-manager'; + +function generateMockWindow(overrides?: object) { + return { + alwaysOnTop: false, + focused: true, + height: 620, + width: 360, + id: 1312883868, + incognito: false, + left: 1326, + state: 'normal', + tabs: [ + { + active: true, + audible: false, + autoDiscardable: true, + discarded: false, + groupId: -1, + }, + ], + top: 25, + type: 'popup', + ...overrides, + }; +} + +jest.mock('webextension-polyfill', () => { + return { + windows: { + onRemoved: { + addListener: jest.fn(), + }, + getAll: jest.fn(), + create: jest.fn(), + update: jest.fn(), + }, + }; +}); + +describe('Notification Manager', () => { + let notificationManager: NotificationManager, + setCurrentPopupIdSpy: (a: number) => void, + focusWindowSpy: () => void, + currentPopupId: number; + + beforeEach(() => { + notificationManager = new NotificationManager(); + }); + + it('should not create a new popup window if there is one', async () => { + focusWindowSpy = jest.fn(); + browser.windows.getAll.mockReturnValue([generateMockWindow()]); + browser.windows.update.mockImplementation(focusWindowSpy); + currentPopupId = 1312883868; + await notificationManager.showPopup(setCurrentPopupIdSpy, currentPopupId); + expect(focusWindowSpy).toHaveBeenCalledTimes(1); + }); + + it('should create a new popup window if there is no existing one', async () => { + const newPopupWindow = generateMockWindow(); + setCurrentPopupIdSpy = jest.fn(); + browser.windows.getAll.mockReturnValue([]); + browser.windows.create.mockReturnValue(newPopupWindow); + currentPopupId = undefined; + await notificationManager.showPopup(setCurrentPopupIdSpy, currentPopupId); + expect(setCurrentPopupIdSpy).toHaveBeenCalledTimes(1); + expect(setCurrentPopupIdSpy).toHaveBeenCalledWith(newPopupWindow.id); + }); +}); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index edca1c3c2..ef949dc0a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1929,6 +1929,7 @@ export default class MetamaskController extends EventEmitter { appStateController.updateNftDropDownState.bind(appStateController), setFirstTimeUsedNetwork: appStateController.setFirstTimeUsedNetwork.bind(appStateController), + // EnsController tryReverseResolveAddress: ensController.reverseResolveAddress.bind(ensController), @@ -3455,7 +3456,7 @@ export default class MetamaskController extends EventEmitter { * @param {object} [req] - The original request, containing the origin. * @param version */ - newUnsignedTypedMessage(msgParams, req, version) { + async newUnsignedTypedMessage(msgParams, req, version) { const promise = this.typedMessageManager.addUnapprovedMessageAsync( msgParams, req, diff --git a/jest.config.js b/jest.config.js index b8cf9cae2..94daa4971 100644 --- a/jest.config.js +++ b/jest.config.js @@ -41,6 +41,7 @@ module.exports = { '/app/scripts/controllers/permissions/**/*.test.js', '/app/scripts/flask/**/*.test.js', '/app/scripts/lib/**/*.test.js', + '/app/scripts/lib/**/*.test.ts', '/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js', '/app/scripts/migrations/*.test.js', '/app/scripts/platforms/*.test.js',