Skip to content

Commit

Permalink
Reland "Dispatch close event when MessagePort is disentangled"
Browse files Browse the repository at this point in the history
This is a reland of commit 6a74d3d8049e1744b862757a73f90f3d7a5e08b0

The tests newly added by the original CL were failing on
chrome_wpt_tests on the Linux Tests bot:
https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests/138259/overview

I changed two tests as follows
・third_party/blink/web_tests/external/wpt/webmessaging/message-channels/close-event/garbage-collected.tentative.any.js
→in order to prevent the test timing out, I added timmeout=long

・third_party/blink/web_tests/external/wpt/html/browsers/browsing-the-web/back-forward-cache/message-port-entangled-after-restore.https.tentative.window.js
→change test expectations

Original change's description:
> Dispatch close event when MessagePort is disentangled
>
> We want to dispatch a close event when an entangled MessagePort is disconnected.
> Given a pair of entangled ports, port1 and port2, if port2 is closed at any point,
> a port1’s error handler is run.
> So we can change an error handler to dispatch a close event.
>
> The tests of close event are as follows:
> 1) port was explicitly closed.
> 2) owning document was destroyed.
> 3) owning document crashed.
> 4) port was garbage collected.
>
> Design Doc:https://docs.google.com/document/d/1lXZU2Pk2ycitqj8aL9kxT2aauwXqpA0vJDUalkXA29Y
> Explainer:https://github.com/fergald/explainer-messageport-close
> HTML spec PR:whatwg/html#9933
>
> Bug: 1495616
> Change-Id: I99f9f5a0d7cc63f0916da316ec666ba793215019
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5003089
> Reviewed-by: Jeremy Roman <jbroman@chromium.org>
> Commit-Queue: Nonoka Muraki <murakinonoka@chromium.org>
> Reviewed-by: Ming-Ying Chung <mych@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1231743}

Change-Id: I0a8f31245d7b5abe9ddfc262dff8063316e953df
  • Loading branch information
nononokam authored and chromium-wpt-export-bot committed Dec 4, 2023
1 parent cce26e3 commit 352b7f1
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// META: timeout=long
// META: title=Confirm close event is not fired when the page enters BFCache and MessagePort still works after the page is restored.
// META: script=/common/dispatcher/dispatcher.js
// META: script=/common/get-host-info.sub.js
// META: script=/common/utils.js
// META: script=/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
// META: script=/service-workers/service-worker/resources/test-helpers.sub.js
// META: script=resources/rc-helper.js

promise_test(async t => {
// Register a service worker.
const scope =
'/html/browsers/browsing-the-web/remote-context-helper/resources'
const workerUrl =
`resources/service-worker.js?pipe=header(Service-Worker-Allowed,${
scope})`;
const registration =
await service_worker_unregister_and_register(t, workerUrl, scope);
t.add_cleanup(_ => registration.unregister());
await wait_for_state(t, registration.installing, 'activated');

// Open a window with noopener so that BFCache will work.
const rcHelper = new RemoteContextHelper();
const rc1 = await rcHelper.addWindow(
/*extraConfig=*/ null, /*options=*/ {features: 'noopener'});

// Confirm the page is controlled.
assert_true(
await rc1.executeScript(
() => (navigator.serviceWorker.controller !== null)),
'The page should be controlled before navigation');

// Send MessagePort to the service worker.
await rc1.executeScript(() => {
const {port1, port2} = new MessageChannel();
port1.start();
const ctrl = navigator.serviceWorker.controller;
ctrl.postMessage({type: 'storeMessagePort'}, [port2]);
self.waitForMessage = (sentMessage) => {
return new Promise(resolve => {
port1.addEventListener('message', (event) => {
resolve(event.data);
});
port1.postMessage(sentMessage);
});
};
});

// Verify that the page was BFCached.
await assertBFCacheEligibility(rc1, /*shouldRestoreFromBFCache=*/ true);

// Confirm MessagePort can still work after the page is restored from
// BFCache.
assert_equals(
await rc1.executeScript(
async () =>
await self.waitForMessage('Confirm the ports can communicate')),
'Receive message');

// Confirm the close event was not fired.
assert_false(await rc1.executeScript(
async () =>
await self.waitForMessage('Ask if the close event was fired')));
}, 'MessagePort still works after the page is restored from BFCache');
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ self.addEventListener('message', function(event) {
client.postMessage("dummyValue");
}
event.data.port.postMessage("PASS");
} else if (event.data.type == 'storeMessagePort') {
let isCloseEventFired = false;
const port = event.ports[0];
port.start();
port.onmessage = (event) => {
if (event.data == 'Confirm the ports can communicate') {
port.postMessage('Receive message');
} else if (event.data == 'Ask if the close event was fired') {
port.postMessage(isCloseEventFired);
}
};
port.onclose = () => {
isCloseEventFired = true;
};
}
});

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// META: title=Close event test when the document is destroyed.
// META: script=/common/dispatcher/dispatcher.js
// META: script=/common/get-host-info.sub.js
// META: script=/common/utils.js
// META: script=/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
// META: script=resources/helper.js

promise_test(async t => {
const rc = await addWindow();
const waitForPort = expectMessagePortFromWindow(window);
await createMessageChannelAndSendPort(rc);
const closeEventPromise = createCloseEventPromise(await waitForPort);
rc.navigateToNew();
await closeEventPromise;
}, 'The context is navigated to a new document and a close event is fired.')

promise_test(async t => {
const rc = await addWindow();
const waitForPort = expectMessagePortFromWindow(window);
await createMessageChannelAndSendPort(rc);
const closeEventPromise = createCloseEventPromise(await waitForPort);
rc.executeScript(() => window.close());
await closeEventPromise;
}, 'The window is closed and a close event is fired.')

promise_test(async t => {
let iframe;
const waitForLoad = new Promise(resolve => {
iframe = document.createElement('iframe');
iframe.onload = resolve;
document.documentElement.appendChild(iframe);
});
await waitForLoad;

const waitForPort = expectMessagePortFromWindow(iframe.contentWindow);
const {port1, port2} = new MessageChannel();
port1.start();
iframe.contentWindow.postMessage('', '*', [port2]);
await waitForPort;
const closeEventPromise = createCloseEventPromise(port1);
iframe.remove();
await closeEventPromise;
}, 'The iframe is deleted and a close event is fired.')
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// META: title=Close event test when an entangled port is explicitly closed.
// META: script=/common/dispatcher/dispatcher.js
// META: script=/common/get-host-info.sub.js
// META: script=/common/utils.js
// META: script=/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
// META: script=resources/helper.js

async_test(t => {
const channel = new MessageChannel();
channel.port1.start();
channel.port2.start();
channel.port2.onclose = t.step_func_done();
channel.port1.close();
}, 'Close event on port2 is fired when port1 is explicitly closed');

async_test(t => {
const channel = new MessageChannel();
channel.port1.start();
channel.port2.start();
channel.port1.onclose =
t.unreached_func('Should not fire a close event on port1');
channel.port1.close();
t.step_timeout(t.step_func_done(), 1000);
}, 'Close event on port1 is not fired when port1 is explicitly closed');

promise_test(async t => {
const rc = await addWindow();
const waitForPort = expectMessagePortFromWindow(window);
await createMessageChannelAndSendPort(rc);
const closeEventPromise = createCloseEventPromise(await waitForPort);
rc.executeScript(() => {
window.closePort();
});
await closeEventPromise;
}, 'Close event on port2 is fired when port1, which is in a different window, is explicitly closed.')
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// META: timeout=long
// META: title=Close event test when an entangled port is GCed.
// META: script=/common/gc.js

/**
* Create a new MessageChannel and return port1 and a weak reference to port2.
* It is expected that port2 will be garbage collected and a close event
* will be fired on port1.
*
* @returns {Array.<[MessagePort, WeakRef<MessagePort>]>}
*/
function createMessageChannelAndWeakReferToPort() {
const {port1, port2} = new MessageChannel();
port1.start();
return [port1, new WeakRef(port2)];
}

promise_test(async t => {
const [port1, weakport2] = createMessageChannelAndWeakReferToPort();
const closeEventPromise = new Promise(resolve => port1.onclose = resolve);
garbageCollect();
await closeEventPromise;
assert_equals(weakport2.deref(), undefined, 'port2 should be GCed');
}, 'Entangled port is garbage collected, and the close event is fired.')
62 changes: 62 additions & 0 deletions webmessaging/message-channels/close-event/resources/helper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* Create a new promise that resolves when the window receives
* the MessagePort and starts it.
*
* @param {Window} window - The window to wait for the MessagePort.
* @returns {Promise<MessagePort>} A promise you should await to ensure the
* window
* receives the MessagePort.
*/
function expectMessagePortFromWindow(window) {
return new Promise(resolve => {
window.onmessage = e => {
try {
assert_true(e.ports[0] instanceof window.MessagePort);
e.ports[0].start();
resolve(e.ports[0]);
} catch (e) {
reject(e);
}
};
});
}

/**
* Create a new MessageChannel and transfers one of the ports to
* the window which opened the window with a remote context provided
* as an argument.
*
* @param {RemoteContextWrapper} remoteContextWrapper
*/
async function createMessageChannelAndSendPort(remoteContextWrapper) {
await remoteContextWrapper.executeScript(() => {
const {port1, port2} = new MessageChannel();
port1.start();
window.opener.postMessage({}, '*', [port2]);
window.closePort = () => {
port1.close();
}
});
}

/**
* Creates a window with a remote context.
*
* @returns {Promise<RemoteContextWrapper>}
*/
async function addWindow() {
const helper = new RemoteContextHelper();
return helper.addWindow();
}

/**
* Creates a new promise that resolves when the close event is fired.
*
* @param {MessagePort} port - MessagePort on which the close event will
* be fired.
* @returns {Promise} A promise you should await to ensure the close event
* is dispatched.
*/
function createCloseEventPromise(port) {
return new Promise(resolve => port.onclose = resolve);
}

0 comments on commit 352b7f1

Please sign in to comment.