Skip to content

Commit

Permalink
Reland "[fetch-later] Force sending when BackgroundSync permission is…
Browse files Browse the repository at this point in the history
… off"

This reverts commit 2e63732c1a8b8120630c8678f1443282be5bae59.

Reason for revert: Fixed broken WPT. Rebased on the latest change.

Original change's description:
> Revert "[fetch-later] Force sending when BackgroundSync permission is off"
>
> This reverts commit d32dc3635147aa9cb4638864865aa6f86f42f2f2.
>
> Reason for revert:
> Caused consistent failures for virtual/fetch-later/wpt_internal/fetch/fetch-later/send-on-deactivate-with-background-sync.tentative.https.window.html
>
> https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.15%20Tests/42380/overview
>
> Original change's description:
> > [fetch-later] Force sending when BackgroundSync permission is off
> >
> > The [privacy decision][1] suggests: after user navigates away from
> > a page, any pending requests left in the page should not be sent
> > if [BackgroundSync permission][2] is off (default is on in Chrome).
> >
> > This CL forces sending out deferred requests on entering
> > BackForwardCache if the page's BackgroundSync permission is off.
> >
> > [1]: WICG/pending-beacon#3 (comment)
> > [2]: https://wicg.github.io/background-sync/spec/
> >
> > Bug: 1465781
> > Change-Id: I120fbbc13294c40fcf13162d5d786f97e2131367
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4845750
> > Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
> > Auto-Submit: Ming-Ying Chung <mych@chromium.org>
> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> > Reviewed-by: Adam Rice <ricea@chromium.org>
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1204080}
>
> Bug: 1465781
> Change-Id: I09e342e97d52745610d751eb21ee694ccf24976c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908141
> Reviewed-by: Cammie Smith Barnes <cammie@chromium.org>
> Owners-Override: Cammie Smith Barnes <cammie@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Cammie Smith Barnes <cammie@chromium.org>
> Auto-Submit: Jian Li <jianli@chromium.org>
> Commit-Queue: Jian Li <jianli@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1204260}

Bug: 1465781
Change-Id: I7abf6340a23066c0794e4d6010b5ce5ce64c8943
  • Loading branch information
mingyc authored and chromium-wpt-export-bot committed Oct 27, 2023
1 parent 2cbd047 commit ff8ac7e
Showing 1 changed file with 60 additions and 11 deletions.
71 changes: 60 additions & 11 deletions fetch/fetch-later/send-on-deactivate.tentative.https.window.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@

'use strict';

// NOTE: Due to the restriction of WPT runner, the following tests are all run
// with BackgroundSync off, which is different from some browsers,
// e.g. Chrome, default behavior, as the testing infra does not support enabling
// it.

parallelPromiseTest(async t => {
const uuid = token();
const url = generateSetBeaconURL(uuid);
Expand All @@ -29,15 +34,19 @@ parallelPromiseTest(async t => {
}, [url]);
// Navigates away to let page enter BFCache.
const rc2 = await rc1.navigateToNew();
// Navigate back.
// Navigates back.
await rc2.historyBack();
// Verify that the page was BFCached.
// Verifies the page was BFCached.
assert_true(await rc1.executeScript(() => {
return window.pageshowEvent.persisted;
}));

await expectBeacon(uuid, {count: 0});
}, `fetchLater() does not send on page entering BFCache.`);
// Theoretically, the request should still be pending thus 0 request received.
// However, 1 request is sent, as by default the WPT test runner, e.g.
// content_shell in Chromium, does not enable BackgroundSync permission,
// resulting in forcing request sending on every navigation.
await expectBeacon(uuid, {count: 1});
}, `fetchLater() sends on page entering BFCache if BackgroundSync is off.`);

parallelPromiseTest(async t => {
const uuid = token();
Expand All @@ -47,7 +56,7 @@ parallelPromiseTest(async t => {
const rc1 = await helper.addWindow(
/*config=*/ null, /*options=*/ {features: 'noopener'});

// When the remote is BFCached, creates a fetchLater request w/
// When the remote is put into BFCached, creates a fetchLater request w/
// activateAfter = 0s. It should be sent out immediately.
await rc1.executeScript(url => {
window.addEventListener('pagehide', e => {
Expand All @@ -62,13 +71,14 @@ parallelPromiseTest(async t => {
}, [url]);
// Navigates away to trigger request sending.
const rc2 = await rc1.navigateToNew();
// Navigate back.
// Navigates back.
await rc2.historyBack();
// Verify that the page was BFCached.
// Verifies the page was BFCached.
assert_true(await rc1.executeScript(() => {
return window.pageshowEvent.persisted;
}));

// NOTE: In this case, it does not matter if BackgroundSync is on or off.
await expectBeacon(uuid, {count: 1});
}, `Call fetchLater() when BFCached with activateAfter=0 sends immediately.`);

Expand All @@ -92,13 +102,14 @@ parallelPromiseTest(async t => {
}, [url]);
// Navigates away to trigger request sending.
const rc2 = await rc1.navigateToNew();
// Navigate back.
// Navigates back.
await rc2.historyBack();
// Verify that the page was NOT BFCached.
// Verifies the page was NOT BFCached.
assert_equals(undefined, await rc1.executeScript(() => {
return window.pageshowEvent;
}));

// NOTE: In this case, it does not matter if BackgroundSync is on or off.
await expectBeacon(uuid, {count: 1});
}, `fetchLater() sends on navigating away a page w/o BFCache.`);

Expand All @@ -125,12 +136,50 @@ parallelPromiseTest(async t => {
}, [url]);
// Navigates away to trigger request sending.
const rc2 = await rc1.navigateToNew();
// Navigate back.
// Navigates back.
await rc2.historyBack();
// Verify that the page was NOT BFCached.
// Verifies the page was NOT BFCached.
assert_equals(undefined, await rc1.executeScript(() => {
return window.pageshowEvent;
}));

// NOTE: In this case, it does not matter if BackgroundSync is on or off.
await expectBeacon(uuid, {count: 1});
}, `fetchLater() does not send aborted request on navigating away a page w/o BFCache.`);

parallelPromiseTest(async t => {
const uuid = token();
const url = generateSetBeaconURL(uuid);
const options = {activateAfter: 60000};
const helper = new RemoteContextHelper();
// Opens a window with noopener so that BFCache will work.
const rc1 = await helper.addWindow(
/*config=*/ null, /*options=*/ {features: 'noopener'});

// Creates a fetchLater request in remote which should only be sent on
// navigating away.
await rc1.executeScript((url) => {
// Sets activateAfter = 1m to indicate the request should NOT be sent out
// immediately.
fetchLater(url, {activateAfter: 60000});
// Adds a pageshow listener to stash the BFCache event.
window.addEventListener('pageshow', e => {
window.pageshowEvent = e;
});
}, [url]);
// Navigates away to trigger request sending.
const rc2 = await rc1.navigateToNew();
// Navigates back.
await rc2.historyBack();
// Verifies the page was BFCached.
assert_true(await rc1.executeScript(() => {
return window.pageshowEvent.persisted;
}));

// Theoretically, the request should still be pending thus 0 request received.
// However, 1 request is sent, as by default the WPT test runner, e.g.
// content_shell in Chromium, does not enable BackgroundSync permission,
// resulting in forcing request sending on every navigation, even if page is
// put into BFCache.
await expectBeacon(uuid, {count: 1});
}, `fetchLater() with activateAfter=1m sends on page entering BFCache if BackgroundSync is off.`);

0 comments on commit ff8ac7e

Please sign in to comment.