1564235 - Expose NavigationPreloadManager interface

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Perry Jiang [:perry] [no longer employee, use ni?]]

This bug isn't concerned with issuing the request while a SW start up. We want as a first step to be able to have a navigation preload flag and query its state. There should also be a pref like dom.serviceWorkers.navigationPreloadManager.enabled (or something similar) if things are to be merged incrementally.

Tests that should pass:
service-workers/service-worker/navigation-preload/navigationPreload.https.html
service-workers/service-worker/navigation-preload/get-state.https.html

Comment 1

[Jan Varga [:janv]]

I guess Sharanya is actively working on this, so P2.

Comment 2

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1564235 - implement NavigationPreloadManager, no fetch integration r?asuth

• Enabled only with dom.serviceWorkers.navigationPreload.enabled=true (enforced
  by the generated bindings) and dom.serviceWorkers.parent_intercept=true
  (enforced by a release assertion in NavigationPreloadManager's constructor).
• Persist the NavigationPreloadState with ServiceWorkerRegistration data.

Comment 3

[Perry Jiang [:perry] [no longer employee, use ni?]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b14d8536792132257e520664a45b60e728c25b7

Comment 4

[Jens Stutte [:jstutte]]

Hi :perry, can we assign this patch to dom-workers-and-storage-reviewers to move it forward more quickly?

Comment 5

[Perry Jiang [:perry] [no longer employee, use ni?]]

We could, but I think asuth would be the only person comfortable reviewing it. The patch is also only a subset of the NavigationPreloadManager API and without the other parts of the API, the patch here wouldn't add any real functionality, so I wouldn't say there's a rush to land it all ASAP (other than avoiding merge conflicts due to the number of files touched).

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Yeah, to be clear, I've been deferring review intentionally pending the parent-intercept stuff. But the bit-rot concern is a good point; I hadn't been expecting to be deferring it this long. Thankfully I think most of my concerns as it relates to that have been addressed.

Probably the right next step is to send out an intent to prototype for this (https://wiki.mozilla.org/ExposureGuidelines#Intent_to_prototype) and then I can review pending the feedback on that. Needinfo on perry for intent to prototype.

Comment 7

[Perry Jiang [:perry] [no longer employee, use ni?]]

Sent an email to the mailing list with subject "Intent to prototype: NavigationPreloadManager API". It's awaiting "moderator approval" so may not be delivered immediately.

Comment 8

[James Graham [:jgraham]]

What's the status here? We have some wpt tests in https://bugzilla.mozilla.org/show_bug.cgi?id=1636178 which are currently timing out because we're missing NavigationPreloadManager.

Comment 9

[Jens Stutte [:jstutte]]

Perry, please see last comment from James. Thanks!

Comment 10

[Perry Jiang [:perry] [no longer employee, use ni?]]

NavigationPreloadManager isn't high up on the priority list right now, but that may change in H2. So as of now I don't think we will be able to address the timing-out tests. This particular bug/patch is only a portion of the feature anyways; landing this bug's patch would only address two of the WPTs in the navigation-preload directory.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

(Our Phabricator integration likes to maintain invariants about the state of the bug when there are patches, but Perry is no longer working on this and it would be premature to commandeer the revision.)

Comment 12

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - implement NavigationPreloadManager, no fetch integration r?asuth

Comment 13

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P1 Pref for serviceworker.navigationPreload API. r=#dom-worker-reviewers

Comment 14

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P2 Create NavigationPreloadManager.webidl and implement webidl binding. r=#dom-worker-reviewers

Depends on D116913

Comment 15

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P3 Create IPC for NavigationPreload in PServiceWorkerRegistration.ipdl. r=#dom-worker-reviewers

Depends on D116914

Comment 16

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P4 Save NavigationPreload data in ServiceWorkerRegistration Info. r=#dom-worker-reviewers

Depends on D116916

Comment 17

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P5 Save NavigationPreload data into serviceworker.txt. r=#dom-worker-reviewers

Depends on D116917

Comment 18

[Eden Chuang[:edenchuang]]

Attachment: Bug 1564235 - P6 Enable partial web-platform tests for navigationPreloadManager. r=#dom-worker-reviewers

Depends on D116918

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

Link to the intent to prototype that was previously sent by Perry on Dec 6th, 2019: https://groups.google.com/g/mozilla.dev.platform/c/wZwIB-QFugs/m/mLPLKbfhBAAJ

Comment 20

[Andrew Sutherland [:asuth] (he/him)]

Many thanks to Perry for preparing the original patch for this and Eden for updating and excellently splitting up the patch into more bite size pieces; it really does simplify the review process immeasurably!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a4cd096b0ba7
P1 Pref for serviceworker.navigationPreload API. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/789ce1a19e9f
P2 Create NavigationPreloadManager.webidl and implement webidl binding. r=dom-worker-reviewers,webidl,asuth
https://hg.mozilla.org/integration/autoland/rev/679bed36bc0b
P3 Create IPC for NavigationPreload in PServiceWorkerRegistration.ipdl. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/b1503c48e2f6
P4 Save NavigationPreload data in ServiceWorkerRegistration Info. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/198a29232047
P5 Save NavigationPreload data into serviceworker.txt. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/78d268b961e6
P6 Enable partial web-platform tests for navigationPreloadManager. r=dom-worker-reviewers,asuth

Comment 22

[Natalia Csoregi [:nataliaCs]]

Backed out for ServiceWorkerRegistrar failures.

backout: https://hg.mozilla.org/integration/autoland/rev/8c5e8c087f2e9732881214e7d20377da3f9c29d8

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=TmOWUijPSH2-JdOkpljYiA.0&revision=78d268b961e67aa1ecd18d3e3803d1c2b6da44bc

failure log: https://treeherder.mozilla.org/logviewer?job_id=342332437&repo=autoland&lineNumber=31286

[task 2021-06-10T01:50:19.256Z] 01:50:19 INFO - TEST-START | ServiceWorkerRegistrar.TestReadData
[task 2021-06-10T01:50:19.256Z] 01:50:19 WARNING - TEST-UNEXPECTED-FAIL | ServiceWorkerRegistrar.TestReadData | Expected equality of these values:
[task 2021-06-10T01:50:19.257Z] 01:50:19 INFO - NS_OK
[task 2021-06-10T01:50:19.257Z] 01:50:19 INFO - Which is: 4-byte object <00-00 00-00>
[task 2021-06-10T01:50:19.257Z] 01:50:19 INFO - rv
[task 2021-06-10T01:50:19.257Z] 01:50:19 INFO - Which is: 4-byte object <57-00 07-80>
[task 2021-06-10T01:50:19.257Z] 01:50:19 INFO - ReadData() should not fail @ /builds/worker/checkouts/gecko/dom/serviceworkers/test/gtest/TestReadWrite.cpp:197
[task 2021-06-10T01:50:19.257Z] 01:50:19 WARNING - TEST-UNEXPECTED-FAIL | ServiceWorkerRegistrar.TestReadData | test completed (time: 1ms)

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2148ef30186a
P1 Pref for serviceworker.navigationPreload API. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/afae3c50f7ac
P2 Create NavigationPreloadManager.webidl and implement webidl binding. r=dom-worker-reviewers,webidl,asuth
https://hg.mozilla.org/integration/autoland/rev/185488c90cb8
P3 Create IPC for NavigationPreload in PServiceWorkerRegistration.ipdl. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/d321a8a2a098
P4 Save NavigationPreload data in ServiceWorkerRegistration Info. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/62a4f85ced42
P5 Save NavigationPreload data into serviceworker.txt. r=dom-worker-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/fc6124f09abd
P6 Enable partial web-platform tests for navigationPreloadManager. r=dom-worker-reviewers,asuth

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/2148ef30186a
https://hg.mozilla.org/mozilla-central/rev/afae3c50f7ac
https://hg.mozilla.org/mozilla-central/rev/185488c90cb8
https://hg.mozilla.org/mozilla-central/rev/d321a8a2a098
https://hg.mozilla.org/mozilla-central/rev/62a4f85ced42
https://hg.mozilla.org/mozilla-central/rev/fc6124f09abd