fix: reset streams on prerender only for affected browser versions (#…

…24317)  ## **Description** The prerender regression in chromium has been resolved already. Resetting streams on prerendered pages no longer required for newer chromium versions and is actually the cause of unresponsive provider in many cases. This PR modifies the existing workaround to target only the affected chromium ranges from >=113 to <121. My bisection of chromium for the fix landed on this range of commits: https://chromium.googlesource.com/chromium/src/+log/55b4344edfb41dda980d197743f25a2841d498a4..c13107c16780c195bd5ec003d9198d87cdcc59dd @shanejonas and I have verified that the prerender issue has been resolved in the latest chrome stable. I have verified that the workaround is causing an issue in develop that is no longer seen in this branch that reverts it. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24293?quickstart=1) ## **Related issues** See: #19727 ## **Manual testing steps** Using chrome, of course. Visit `chrome://settings/?search=preload` and ensure it is enabled and on "standard" 1. On develop, make a build and load it 2. Visit `https://voyager-snap.linea.build/` 3. Install the snap and connect 4. Copy `https://voyager-snap.linea.build/` to your clipboard 5. Close all instances of `https://voyager-snap.linea.build/` that may be open 6. Rapidly, open a new tab, paste the url, hit enter, see if the page is stuck on "Loading", if not close the tab and repeat 7. Eventually the page should be get prerendered and stuck in a broken state Do the same steps above, but using the changes in this branch. The page should never be stuck with "Loading" if you are on a chromium version from 102 to 112, or 121 onwards. ## **Screenshots/Recordings**  ### **Before** https://github.com/MetaMask/metamask-extension/assets/918701/6bda7a5d-12d2-46e4-b5d6-8562c3143977 ### **After** https://github.com/MetaMask/metamask-extension/assets/918701/14289f0e-89eb-4534-8815-615a57e0386b ### Browser version targeting proof The following screenshots show the return value for `getIsBrowserPrerenderBroken()` on various chromium versions 120: ![Screenshot 2024-04-30 at 12 47 57 PM](https://github.com/MetaMask/metamask-extension/assets/918701/8ce52f2f-7b1d-4825-a265-9fabae1f7be4) 119: ![Screenshot 2024-04-30 at 12 50 57 PM](https://github.com/MetaMask/metamask-extension/assets/918701/965fe680-2579-4bd8-98db-3cf19a6dfaed) 113: ![Screenshot 2024-04-30 at 11 22 20 AM](https://github.com/MetaMask/metamask-extension/assets/918701/7b643d07-4731-444f-b888-d0a49a79feb5) 112: ![Screenshot 2024-04-30 at 11 20 43 AM](https://github.com/MetaMask/metamask-extension/assets/918701/80040268-c137-4d98-9dd9-661f6c78ade7) ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · May 9, 2024 · 1357fdb · 1357fdb
1 parent f9b2ea1
commit 1357fdb
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 6 deletions.
diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js
@@ -5,7 +5,10 @@ import pump from 'pump';
 import { obj as createThoughStream } from 'through2';
 import browser from 'webextension-polyfill';
 import { EXTENSION_MESSAGES } from '../../shared/constants/app';
-import { checkForLastError } from '../../shared/modules/browser-runtime.utils';
+import {
+  checkForLastError,
+  getIsBrowserPrerenderBroken,
+} from '../../shared/modules/browser-runtime.utils';
 import { isManifestV3 } from '../../shared/modules/mv3.utils';
 import shouldInjectProvider from '../../shared/modules/provider-injection';
 
@@ -519,11 +522,7 @@ const start = () => {
   if (shouldInjectProvider()) {
     initStreams();
 
-    // https://bugs.chromium.org/p/chromium/issues/detail?id=1457040
-    // Temporary workaround for chromium bug that breaks the content script <=> background connection
-    // for prerendered pages. This resets potentially broken extension streams if a page transitions
-    // from the prerendered state to the active state.
-    if (document.prerendering) {
+    if (document.prerendering && getIsBrowserPrerenderBroken()) {
       document.addEventListener('prerenderingchange', () => {
         onDisconnectDestroyStreams(
           new Error('Prerendered page has become active.'),

diff --git a/shared/modules/browser-runtime.utils.js b/shared/modules/browser-runtime.utils.js
@@ -2,8 +2,13 @@
  * Utility Functions to support browser.runtime JavaScript API
  */
 
+import Bowser from 'bowser';
 import browser from 'webextension-polyfill';
 import log from 'loglevel';
+import {
+  BROKEN_PRERENDER_BROWSER_VERSIONS,
+  FIXED_PRERENDER_BROWSER_VERSIONS,
+} from '../../ui/helpers/constants/common';
 
 /**
  * Returns an Error if extension.runtime.lastError is present
@@ -53,3 +58,21 @@ export function checkForLastErrorAndWarn() {
 
   return error;
 }
+
+/**
+ * Returns true if the browser is affected by a regression that causes the
+ * extension port stream established between the contentscript and background
+ * to be broken when a prerendered (eagerly rendered, hidden) page becomes active (visible to the user).
+ *
+ * @param {Bowser} bowser - optional Bowser instance to check against
+ * @returns {boolean} Whether the browser is affected by the prerender regression
+ */
+export function getIsBrowserPrerenderBroken(
+  bowser = Bowser.getParser(window.navigator.userAgent),
+) {
+  return (
+    (bowser.satisfies(BROKEN_PRERENDER_BROWSER_VERSIONS) &&
+      !bowser.satisfies(FIXED_PRERENDER_BROWSER_VERSIONS)) ??
+    false
+  );
+}
diff --git a/shared/modules/browser-runtime.utils.test.js b/shared/modules/browser-runtime.utils.test.js
@@ -1,4 +1,5 @@
 import sinon from 'sinon';
+import Bowser from 'bowser';
 import browser from 'webextension-polyfill';
 import log from 'loglevel';
 import * as BrowserRuntimeUtil from './browser-runtime.utils';
@@ -51,4 +52,101 @@ describe('Browser Runtime Utils', () => {
       log.error.restore();
     });
   });
+
+  describe('getIsBrowserPrerenderBroken', () => {
+    it('should call Bowser.getParser when no parameter is passed', () => {
+      const spy = jest.spyOn(Bowser, 'getParser');
+      BrowserRuntimeUtil.getIsBrowserPrerenderBroken();
+      expect(spy).toHaveBeenCalled();
+    });
+    it.each([
+      ['windows', '112.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['windows', '120.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['macos', '112.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['macos', '120.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['linux', '112.0.0.0', 'X11; Linux x86_64'],
+      ['linux', '121.0.0.0', 'X11; Linux x86_64'],
+    ])(
+      'should return false when given a chrome browser with working prerender in %s on version %s',
+      (_, version, os) => {
+        const bowser = Bowser.getParser(
+          `Mozilla/5.0 (${os}) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/${version} Safari/537.36`,
+        );
+        const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+        expect(result).toStrictEqual(false);
+      },
+    );
+    it.each([
+      ['windows', '113.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['windows', '119.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['macos', '113.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['macos', '119.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['linux', '113.0.0.0', 'X11; Linux x86_64'],
+      ['linux', '120.0.0.0', 'X11; Linux x86_64'],
+    ])(
+      'should return true when given a chrome browser with broken prerender in %s on version %s',
+      (_, version, os) => {
+        const bowser = Bowser.getParser(
+          `Mozilla/5.0 (${os}) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/${version} Safari/537.36`,
+        );
+        const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+        expect(result).toStrictEqual(true);
+      },
+    );
+    it.each([
+      ['windows', '112.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['windows', '120.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['macos', '112.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['macos', '120.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['linux', '112.0.0.0', 'X11; Linux x86_64'],
+      ['linux', '121.0.0.0', 'X11; Linux x86_64'],
+    ])(
+      'should return false when given an edge browser with working prerender in %s on version %s',
+      (_, version, os) => {
+        const bowser = Bowser.getParser(
+          `Mozilla/5.0 (${os}) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/i${version} Safari/537.36 Edg/${version}`,
+        );
+        const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+        expect(result).toStrictEqual(false);
+      },
+    );
+    it.each([
+      ['windows', '113.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['windows', '119.0.0.0', 'Windows NT 10.0; Win64; x64'],
+      ['macos', '113.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['macos', '119.0.0.0', 'Macintosh; Intel Mac OS X 10_16_0'],
+      ['linux', '113.0.0.0', 'X11; Linux x86_64'],
+      ['linux', '120.0.0.0', 'X11; Linux x86_64'],
+    ])(
+      'should return true when given an edge browser with broken prerender in %s on version %s',
+      (_, version, os) => {
+        const bowser = Bowser.getParser(
+          `Mozilla/5.0 (${os}) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/i${version} Safari/537.36 Edg/${version}`,
+        );
+        const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+        expect(result).toStrictEqual(true);
+      },
+    );
+    it('should return false when given a firefox browser', () => {
+      const bowser = Bowser.getParser(
+        'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/91.0',
+      );
+      const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+      expect(result).toStrictEqual(false);
+    });
+    it('should return false when given an opera browser', () => {
+      const bowser = Bowser.getParser(
+        'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_16_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.3578.98 Safari/537.36 OPR/76.0.3135.47',
+      );
+      const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+      expect(result).toStrictEqual(false);
+    });
+    it('should return false when given an unknown browser', () => {
+      const bowser = Bowser.getParser(
+        'Mozilla/5.0 (Nintendo Switch; WebApplet) AppleWebKit/609.4 (KHTML, like Gecko) NF/6.0.2.21.3 NintendoBrowser/5.1.0.22474',
+      );
+      const result = BrowserRuntimeUtil.getIsBrowserPrerenderBroken(bowser);
+      expect(result).toStrictEqual(false);
+    });
+  });
 });
diff --git a/ui/helpers/constants/common.ts b/ui/helpers/constants/common.ts
@@ -23,3 +23,37 @@ export const OUTDATED_BROWSER_VERSIONS = {
   // See https://en.wikipedia.org/wiki/History_of_the_Opera_web_browser
   opera: '<76',
 };
+
+/**
+ * Specifies the browser and their versions where a regression in the extension port
+ * stream established between the contentscript and background was breaking for
+ * prerendered pages.
+ *
+ * @see {@link https://issues.chromium.org/issues/40273420}
+ */
+export const BROKEN_PRERENDER_BROWSER_VERSIONS = {
+  chrome: '>=113',
+  edge: '>=113',
+};
+
+/**
+ * Specifies the browser and their versions on a specific OS where a fix for the
+ * prerender regression specified in BROKEN_PRERENDER_BROWSER_VERSIONS was resolved.
+ *
+ * @see {@link https://chromium.googlesource.com/chromium/src/+/a88eee8a2798c1dc4d69b255ccad24fea5ff2d8b}
+ */
+export const FIXED_PRERENDER_BROWSER_VERSIONS = {
+  // https://chromiumdash.appspot.com/commits?commit=a88eee8a2798c1dc4d69b255ccad24fea5ff2d8b&platform=Windows
+  windows: {
+    chrome: '>=120',
+    edge: '>=120',
+  },
+  // https://chromiumdash.appspot.com/commits?commit=a88eee8a2798c1dc4d69b255ccad24fea5ff2d8b&platform=Mac
+  macos: {
+    chrome: '>=120',
+    edge: '>=120',
+  },
+  // https://chromiumdash.appspot.com/commits?commit=a88eee8a2798c1dc4d69b255ccad24fea5ff2d8b&platform=Linux
+  chrome: '>=121',
+  edge: '>=121',
+};