Stop GasFeeController polling when pop closes (#11746)

* Stop GasFeeController polling when pop closes

* Stop estimate gas polling on window unload

* lint + comments

* Improve client closed logic

* lint

* Add back _beforeUnload on unmount in gas-modal-page-container

* Add full check and call onClientClosed method for notifcation environment

* Add gas pollingToken tracking to appStateController and use to disconnect polling for each environment type

* remove unused method

* move controller manipulation logic from background.js to metamask-controller, disaggregate methods

* add beforeunload handling to reset gas polling tokens from root of send page

* cleanup, lint and address feedback

* clear appState gasPollingTokens when all instances of all env types are closed, fix pollingTokenType arg from onEnvironmentTypeClosed call in metamask-controller

* mock new methods to fix tests

* final bit of cleanup + comments

Co-authored-by: Dan Miller <danjm.com@gmail.com>

app/scripts/background.js

@@ -302,6 +302,24 @@ function setupController(initState, initLangCode) {
     );
   };
 
+  const onCloseEnvironmentInstances = (isClientOpen, environmentType) => {
+    // if all instances of metamask are closed we call a method on the controller to stop gasFeeController polling
+    if (isClientOpen === false) {
+      controller.onClientClosed();
+      // otherwise we want to only remove the polling tokens for the environment type that has closed
+    } else {
+      // in the case of fullscreen environment a user might have multiple tabs open so we don't want to disconnect all of
+      // its corresponding polling tokens unless all tabs are closed.
+      if (
+        environmentType === ENVIRONMENT_TYPE_FULLSCREEN &&
+        Boolean(Object.keys(openMetamaskTabsIDs).length)
+      ) {
+        return;
+      }
+      controller.onEnvironmentTypeClosed(environmentType);
+    }
+  };
+
   /**
    * A runtime.Port object, as provided by the browser:
    * @see https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/Port
@@ -330,10 +348,11 @@ function setupController(initState, initLangCode) {
 
       if (processName === ENVIRONMENT_TYPE_POPUP) {
         popupIsOpen = true;
-
         endOfStream(portStream, () => {
           popupIsOpen = false;
-          controller.isClientOpen = isClientOpenStatus();
+          const isClientOpen = isClientOpenStatus();
+          controller.isClientOpen = isClientOpen;
+          onCloseEnvironmentInstances(isClientOpen, ENVIRONMENT_TYPE_POPUP);
         });
       }
 
@@ -342,7 +361,12 @@ function setupController(initState, initLangCode) {
 
         endOfStream(portStream, () => {
           notificationIsOpen = false;
-          controller.isClientOpen = isClientOpenStatus();
+          const isClientOpen = isClientOpenStatus();
+          controller.isClientOpen = isClientOpen;
+          onCloseEnvironmentInstances(
+            isClientOpen,
+            ENVIRONMENT_TYPE_NOTIFICATION,
+          );
         });
       }
 
@@ -352,7 +376,12 @@ function setupController(initState, initLangCode) {
 
         endOfStream(portStream, () => {
           delete openMetamaskTabsIDs[tabId];
-          controller.isClientOpen = isClientOpenStatus();
+          const isClientOpen = isClientOpenStatus();
+          controller.isClientOpen = isClientOpen;
+          onCloseEnvironmentInstances(
+            isClientOpen,
+            ENVIRONMENT_TYPE_FULLSCREEN,
+          );
         });
       }
     } else {

app/scripts/controllers/app-state.js

@@ -25,6 +25,9 @@ export default class AppStateController extends EventEmitter {
       connectedStatusPopoverHasBeenShown: true,
       defaultHomeActiveTabName: null,
       browserEnvironment: {},
+      popupGasPollTokens: [],
+      notificationGasPollTokens: [],
+      fullScreenGasPollTokens: [],
       recoveryPhraseReminderHasBeenShown: false,
       recoveryPhraseReminderLastShown: new Date().getTime(),
       ...initState,
@@ -191,4 +194,38 @@ export default class AppStateController extends EventEmitter {
   setBrowserEnvironment(os, browser) {
     this.store.updateState({ browserEnvironment: { os, browser } });
   }
+
+  /**
+   * Adds a pollingToken for a given environmentType
+   * @returns {void}
+   */
+  addPollingToken(pollingToken, pollingTokenType) {
+    const prevState = this.store.getState()[pollingTokenType];
+    this.store.updateState({
+      [pollingTokenType]: [...prevState, pollingToken],
+    });
+  }
+
+  /**
+   * removes a pollingToken for a given environmentType
+   * @returns {void}
+   */
+  removePollingToken(pollingToken, pollingTokenType) {
+    const prevState = this.store.getState()[pollingTokenType];
+    this.store.updateState({
+      [pollingTokenType]: prevState.filter((token) => token !== pollingToken),
+    });
+  }
+
+  /**
+   * clears all pollingTokens
+   * @returns {void}
+   */
+  clearPollingTokens() {
+    this.store.updateState({
+      popupGasPollTokens: [],
+      notificationGasPollTokens: [],
+      fullScreenGasPollTokens: [],
+    });
+  }
 }

app/scripts/metamask-controller.js

@@ -32,6 +32,7 @@ import { MAINNET_CHAIN_ID } from '../../shared/constants/network';
 import { UI_NOTIFICATIONS } from '../../shared/notifications';
 import { toChecksumHexAddress } from '../../shared/modules/hexstring-utils';
 import { MILLISECOND } from '../../shared/constants/time';
+import { POLLING_TOKEN_ENVIRONMENT_TYPES } from '../../shared/constants/app';
 
 import { hexToDecimal } from '../../ui/helpers/utils/conversions.util';
 import ComposableObservableStore from './lib/ComposableObservableStore';
@@ -1130,6 +1131,16 @@ export default class MetamaskController extends EventEmitter {
         this.gasFeeController.getTimeEstimate,
         this.gasFeeController,
       ),
+
+      addPollingTokenToAppState: nodeify(
+        this.appStateController.addPollingToken,
+        this.appStateController,
+      ),
+
+      removePollingTokenFromAppState: nodeify(
+        this.appStateController.removePollingToken,
+        this.appStateController,
+      ),
     };
   }
 
@@ -2969,7 +2980,6 @@ export default class MetamaskController extends EventEmitter {
   /* eslint-disable accessor-pairs */
   /**
    * A method for recording whether the MetaMask user interface is open or not.
-   * @private
    * @param {boolean} open
    */
   set isClientOpen(open) {
@@ -2978,6 +2988,38 @@ export default class MetamaskController extends EventEmitter {
   }
   /* eslint-enable accessor-pairs */
 
+  /**
+   * A method that is called by the background when all instances of metamask are closed.
+   * Currently used to stop polling in the gasFeeController.
+   */
+  onClientClosed() {
+    try {
+      this.gasFeeController.stopPolling();
+      this.appStateController.clearPollingTokens();
+    } catch (error) {
+      console.error(error);
+    }
+  }
+
+  /**
+   * A method that is called by the background when a particular environment type is closed (fullscreen, popup, notification).
+   * Currently used to stop polling in the gasFeeController for only that environement type
+   */
+  onEnvironmentTypeClosed(environmentType) {
+    const appStatePollingTokenType =
+      POLLING_TOKEN_ENVIRONMENT_TYPES[environmentType];
+    const pollingTokensToDisconnect = this.appStateController.store.getState()[
+      appStatePollingTokenType
+    ];
+    pollingTokensToDisconnect.forEach((pollingToken) => {
+      this.gasFeeController.disconnectPoller(pollingToken);
+      this.appStateController.removePollingToken(
+        pollingToken,
+        appStatePollingTokenType,
+      );
+    });
+  }
+
   /**
    * Adds a domain to the PhishingController safelist
    * @param {string} hostname - the domain to safelist

shared/constants/app.js

@@ -30,3 +30,9 @@ export const MESSAGE_TYPE = {
   ADD_ETHEREUM_CHAIN: 'wallet_addEthereumChain',
   SWITCH_ETHEREUM_CHAIN: 'wallet_switchEthereumChain',
 };
+
+export const POLLING_TOKEN_ENVIRONMENT_TYPES = {
+  [ENVIRONMENT_TYPE_POPUP]: 'popupGasPollTokens',
+  [ENVIRONMENT_TYPE_NOTIFICATION]: 'notificationGasPollTokens',
+  [ENVIRONMENT_TYPE_FULLSCREEN]: 'fullScreenGasPollTokens',
+};

ui/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container-component.test.js

@@ -13,6 +13,7 @@ jest.mock('../../../../store/actions', () => ({
   getGasFeeEstimatesAndStartPolling: jest
     .fn()
     .mockImplementation(() => Promise.resolve()),
+  addPollingTokenToAppState: jest.fn(),
 }));
 
 const propsMethodSpies = {

ui/components/app/gas-customization/gas-modal-page-container/gas-modal-page-container.component.js

@@ -5,6 +5,8 @@ import { Tabs, Tab } from '../../../ui/tabs';
 import {
   disconnectGasFeeEstimatePoller,
   getGasFeeEstimatesAndStartPolling,
+  addPollingTokenToAppState,
+  removePollingTokenFromAppState,
 } from '../../../../store/actions';
 import AdvancedTabContent from './advanced-tab-content';
 import BasicTabContent from './basic-tab-content';
@@ -52,18 +54,27 @@ export default class GasModalPageContainer extends Component {
     this._isMounted = true;
     getGasFeeEstimatesAndStartPolling().then((pollingToken) => {
       if (this._isMounted) {
+        addPollingTokenToAppState(pollingToken);
         this.setState({ pollingToken });
       } else {
         disconnectGasFeeEstimatePoller(pollingToken);
+        removePollingTokenFromAppState(pollingToken);
       }
     });
+    window.addEventListener('beforeunload', this._beforeUnload);
   }
 
-  componentWillUnmount() {
+  _beforeUnload = () => {
     this._isMounted = false;
     if (this.state.pollingToken) {
       disconnectGasFeeEstimatePoller(this.state.pollingToken);
+      removePollingTokenFromAppState(this.state.pollingToken);
     }
+  };
+
+  componentWillUnmount() {
+    this._beforeUnload();
+    window.removeEventListener('beforeunload', this._beforeUnload);
   }
 
   renderBasicTabContent(gasPriceButtonGroupProps) {

ui/ducks/send/send.js

@@ -50,6 +50,8 @@ import {
   showLoadingIndication,
   updateTokenType,
   updateTransaction,
+  addPollingTokenToAppState,
+  removePollingTokenFromAppState,
 } from '../../store/actions';
 import { setCustomGasLimit } from '../gas/gas.duck';
 import {
@@ -84,7 +86,6 @@ import {
 import { CHAIN_ID_TO_GAS_LIMIT_BUFFER_MAP } from '../../../shared/constants/network';
 import { ETH, GWEI } from '../../helpers/constants/common';
 import { TRANSACTION_ENVELOPE_TYPES } from '../../../shared/constants/transaction';
-
 // typedefs
 /**
  * @typedef {import('@reduxjs/toolkit').PayloadAction} PayloadAction
@@ -437,6 +438,9 @@ export const initializeSendState = createAsyncThunk(
 
     // Instruct the background process that polling for gas prices should begin
     gasEstimatePollToken = await getGasFeeEstimatesAndStartPolling();
+
+    addPollingTokenToAppState(gasEstimatePollToken);
+
     const {
       metamask: { gasFeeEstimates, gasEstimateType },
     } = thunkApi.getState();
@@ -1298,6 +1302,7 @@ export function resetSendState() {
       await disconnectGasFeeEstimatePoller(
         state[name].gas.gasEstimatePollToken,
       );
+      removePollingTokenFromAppState(state[name].gas.gasEstimatePollToken);
     }
   };
 }

ui/hooks/useGasFeeEstimates.test.js

@@ -16,6 +16,8 @@ import { useGasFeeEstimates } from './useGasFeeEstimates';
 jest.mock('../store/actions', () => ({
   disconnectGasFeeEstimatePoller: jest.fn(),
   getGasFeeEstimatesAndStartPolling: jest.fn(),
+  addPollingTokenToAppState: jest.fn(),
+  removePollingTokenFromAppState: jest.fn(),
 }));
 
 jest.mock('react-redux', () => {

ui/hooks/useSafeGasEstimatePolling.js

@@ -2,6 +2,8 @@ import { useEffect } from 'react';
 import {
   disconnectGasFeeEstimatePoller,
   getGasFeeEstimatesAndStartPolling,
+  addPollingTokenToAppState,
+  removePollingTokenFromAppState,
 } from '../store/actions';
 
 /**
@@ -16,18 +18,30 @@ export function useSafeGasEstimatePolling() {
   useEffect(() => {
     let active = true;
     let pollToken;
+
+    const cleanup = () => {
+      active = false;
+      if (pollToken) {
+        disconnectGasFeeEstimatePoller(pollToken);
+        removePollingTokenFromAppState(pollToken);
+      }
+    };
+
     getGasFeeEstimatesAndStartPolling().then((newPollToken) => {
       if (active) {
         pollToken = newPollToken;
+        addPollingTokenToAppState(pollToken);
       } else {
         disconnectGasFeeEstimatePoller(newPollToken);
+        removePollingTokenFromAppState(pollToken);
       }
     });
+
+    window.addEventListener('beforeunload', cleanup);
+
     return () => {
-      active = false;
-      if (pollToken) {
-        disconnectGasFeeEstimatePoller(pollToken);
-      }
+      cleanup();
+      window.removeEventListener('beforeunload', cleanup);
     };
   }, []);
 }

ui/pages/confirm-transaction-base/confirm-transaction-base.component.js

@@ -40,6 +40,8 @@ import { COLORS } from '../../helpers/constants/design-system';
 import {
   disconnectGasFeeEstimatePoller,
   getGasFeeEstimatesAndStartPolling,
+  addPollingTokenToAppState,
+  removePollingTokenFromAppState,
 } from '../../store/actions';
 
 export default class ConfirmTransactionBase extends Component {
@@ -679,10 +681,19 @@ export default class ConfirmTransactionBase extends Component {
     cancelTransaction({ id });
   };
 
+  _beforeUnloadForGasPolling = () => {
+    this._isMounted = false;
+    if (this.state.pollingToken) {
+      disconnectGasFeeEstimatePoller(this.state.pollingToken);
+      removePollingTokenFromAppState(this.state.pollingToken);
+    }
+  };
+
   _removeBeforeUnload = () => {
     if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
       window.removeEventListener('beforeunload', this._beforeUnload);
     }
+    window.removeEventListener('beforeunload', this._beforeUnloadForGasPolling);
   };
 
   componentDidMount() {
@@ -723,18 +734,18 @@ export default class ConfirmTransactionBase extends Component {
      */
     getGasFeeEstimatesAndStartPolling().then((pollingToken) => {
       if (this._isMounted) {
+        addPollingTokenToAppState(pollingToken);
         this.setState({ pollingToken });
       } else {
         disconnectGasFeeEstimatePoller(pollingToken);
+        removePollingTokenFromAppState(this.state.pollingToken);
       }
     });
+    window.addEventListener('beforeunload', this._beforeUnloadForGasPolling);
   }
 
   componentWillUnmount() {
-    this._isMounted = false;
-    if (this.state.pollingToken) {
-      disconnectGasFeeEstimatePoller(this.state.pollingToken);
-    }
+    this._beforeUnloadForGasPolling();
     this._removeBeforeUnload();
   }
 

ui/pages/confirm-transaction/confirm-transaction.component.js

@@ -28,6 +28,8 @@ import {
 import {
   disconnectGasFeeEstimatePoller,
   getGasFeeEstimatesAndStartPolling,
+  addPollingTokenToAppState,
+  removePollingTokenFromAppState,
 } from '../../store/actions';
 import ConfTx from './conf-tx';
 
@@ -57,6 +59,14 @@ export default class ConfirmTransaction extends Component {
     this.state = {};
   }
 
+  _beforeUnload = () => {
+    this._isMounted = false;
+    if (this.state.pollingToken) {
+      disconnectGasFeeEstimatePoller(this.state.pollingToken);
+      removePollingTokenFromAppState(this.state.pollingToken);
+    }
+  };
+
   componentDidMount() {
     this._isMounted = true;
     const {
@@ -75,11 +85,15 @@ export default class ConfirmTransaction extends Component {
     getGasFeeEstimatesAndStartPolling().then((pollingToken) => {
       if (this._isMounted) {
         this.setState({ pollingToken });
+        addPollingTokenToAppState(pollingToken);
       } else {
         disconnectGasFeeEstimatePoller(pollingToken);
+        removePollingTokenFromAppState(pollingToken);
       }
     });
 
+    window.addEventListener('beforeunload', this._beforeUnload);
+
     if (!totalUnapprovedCount && !sendTo) {
       history.replace(mostRecentOverviewPage);
       return;
@@ -96,10 +110,8 @@ export default class ConfirmTransaction extends Component {
   }
 
   componentWillUnmount() {
-    this._isMounted = false;
-    if (this.state.pollingToken) {
-      disconnectGasFeeEstimatePoller(this.state.pollingToken);
-    }
+    this._beforeUnload();
+    window.removeEventListener('beforeunload', this._beforeUnload);
   }
 
   componentDidUpdate(prevProps) {

ui/pages/send/send.js

@@ -1,4 +1,4 @@
-import React, { useEffect } from 'react';
+import React, { useEffect, useCallback } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { useHistory, useLocation } from 'react-router-dom';
 import {
@@ -47,11 +47,17 @@ export default function SendTransactionScreen() {
   });
 
   const dispatch = useDispatch();
+
+  const cleanup = useCallback(() => {
+    dispatch(resetSendState());
+  }, [dispatch]);
+
   useEffect(() => {
     if (chainId !== undefined) {
       dispatch(initializeSendState());
+      window.addEventListener('beforeunload', cleanup);
     }
-  }, [chainId, dispatch]);
+  }, [chainId, dispatch, cleanup]);
 
   useEffect(() => {
     if (location.search === '?scan=true') {
@@ -67,8 +73,9 @@ export default function SendTransactionScreen() {
   useEffect(() => {
     return () => {
       dispatch(resetSendState());
+      window.removeEventListener('beforeunload', cleanup);
     };
-  }, [dispatch]);
+  }, [dispatch, cleanup]);
 
   let content;
 

ui/store/actions.js

@@ -10,7 +10,10 @@ import {
 import { getMethodDataAsync } from '../helpers/utils/transactions.util';
 import { getSymbolAndDecimals } from '../helpers/utils/token-util';
 import switchDirection from '../helpers/utils/switch-direction';
-import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../shared/constants/app';
+import {
+  ENVIRONMENT_TYPE_NOTIFICATION,
+  POLLING_TOKEN_ENVIRONMENT_TYPES,
+} from '../../shared/constants/app';
 import { hasUnconfirmedTransactions } from '../helpers/utils/confirm-tx.util';
 import txHelper from '../helpers/utils/tx-helper';
 import { getEnvironmentType, addHexPrefix } from '../../app/scripts/lib/util';
@@ -2787,6 +2790,20 @@ export function disconnectGasFeeEstimatePoller(pollToken) {
   return promisifiedBackground.disconnectGasFeeEstimatePoller(pollToken);
 }
 
+export async function addPollingTokenToAppState(pollingToken) {
+  return promisifiedBackground.addPollingTokenToAppState(
+    pollingToken,
+    POLLING_TOKEN_ENVIRONMENT_TYPES[getEnvironmentType()],
+  );
+}
+
+export async function removePollingTokenFromAppState(pollingToken) {
+  return promisifiedBackground.removePollingTokenFromAppState(
+    pollingToken,
+    POLLING_TOKEN_ENVIRONMENT_TYPES[getEnvironmentType()],
+  );
+}
+
 export function getGasFeeTimeEstimate(maxPriorityFeePerGas, maxFeePerGas) {
   return promisifiedBackground.getGasFeeTimeEstimate(
     maxPriorityFeePerGas,