Netmonitor no longer shows fetch requests from workers
Categories
(Core :: DOM: Workers, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr102 | --- | unaffected |
firefox110 | --- | unaffected |
firefox111 | --- | unaffected |
firefox112 | --- | wontfix |
firefox113 | --- | wontfix |
firefox114 | --- | fixed |
People
(Reporter: jdescottes, Assigned: edenchuang)
References
(Regression)
Details
(Keywords: regression)
Attachments
(6 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
The test browser_net_worker_stacks.js was disabled on Nightly when enabling the preference dom.workers.pFetch.enabled.
This test checks if we show fetch requests issued from workers, so we should definitely fix it and restore it.
Comment 1•9 months ago
|
||
Set release status flags based on info from the regressing bug 1812039
:edenchuang, since you are the author of the regressor, bug 1812039, could you take a look?
For more information, please visit auto_nag documentation.
Reporter | ||
Comment 2•9 months ago
|
||
There's already a needinfo at https://bugzilla.mozilla.org/show_bug.cgi?id=1819307
Reporter | ||
Comment 3•9 months ago
|
||
Repurposing to fix the regression.
:edenchuang shared that the cause of the regression is that the channel is now created in the parent process instead. Copying my answer from phabricator at https://phabricator.services.mozilla.com/D171449#5640782
we already monitor channels from the parent process, it's just that we can't link this channel to the tab we are debugging.
Looking quickly we fail to match around https://searchfox.org/mozilla-central/rev/f7edb0b474a1a922f3285107620e802c6e19914d/devtools/shared/network-observer/NetworkUtils.sys.mjs#358-371
There are two things we try to use to match the channel, first channel.loadInfo.browsingContext (not set for this channel) and then the loadContext.Is there a way we can set channel.loadInfo.browsingContexthere ? Or allow to get the loadContext from the channel via loadGroup.notificationCallbacks.getInterface(Ci.nsILoadContext);?
If none of those work, then what could be a good way to match this channel to a given BrowsingContext?
Assignee | ||
Updated•9 months ago
|
Comment 4•9 months ago
|
||
:edenchuang as a reminder that code freeze starts tomorrow... is there a plan to fix this before Fx112 goes to beta?
Comment 5•9 months ago
|
||
(In reply to Dianna Smith [:diannaS] from comment #4)
:edenchuang as a reminder that code freeze starts tomorrow... is there a plan to fix this before Fx112 goes to beta?
:edenchuang has a WIP patch, but it will target 113, and if everything goes fine, uplift it to beta 112.
Comment 6•9 months ago
|
||
Set release status flags based on info from the regressing bug 1812039
Comment 7•9 months ago
|
||
just checking in for an update on this patch, is the plan still to uplift this to 112 beta?
Assignee | ||
Comment 9•8 months ago
|
||
I am testing the local patch with some try runs to make sure it would not make any side-effect.
Will request a review in these two days.
I think it still targets 113. However, if it goes fine, I think we can still uplift it to 112 beta.
Updated•8 months ago
|
Assignee | ||
Comment 10•8 months ago
|
||
Updated•8 months ago
|
Assignee | ||
Comment 11•8 months ago
|
||
After PFetch is enabled, fetch() call in workers will not create a channel in the content process anymore.
Although netmonitor also watches the channels in the parent process, the created channel still loses the BrowsingContext information for netmonitor to connect the network event and the channel.
In P1, https://phabricator.services.mozilla.com/D174249, we propagate the BrowsingContext ID through PFetch.
In this patch, we need to save it in channel's LoadInfo for netmonitor.
In order not to confuse with nsILoadInfo's BrowsingContextID, we create a new attribute WorkerAssociatedBrowsingContextID in nsILoadInfo.
Depends on D174249
Assignee | ||
Comment 12•8 months ago
|
||
After PFetch is enabled, fetch() call in workers will not create a channel in the content process anymore.
Although netmonitor also watches the channels in the parent process, the created channel still loses the BrowsingContext information for netmonitor to connect the network event and the channel.
Depends on D174441
Assignee | ||
Comment 13•8 months ago
|
||
After PFetch is enabled, fetch() call in workers will not create a channel in the content process anymore.
Although netmonitor watches channels and NetEvents, stack traces are only caught in the content process.
That means PFetch should notify the netmonitor about the stack trace of the fetch at the proper moment.
In original fetch steps, FetchDriver would notify the netmonitor the fetch stack trace at
https://searchfox.org/mozilla-central/rev/cdddec7fd690700efa4d6b48532cf70155e0386b/dom/fetch/FetchDriver.cpp#834
When PFetch is enabled, PFetch needs also to propagate this notification back to the content process.
Depends on D174442
Assignee | ||
Comment 14•8 months ago
|
||
Depends on D174443
Updated•7 months ago
|
Updated•7 months ago
|
Reporter | ||
Comment 18•7 months ago
|
||
For info, we are getting reports of users missing requests in devtools, it would be great to land this soon (or to uplift in 114 after the freeze)
Comment 19•7 months ago
|
||
Pushed by echuang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d73ec5415a4 P1 Propagate worker's assocaited BrowsingContextID through PFetch to the parent process. r=asuth https://hg.mozilla.org/integration/autoland/rev/81e926cf92dc P2 Save worker's associated BrowsingContextID in channel's LoadInfo for the netmonitor. r=necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/c8534cf58f86 P3 using nsILoadInfo.WorkerAssociatedBrowsingContext for netmonitor. r=jdescottes,devtools-reviewers https://hg.mozilla.org/integration/autoland/rev/addc41903c2f P4 Propagte stack trace notification back to the content process for PFetch. r=asuth https://hg.mozilla.org/integration/autoland/rev/e73379145f9a P5 Let netmonitor to watch stack trace events on nsIWorkerChannleInfo. r=jdescottes,devtools-reviewers
Comment 20•7 months ago
|
||
Backed out for causing dt failures on browser_net_worker_stacks.js.
[task 2023-05-04T04:06:18.146Z] 04:06:18 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The requestItem exists -
[task 2023-05-04T04:06:18.147Z] 04:06:18 INFO - Visible index of item: 5
[task 2023-05-04T04:06:18.148Z] 04:06:18 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The attached method is correct. -
[task 2023-05-04T04:06:18.148Z] 04:06:18 INFO - Buffered messages finished
[task 2023-05-04T04:06:18.150Z] 04:06:18 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The attached url is correct. - Got "https://example.com/browser/devtools/client/netmonitor/test/missing.txt", expected "https://example.com/browser/devtools/client/netmonitor/test/missing.json"
[task 2023-05-04T04:06:18.150Z] 04:06:18 INFO - Stack trace:
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:test_is:1612
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:verifyRequestItemTarget:596
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:validateRequests/<:1187
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:validateRequests:1183
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_worker_stacks.js:null:116
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:handleTask:1133
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1205
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1347
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1122
[task 2023-05-04T04:06:18.151Z] 04:06:18 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1056
[task 2023-05-04T04:06:18.152Z] 04:06:18 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The displayed method is correct. -
[task 2023-05-04T04:06:18.153Z] 04:06:18 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The displayed file is correct. -
[task 2023-05-04T04:06:18.154Z] 04:06:18 INFO - Not taking screenshot here: see the one that was previously logged
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The tooltip file is correct. - Got "https://example.com/browser/devtools/client/netmonitor/test/missing.txt", expected "https://example.com/browser/devtools/client/netmonitor/test/missing.json"
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - Stack trace:
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:test_is:1612
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:verifyRequestItemTarget:625
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:validateRequests/<:1187
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/head.js:validateRequests:1183
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_worker_stacks.js:null:116
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:handleTask:1133
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1205
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1347
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1122
[task 2023-05-04T04:06:18.156Z] 04:06:18 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1056
[task 2023-05-04T04:06:18.157Z] 04:06:18 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_worker_stacks.js | The displayed protocol is correct. -
Updated•7 months ago
|
Reporter | ||
Comment 21•7 months ago
|
||
This might be one of the tests where the netmonitor assumes a request order which is not necessarily guaranteed. I don't think this really highlights an implementation issue. I can help to update this test so that it no longer expects the missing.txt request to be listed before the missing.json request.
Comment 22•7 months ago
|
||
Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/335ef5d9bbb7
Reporter | ||
Comment 23•7 months ago
|
||
The following patch should avoid the intermittent: https://hg.mozilla.org/try/rev/197aa6c5cb20b4b750443e872cd9a2a4f6d86d18
Feel free to fold that in, I can ping another devtools peer for review.
Hubert: can you take a look at this patch, I think it's fine to relax the expectation on request order here?
Comment 24•7 months ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #23)
The following patch should avoid the intermittent: https://hg.mozilla.org/try/rev/197aa6c5cb20b4b750443e872cd9a2a4f6d86d18
Feel free to fold that in, I can ping another devtools peer for review.
Hubert: can you take a look at this patch, I think it's fine to relax the expectation on request order here?
Yes that sounds fine. The patch LGTM!
Assignee | ||
Comment 25•7 months ago
|
||
Depends on D174444
Comment 26•7 months ago
|
||
Pushed by echuang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/674cfac561b6 P1 Propagate worker's assocaited BrowsingContextID through PFetch to the parent process. r=asuth https://hg.mozilla.org/integration/autoland/rev/6a5e5ebc576c P2 Save worker's associated BrowsingContextID in channel's LoadInfo for the netmonitor. r=necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/5d99cbb6461a P3 using nsILoadInfo.WorkerAssociatedBrowsingContext for netmonitor. r=jdescottes,devtools-reviewers https://hg.mozilla.org/integration/autoland/rev/c6fa624678e8 P4 Propagte stack trace notification back to the content process for PFetch. r=asuth https://hg.mozilla.org/integration/autoland/rev/33a18bb015ff P5 Let netmonitor to watch stack trace events on nsIWorkerChannleInfo. r=jdescottes,devtools-reviewers https://hg.mozilla.org/integration/autoland/rev/6307e9e20326 P6 Allow a different request order in netmonitor tests. r=jdescottes,devtools-reviewers
Comment 27•7 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/674cfac561b6
https://hg.mozilla.org/mozilla-central/rev/6a5e5ebc576c
https://hg.mozilla.org/mozilla-central/rev/5d99cbb6461a
https://hg.mozilla.org/mozilla-central/rev/c6fa624678e8
https://hg.mozilla.org/mozilla-central/rev/33a18bb015ff
https://hg.mozilla.org/mozilla-central/rev/6307e9e20326
Assignee | ||
Updated•6 months ago
|
Description
•