Reject popup confirmations on close (#12643)

* Background clears confirmations on popup close

* [WIP] Remove clearing confirmations through UI

* Confirmations are now rejected instead of cleared

* Erased commented out code

* Fix linter errors

* Changes after code review

* Moved metrics events from onWindowUnload to background

* PR review fixes

* Added abillity to add reason to rejection of messages

* Fix prettier

* Added type metrics event to signature cancel

* Fix test

* The uncofirmed transactions are now cleared even if Metamask is locked

app/scripts/background.js

@@ -17,13 +17,19 @@ import {
   ENVIRONMENT_TYPE_FULLSCREEN,
 } from '../../shared/constants/app';
 import { SECOND } from '../../shared/constants/time';
+import {
+  REJECT_NOTFICIATION_CLOSE,
+  REJECT_NOTFICIATION_CLOSE_SIG,
+} from '../../shared/constants/metametrics';
 import migrations from './migrations';
 import Migrator from './lib/migrator';
 import ExtensionPlatform from './platforms/extension';
 import LocalStore from './lib/local-store';
 import ReadOnlyNetworkStore from './lib/network-store';
 import createStreamSink from './lib/createStreamSink';
-import NotificationManager from './lib/notification-manager';
+import NotificationManager, {
+  NOTIFICATION_MANAGER_EVENTS,
+} from './lib/notification-manager';
 import MetamaskController, {
   METAMASK_CONTROLLER_EVENTS,
 } from './metamask-controller';
@@ -475,6 +481,69 @@ function setupController(initState, initLangCode) {
     extension.browserAction.setBadgeBackgroundColor({ color: '#037DD6' });
   }
 
+  notificationManager.on(
+    NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED,
+    rejectUnapprovedNotifications,
+  );
+
+  function rejectUnapprovedNotifications() {
+    Object.keys(
+      controller.txController.txStateManager.getUnapprovedTxList(),
+    ).forEach((txId) =>
+      controller.txController.txStateManager.setTxStatusRejected(txId),
+    );
+    controller.messageManager.messages
+      .filter((msg) => msg.status === 'unapproved')
+      .forEach((tx) =>
+        controller.messageManager.rejectMsg(
+          tx.id,
+          REJECT_NOTFICIATION_CLOSE_SIG,
+        ),
+      );
+    controller.personalMessageManager.messages
+      .filter((msg) => msg.status === 'unapproved')
+      .forEach((tx) =>
+        controller.personalMessageManager.rejectMsg(
+          tx.id,
+          REJECT_NOTFICIATION_CLOSE_SIG,
+        ),
+      );
+    controller.typedMessageManager.messages
+      .filter((msg) => msg.status === 'unapproved')
+      .forEach((tx) =>
+        controller.typedMessageManager.rejectMsg(
+          tx.id,
+          REJECT_NOTFICIATION_CLOSE_SIG,
+        ),
+      );
+    controller.decryptMessageManager.messages
+      .filter((msg) => msg.status === 'unapproved')
+      .forEach((tx) =>
+        controller.decryptMessageManager.rejectMsg(
+          tx.id,
+          REJECT_NOTFICIATION_CLOSE,
+        ),
+      );
+    controller.encryptionPublicKeyManager.messages
+      .filter((msg) => msg.status === 'unapproved')
+      .forEach((tx) =>
+        controller.encryptionPublicKeyManager.rejectMsg(
+          tx.id,
+          REJECT_NOTFICIATION_CLOSE,
+        ),
+      );
+
+    // We're specifcally avoid using approvalController directly for better
+    // Error support during rejection
+    Object.keys(
+      controller.permissionsController.approvals.state.pendingApprovals,
+    ).forEach((approvalId) =>
+      controller.permissionsController.rejectPermissionsRequest(approvalId),
+    );
+
+    updateBadge();
+  }
+
   return Promise.resolve();
 }
 

app/scripts/lib/decrypt-message-manager.js

@@ -38,13 +38,14 @@ export default class DecryptMessageManager extends EventEmitter {
    * @property {Array} messages Holds all messages that have been created by this DecryptMessageManager
    *
    */
-  constructor() {
+  constructor(opts) {
     super();
     this.memStore = new ObservableStore({
       unapprovedDecryptMsgs: {},
       unapprovedDecryptMsgCount: 0,
     });
     this.messages = [];
+    this.metricsEvent = opts.metricsEvent;
   }
 
   /**
@@ -237,7 +238,16 @@ export default class DecryptMessageManager extends EventEmitter {
    * @param {number} msgId The id of the DecryptMessage to reject.
    *
    */
-  rejectMsg(msgId) {
+  rejectMsg(msgId, reason = undefined) {
+    if (reason) {
+      this.metricsEvent({
+        event: reason,
+        category: 'Messages',
+        properties: {
+          action: 'Decrypt Message Request',
+        },
+      });
+    }
     this._setMsgStatus(msgId, 'rejected');
   }
 

app/scripts/lib/encryption-public-key-manager.js

@@ -34,13 +34,14 @@ export default class EncryptionPublicKeyManager extends EventEmitter {
    * @property {Array} messages Holds all messages that have been created by this EncryptionPublicKeyManager
    *
    */
-  constructor() {
+  constructor(opts) {
     super();
     this.memStore = new ObservableStore({
       unapprovedEncryptionPublicKeyMsgs: {},
       unapprovedEncryptionPublicKeyMsgCount: 0,
     });
     this.messages = [];
+    this.metricsEvent = opts.metricsEvent;
   }
 
   /**
@@ -226,7 +227,16 @@ export default class EncryptionPublicKeyManager extends EventEmitter {
    * @param {number} msgId The id of the EncryptionPublicKey to reject.
    *
    */
-  rejectMsg(msgId) {
+  rejectMsg(msgId, reason = undefined) {
+    if (reason) {
+      this.metricsEvent({
+        event: reason,
+        category: 'Messages',
+        properties: {
+          action: 'Encryption public key Request',
+        },
+      });
+    }
     this._setMsgStatus(msgId, 'rejected');
   }
 

app/scripts/lib/message-manager.js

@@ -35,13 +35,14 @@ export default class MessageManager extends EventEmitter {
    * @property {Array} messages Holds all messages that have been created by this MessageManager
    *
    */
-  constructor() {
+  constructor({ metricsEvent }) {
     super();
     this.memStore = new ObservableStore({
       unapprovedMsgs: {},
       unapprovedMsgCount: 0,
     });
     this.messages = [];
+    this.metricsEvent = metricsEvent;
   }
 
   /**
@@ -217,7 +218,18 @@ export default class MessageManager extends EventEmitter {
    * @param {number} msgId - The id of the Message to reject.
    *
    */
-  rejectMsg(msgId) {
+  rejectMsg(msgId, reason = undefined) {
+    if (reason) {
+      const msg = this.getMsg(msgId);
+      this.metricsEvent({
+        event: reason,
+        category: 'Transactions',
+        properties: {
+          action: 'Sign Request',
+          type: msg.type,
+        },
+      });
+    }
     this._setMsgStatus(msgId, 'rejected');
   }
 

app/scripts/lib/message-manager.test.js

@@ -1,12 +1,15 @@
 import { strict as assert } from 'assert';
+import sinon from 'sinon';
 import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
 import MessageManager from './message-manager';
 
 describe('Message Manager', function () {
   let messageManager;
 
   beforeEach(function () {
-    messageManager = new MessageManager();
+    messageManager = new MessageManager({
+      metricsEvent: sinon.fake(),
+    });
   });
 
   describe('#getMsgList', function () {

app/scripts/lib/notification-manager.js

@@ -1,9 +1,14 @@
+import EventEmitter from 'safe-event-emitter';
 import ExtensionPlatform from '../platforms/extension';
 
 const NOTIFICATION_HEIGHT = 620;
 const NOTIFICATION_WIDTH = 360;
 
-export default class NotificationManager {
+export const NOTIFICATION_MANAGER_EVENTS = {
+  POPUP_CLOSED: 'onPopupClosed',
+};
+
+export default class NotificationManager extends EventEmitter {
   /**
    * A collection of methods for controlling the showing and hiding of the notification popup.
    *
@@ -12,7 +17,9 @@ export default class NotificationManager {
    */
 
   constructor() {
+    super();
     this.platform = new ExtensionPlatform();
+    this.platform.addOnRemovedListener(this._onWindowClosed.bind(this));
   }
 
   /**
@@ -62,6 +69,13 @@ export default class NotificationManager {
     }
   }
 
+  _onWindowClosed(windowId) {
+    if (windowId === this._popupId) {
+      this._popupId = undefined;
+      this.emit(NOTIFICATION_MANAGER_EVENTS.POPUP_CLOSED);
+    }
+  }
+
   /**
    * Checks all open MetaMask windows, and returns the first one it finds that is a notification window (i.e. has the
    * type 'popup')

app/scripts/lib/personal-message-manager.js

@@ -40,13 +40,14 @@ export default class PersonalMessageManager extends EventEmitter {
    * @property {Array} messages Holds all messages that have been created by this PersonalMessageManager
    *
    */
-  constructor() {
+  constructor({ metricsEvent }) {
     super();
     this.memStore = new ObservableStore({
       unapprovedPersonalMsgs: {},
       unapprovedPersonalMsgCount: 0,
     });
     this.messages = [];
+    this.metricsEvent = metricsEvent;
   }
 
   /**
@@ -238,7 +239,18 @@ export default class PersonalMessageManager extends EventEmitter {
    * @param {number} msgId - The id of the PersonalMessage to reject.
    *
    */
-  rejectMsg(msgId) {
+  rejectMsg(msgId, reason = undefined) {
+    if (reason) {
+      const msg = this.getMsg(msgId);
+      this.metricsEvent({
+        event: reason,
+        category: 'Transactions',
+        properties: {
+          action: 'Sign Request',
+          type: msg.type,
+        },
+      });
+    }
     this._setMsgStatus(msgId, 'rejected');
   }
 

app/scripts/lib/personal-message-manager.test.js

@@ -1,12 +1,13 @@
 import { strict as assert } from 'assert';
+import sinon from 'sinon';
 import { TRANSACTION_STATUSES } from '../../../shared/constants/transaction';
 import PersonalMessageManager from './personal-message-manager';
 
 describe('Personal Message Manager', function () {
   let messageManager;
 
   beforeEach(function () {
-    messageManager = new PersonalMessageManager();
+    messageManager = new PersonalMessageManager({ metricsEvent: sinon.fake() });
   });
 
   describe('#getMsgList', function () {

app/scripts/lib/typed-message-manager.js

@@ -32,14 +32,15 @@ export default class TypedMessageManager extends EventEmitter {
   /**
    * Controller in charge of managing - storing, adding, removing, updating - TypedMessage.
    */
-  constructor({ getCurrentChainId }) {
+  constructor({ getCurrentChainId, metricEvents }) {
     super();
     this._getCurrentChainId = getCurrentChainId;
     this.memStore = new ObservableStore({
       unapprovedTypedMessages: {},
       unapprovedTypedMessagesCount: 0,
     });
     this.messages = [];
+    this.metricEvents = metricEvents;
   }
 
   /**
@@ -301,7 +302,19 @@ export default class TypedMessageManager extends EventEmitter {
    * @param {number} msgId - The id of the TypedMessage to reject.
    *
    */
-  rejectMsg(msgId) {
+  rejectMsg(msgId, reason = undefined) {
+    if (reason) {
+      const msg = this.getMsg(msgId);
+      this.metricsEvent({
+        event: reason,
+        category: 'Transactions',
+        properties: {
+          action: 'Sign Request',
+          version: msg.msgParams.version,
+          type: msg.type,
+        },
+      });
+    }
     this._setMsgStatus(msgId, 'rejected');
   }
 

app/scripts/lib/typed-message-manager.test.js

@@ -17,6 +17,7 @@ describe('Typed Message Manager', function () {
   beforeEach(async function () {
     typedMessageManager = new TypedMessageManager({
       getCurrentChainId: sinon.fake.returns('0x1'),
+      metricsEvent: sinon.fake(),
     });
 
     msgParamsV1 = {

app/scripts/metamask-controller.js

@@ -526,14 +526,33 @@ export default class MetamaskController extends EventEmitter {
       }
     });
     this.networkController.lookupNetwork();
-    this.messageManager = new MessageManager();
-    this.personalMessageManager = new PersonalMessageManager();
-    this.decryptMessageManager = new DecryptMessageManager();
-    this.encryptionPublicKeyManager = new EncryptionPublicKeyManager();
+    this.messageManager = new MessageManager({
+      metricsEvent: this.metaMetricsController.trackEvent.bind(
+        this.metaMetricsController,
+      ),
+    });
+    this.personalMessageManager = new PersonalMessageManager({
+      metricsEvent: this.metaMetricsController.trackEvent.bind(
+        this.metaMetricsController,
+      ),
+    });
+    this.decryptMessageManager = new DecryptMessageManager({
+      metricsEvent: this.metaMetricsController.trackEvent.bind(
+        this.metaMetricsController,
+      ),
+    });
+    this.encryptionPublicKeyManager = new EncryptionPublicKeyManager({
+      metricsEvent: this.metaMetricsController.trackEvent.bind(
+        this.metaMetricsController,
+      ),
+    });
     this.typedMessageManager = new TypedMessageManager({
       getCurrentChainId: this.networkController.getCurrentChainId.bind(
         this.networkController,
       ),
+      metricsEvent: this.metaMetricsController.trackEvent.bind(
+        this.metaMetricsController,
+      ),
     });
 
     this.swapsController = new SwapsController({

app/scripts/platforms/extension.js

@@ -162,6 +162,10 @@ export default class ExtensionPlatform {
     }
   }
 
+  addOnRemovedListener(listener) {
+    extension.windows.onRemoved.addListener(listener);
+  }
+
   getAllWindows() {
     return new Promise((resolve, reject) => {
       extension.windows.getAll((windows) => {