fix(17855): persist popup when sw is restarted (#17855)

* fix(17463): persist popup when sw is restarted

* feat(17855): clear local variable when close window

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;
     }

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;
+  }
 }

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,

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);
+  });
+});

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,

jest.config.js

@@ -41,6 +41,7 @@ module.exports = {
     '<rootDir>/app/scripts/controllers/permissions/**/*.test.js',
     '<rootDir>/app/scripts/flask/**/*.test.js',
     '<rootDir>/app/scripts/lib/**/*.test.js',
+    '<rootDir>/app/scripts/lib/**/*.test.ts',
     '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js',
     '<rootDir>/app/scripts/migrations/*.test.js',
     '<rootDir>/app/scripts/platforms/*.test.js',