Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Reset streams on BFCache events #24950

Merged
merged 31 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
dbbff4a
Add BFCache handling
jiexi May 30, 2024
d065afa
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi May 30, 2024
5f41b8f
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi May 31, 2024
b2b7f13
Add back warnings
jiexi May 31, 2024
a62f73f
Add back disconnect
jiexi May 31, 2024
0e86bc1
lint
jiexi May 31, 2024
aaa1844
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi May 31, 2024
851c995
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi May 31, 2024
dec9f77
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 3, 2024
195a2d0
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 10, 2024
1aaa95b
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 12, 2024
7ae24c3
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 13, 2024
8ac56f2
Add BFCache spec
jiexi Jun 13, 2024
fecc52b
Merge remote-tracking branch 'origin/jl/reset-streams-on-bfcache-page…
jiexi Jun 13, 2024
9856738
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 13, 2024
0deff1e
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 14, 2024
acc3afb
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 21, 2024
a1e1a39
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 21, 2024
4ac3009
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 21, 2024
7ef7385
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Jun 21, 2024
8d59def
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 20, 2024
8e38ccc
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 20, 2024
194b155
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 21, 2024
356b3e8
set METAMASK_EXTENSION_CONNECT_SENT to false after destroying streams
jiexi Nov 21, 2024
9ad9ea2
Merge remote-tracking branch 'origin/jl/reset-streams-on-bfcache-page…
jiexi Nov 21, 2024
6592fe6
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 21, 2024
2f1000c
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 21, 2024
2dfb287
remove BFCache spec loop
jiexi Nov 22, 2024
d943016
Merge remote-tracking branch 'origin/jl/reset-streams-on-bfcache-page…
jiexi Nov 22, 2024
e4d55a5
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 22, 2024
149c240
Merge branch 'develop' into jl/reset-streams-on-bfcache-pageshow
jiexi Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/scripts/contentscript.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { getIsBrowserPrerenderBroken } from '../../shared/modules/browser-runtime.utils';
import shouldInjectProvider from '../../shared/modules/provider-injection';
import {
destroyStreams,
initStreams,
onDisconnectDestroyStreams,
setupExtensionStreams,
} from './streams/provider-stream';
import {
isDetectedPhishingSite,
Expand Down Expand Up @@ -33,6 +35,20 @@ const start = () => {
);
});
}

window.addEventListener('pageshow', (event) => {
if (event.persisted) {
console.warn('BFCached page has become active. Restoring the streams.');
setupExtensionStreams();
}
});

window.addEventListener('pagehide', (event) => {
if (event.persisted) {
console.warn('Page may become BFCached. Destroying the streams.');
destroyStreams();
}
});
}
};

Expand Down
32 changes: 24 additions & 8 deletions app/scripts/streams/provider-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ let legacyExtMux: ObjectMultiplex,

let extensionMux: ObjectMultiplex,
extensionChannel: Substream,
extensionPort: browser.Runtime.Port,
extensionPort: browser.Runtime.Port | null,
extensionStream: PortStream | null,
pageMux: ObjectMultiplex,
pageChannel: Substream;
Expand Down Expand Up @@ -65,7 +65,7 @@ const setupPageStreams = () => {
// The field below is used to ensure that replay is done only once for each restart.
let METAMASK_EXTENSION_CONNECT_SENT = false;

const setupExtensionStreams = () => {
export const setupExtensionStreams = () => {
METAMASK_EXTENSION_CONNECT_SENT = true;
extensionPort = browser.runtime.connect({ name: CONTENT_SCRIPT });
extensionStream = new PortStream(extensionPort);
Expand Down Expand Up @@ -226,19 +226,35 @@ const onMessageSetUpExtensionStreams = (msg: MessageType) => {
return undefined;
};

/**
* Ends two-way communication streams between browser extension and
* the local per-page browser context.
*/
export function destroyStreams() {
if (!extensionPort) {
return;
}
extensionPort.onDisconnect.removeListener(onDisconnectDestroyStreams);

destroyExtensionStreams();
destroyLegacyExtensionStreams();

extensionPort.disconnect();
extensionPort = null;

METAMASK_EXTENSION_CONNECT_SENT = false;
}

/**
* This listener destroys the extension streams when the extension port is disconnected,
* so that streams may be re-established later when the extension port is reconnected.
*
* @param [err] - Stream connection error
*/
export const onDisconnectDestroyStreams = (err: unknown) => {
export function onDisconnectDestroyStreams(err: unknown) {
const lastErr = err || checkForLastError();

extensionPort.onDisconnect.removeListener(onDisconnectDestroyStreams);

destroyExtensionStreams();
destroyLegacyExtensionStreams();
destroyStreams();

/**
* If an error is found, reset the streams. When running two or more dapps, resetting the service
Expand All @@ -251,7 +267,7 @@ export const onDisconnectDestroyStreams = (err: unknown) => {
console.warn(`${lastErr} Resetting the streams.`);
setTimeout(setupExtensionStreams, 1000);
}
};
}

/**
* Initializes two-way communication streams between the browser extension and
Expand Down
65 changes: 65 additions & 0 deletions test/e2e/provider/bfcache.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const { strict: assert } = require('assert');
const {
withFixtures,
defaultGanacheOptions,
DAPP_URL,
openDapp,
} = require('../helpers');
const FixtureBuilder = require('../fixture-builder');

const triggerBFCache = async (driver) => {
await driver.executeScript(`
window.addEventListener('pageshow', (event) => {
if (event.persisted) {
window.restoredFromBFCache = true
}
});
`);

await driver.driver.get(`chrome://terms/`);

await driver.driver.navigate().back();

const restoredFromBFCache = await driver.executeScript(
`return window.restoredFromBFCache`,
);

if (!restoredFromBFCache) {
assert.fail(new Error('Failed to trigger BFCache'));
}
};

describe('BFCache', function () {
it('has a working provider stream when a dapp is restored from BFCache', async function () {
await withFixtures(
{
dapp: true,
fixtures: new FixtureBuilder().build(),
ganacheOptions: defaultGanacheOptions,
title: this.test.fullTitle(),
},
async ({ driver }) => {
await openDapp(driver, undefined, DAPP_URL);

const request = JSON.stringify({
jsonrpc: '2.0',
method: 'eth_chainId',
params: [],
id: 0,
});

const initialResult = await driver.executeScript(
`return window.ethereum.request(${request})`,
);
assert.equal(initialResult, '0x539');

await triggerBFCache(driver);

const bfcacheResult = await driver.executeScript(
`return window.ethereum.request(${request})`,
);
assert.equal(bfcacheResult, '0x539');
},
);
});
});
Loading