Create DocumentChannel directly in the parent process for that initiate there
Categories
(Core :: Networking, task, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: mattwoodrow, Assigned: mattwoodrow)
References
(Blocks 3 open bugs)
Details
(Whiteboard: [necko-triaged])
Attachments
(20 files, 3 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
We want to be able to create the parent-side of DocumentChannel (DocumentLoadListener) directly in the parent process when we initiate a load from there, and only connect to a content process once we've received a response.
We don't want to initiate front-end loads in the old content process, since this leaks the new URI to a potentially cross-origin content process.
We also don't want to perform a process switch before initiating the load in that process since redirects might mean this switch was to the wrong process. This is also broken if the new load is handled as a download, since we need to retain the old content in that case.
Bug 1601779 is proposing to switch our current behaviour from the second to the first (getting correct behaviour, but adding a low-impact data leak). This bug tracks doing the work for the best fix.
We also want this behaviour for session history, where the state will live in the parent, and we want to initiate loads to a node in the tree without notifying the wrong content process.
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
We need a few things for this:
We need all the parameters that the content process would normally pass up to the parent for a URL load. I started writing a list of those, and how we could get most of them - https://docs.google.com/document/d/1u5GVULrkG0NDoiSi5mDGop6yIBCbmeEuO6VGN3_5s58
We need to synthesize any nsIWebProgressListener notifications that would normally come from the docshell and forwarded to the parent.
We also likely want to ensure that we pre-load a content process so that it can be ready at the same time as the load starts returning data.
Assignee | ||
Comment 3•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
Same process origin changes are handled by the docshell, which detects this during AsyncOnChannelRedirect and clears the mixed content permission.
Process switches load in a fresh docshell, so we need to make sure we appropriately set or clear the mixed content permission.
Depends on D67094
Assignee | ||
Comment 6•5 years ago
|
||
This was previously true, since we only used ResumeRedirectedLoad with a brand new docshell. This bug adds code for using it with existing docshells, which can have any Document (and associated timing object) loaded in them.
Depends on D67096
Assignee | ||
Comment 9•5 years ago
|
||
When we initiate URL bar loads from the parent, loads that are handled externally won't fire a load event from the content process docshell, so we should use the progress listener instead.
Depends on D67099
Comment 22•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc9da03cb3a3 Associate a current DocumentLoadListener with CanonicalBrowsingContext. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/8015780587cd Initialize mixed content channel for process-switches. r=ckerschb,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/78930a97c2df Defer nsIRemoteWindowContext load requests to avoid re-entrancy. r=nika,farre,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/8f7360d827dc Remove incorrect assertion. r=jya https://hg.mozilla.org/integration/autoland/rev/57d568114c5a Change search engine tests to listen for, and block loads in the parent. r=kmag https://hg.mozilla.org/integration/autoland/rev/b5e2aa4741b2 Make sure we wait for the right load to finish. r=kmag https://hg.mozilla.org/integration/autoland/rev/0fc27c2e09d9 Make download tests wait for the STOP progress event instead of load. r=kmag https://hg.mozilla.org/integration/autoland/rev/0fc27502503a Remove nsILoadContext from DocumentLoadListener. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/7c1eeb299b78 Fix unified build issues. r=jya,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/94142944fb54 Make sure rv is initialized correctly in all branches of OpenInitializedChannel. r=nika https://hg.mozilla.org/integration/autoland/rev/cbc308486b17 Fix race in browser_ext_tabs_onUpdated by installing the listener before adding the new tab. r=zombie https://hg.mozilla.org/integration/autoland/rev/6d0783cd5e01 Make xpihandler tests wait for load before starting the test. r=kmag https://hg.mozilla.org/integration/autoland/rev/b298015ee960 Make some browser tests wait on the test uri to load instead of any uri. r=kmag https://hg.mozilla.org/integration/autoland/rev/79b540f8619d Allow less a smaller number of pref checks. r=mconley https://hg.mozilla.org/integration/autoland/rev/184da6309f0c Make RemoteWebProgress:stop also stop any current loads in the parent process. r=nika https://hg.mozilla.org/integration/autoland/rev/e496ab6c0857 Expose some new functionality on the ADocumentChannelBridge/DocumentLoadListener boundary. r=jya,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/a7091729d8c9 Start loads directly from CanonicalBrowsingContext when possible. r=nika,jya,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/7240b27afe28 Expose LoadContext on BrowsingContext webidl. r=nika,kmag https://hg.mozilla.org/integration/autoland/rev/4b3345b2e33b Disable parent-initiated loads when using devtools. r=nika,ochameau
Comment 23•4 years ago
|
||
Backed out 19 changesets (Bug 1602318) for causing multiple failures
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=4b3345b2e33b0e3927534f5c857b0c3086ed731b&selectedJob=298532363
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298532363&repo=autoland&lineNumber=2440
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298532440&repo=autoland&lineNumber=20328
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298541592&repo=autoland&lineNumber=3971
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298532675&repo=autoland&lineNumber=43824
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298532669&repo=autoland&lineNumber=15075
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298544949&repo=autoland&lineNumber=1700
Backout: https://hg.mozilla.org/integration/autoland/rev/61a2d0d369c8d01189408383cc8f9477732a3354
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 28•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a3b056ec6be3 Associate a current DocumentLoadListener with CanonicalBrowsingContext. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/7d1c0d968a09 Initialize mixed content channel for process-switches. r=ckerschb,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/a1b9429b3298 Defer nsIRemoteWindowContext load requests to avoid re-entrancy. r=nika,farre,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/983e5a9abe02 Remove incorrect assertion. r=jya https://hg.mozilla.org/integration/autoland/rev/1756c7584491 Change search engine tests to listen for, and block loads in the parent. r=kmag https://hg.mozilla.org/integration/autoland/rev/db2832973382 Make sure we wait for the right load to finish. r=kmag https://hg.mozilla.org/integration/autoland/rev/8f27319f2c34 Make download tests wait for the STOP progress event instead of load. r=kmag https://hg.mozilla.org/integration/autoland/rev/21ad350f9617 Remove nsILoadContext from DocumentLoadListener. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/028bd63e710d Fix unified build issues. r=jya,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/fc4efd11e643 Make sure rv is initialized correctly in all branches of OpenInitializedChannel. r=nika https://hg.mozilla.org/integration/autoland/rev/5d0fc0102a7f Fix race in browser_ext_tabs_onUpdated by installing the listener before adding the new tab. r=zombie https://hg.mozilla.org/integration/autoland/rev/6c24ba022911 Make xpihandler tests wait for load before starting the test. r=kmag https://hg.mozilla.org/integration/autoland/rev/13bec3079540 Make some browser tests wait on the test uri to load instead of any uri. r=kmag https://hg.mozilla.org/integration/autoland/rev/f5742e84912b Allow less a smaller number of pref checks. r=mconley https://hg.mozilla.org/integration/autoland/rev/5de6321939f2 Expose LoadContext on BrowsingContext webidl. r=nika,kmag,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/088ea9d20617 Add load identifier and copy-ctor to nsDocShellLoadState. r=nika https://hg.mozilla.org/integration/autoland/rev/5f341ebd8591 Simplify DocumentChannelParent construction so that everything happens in Init. r=jya,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/44e82f4339a1 Initiate document loads in the parent process in parallel with setting up the content process side. r=nika,jya https://hg.mozilla.org/integration/autoland/rev/0e0bdebf223b Disable parent-initiated loads when using devtools. r=nika,ochameau
Comment 29•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f71e3eff7a8c Enable parent initiated loads pref. r=jya
Comment 30•4 years ago
|
||
Backout by csabou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ab38fb8e945d Backed out 20 changesets for causing multiple types of failures. CLOSED TREE
Comment 31•4 years ago
|
||
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=0e0bdebf223bc33c71c83ed0321c66c12fd367fb&selectedJob=299184777
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=299184777&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=299184775&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=299184976&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/ab38fb8e945d
Comment 32•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0b56956c6242 Associate a current DocumentLoadListener with CanonicalBrowsingContext. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/f64350664fbf Initialize mixed content channel for process-switches. r=ckerschb,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/faa49f460dd5 Defer nsIRemoteWindowContext load requests to avoid re-entrancy. r=nika,farre,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/ebba2085a6b5 Remove incorrect assertion. r=jya https://hg.mozilla.org/integration/autoland/rev/e571f05c40db Change search engine tests to listen for, and block loads in the parent. r=kmag https://hg.mozilla.org/integration/autoland/rev/d4dd2b03884d Make sure we wait for the right load to finish. r=kmag https://hg.mozilla.org/integration/autoland/rev/7e2902385a6e Make download tests wait for the STOP progress event instead of load. r=kmag https://hg.mozilla.org/integration/autoland/rev/bd6752b9c333 Remove nsILoadContext from DocumentLoadListener. r=nika,necko-reviewers,dragana https://hg.mozilla.org/integration/autoland/rev/54aff65a2d4f Fix unified build issues. r=jya,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/18451f8ef984 Make sure rv is initialized correctly in all branches of OpenInitializedChannel. r=nika https://hg.mozilla.org/integration/autoland/rev/bf51916fa039 Fix race in browser_ext_tabs_onUpdated by installing the listener before adding the new tab. r=zombie https://hg.mozilla.org/integration/autoland/rev/34931634acb7 Make xpihandler tests wait for load before starting the test. r=kmag https://hg.mozilla.org/integration/autoland/rev/ab3a2afede33 Make some browser tests wait on the test uri to load instead of any uri. r=kmag https://hg.mozilla.org/integration/autoland/rev/77aaa5f1a81b Allow less a smaller number of pref checks. r=mconley https://hg.mozilla.org/integration/autoland/rev/cc172a5fcbc9 Expose LoadContext on BrowsingContext webidl. r=nika,kmag,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/cf6bf750f7aa Add load identifier and copy-ctor to nsDocShellLoadState. r=nika https://hg.mozilla.org/integration/autoland/rev/6d02fda2ffc5 Simplify DocumentChannelParent construction so that everything happens in Init. r=jya,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/beabc71edde2 Initiate document loads in the parent process in parallel with setting up the content process side. r=nika,jya https://hg.mozilla.org/integration/autoland/rev/eda5ef391b25 Disable parent-initiated loads when using devtools. r=nika,ochameau
Comment 33•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d61dbc091c36 Enable parent initiated loads pref. r=jya
Comment 34•4 years ago
|
||
Backed out changeset d61dbc091c36 (Bug 1602318) for geckoview failures.
https://hg.mozilla.org/integration/autoland/rev/21659f178a1285e10dec0579230e52eac00d05ae
Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299448580&repo=autoland&lineNumber=6935
Comment 35•4 years ago
|
||
There are also bc asan perma failures like:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299448604&repo=autoland&lineNumber=2996
Comment 36•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0b56956c6242
https://hg.mozilla.org/mozilla-central/rev/f64350664fbf
https://hg.mozilla.org/mozilla-central/rev/faa49f460dd5
https://hg.mozilla.org/mozilla-central/rev/ebba2085a6b5
https://hg.mozilla.org/mozilla-central/rev/e571f05c40db
https://hg.mozilla.org/mozilla-central/rev/d4dd2b03884d
https://hg.mozilla.org/mozilla-central/rev/7e2902385a6e
https://hg.mozilla.org/mozilla-central/rev/bd6752b9c333
https://hg.mozilla.org/mozilla-central/rev/54aff65a2d4f
https://hg.mozilla.org/mozilla-central/rev/18451f8ef984
https://hg.mozilla.org/mozilla-central/rev/bf51916fa039
https://hg.mozilla.org/mozilla-central/rev/34931634acb7
https://hg.mozilla.org/mozilla-central/rev/ab3a2afede33
https://hg.mozilla.org/mozilla-central/rev/77aaa5f1a81b
https://hg.mozilla.org/mozilla-central/rev/cc172a5fcbc9
https://hg.mozilla.org/mozilla-central/rev/cf6bf750f7aa
https://hg.mozilla.org/mozilla-central/rev/6d02fda2ffc5
https://hg.mozilla.org/mozilla-central/rev/beabc71edde2
https://hg.mozilla.org/mozilla-central/rev/eda5ef391b25
Assignee | ||
Comment 37•4 years ago
|
||
The pref flip patch got backed out, so reopening until that sticks.
Updated•4 years ago
|
Comment 38•4 years ago
|
||
Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2f45a8fc88cb Enable parent initiated loads pref. r=jya
Comment 39•4 years ago
|
||
bugherder |
Comment 40•4 years ago
|
||
== Change summary for alert #25779 (as of Wed, 29 Apr 2020 18:34:13 GMT) ==
Regressions:
44% raptor-tp6-twitch-firefox-cold fcp linux64-shippable-qr opt 68.33 -> 98.58
36% raptor-tp6-twitch-firefox-cold fcp linux64-shippable-qr opt 72.88 -> 98.75
Improvements:
7% raptor-tp6-instagram-firefox-cold fcp linux64-shippable opt 205.67 -> 192.25
6% raptor-tp6-outlook-firefox-cold fcp linux64-shippable opt 216.33 -> 202.92
6% raptor-tp6-bing-firefox-cold fcp windows10-64-shippable-qr opt 245.58 -> 231.08
6% raptor-tp6-outlook-firefox-cold fcp linux64-shippable opt 215.88 -> 203.25
5% raptor-tp6-reddit-firefox-cold fcp linux64-shippable opt 270.50 -> 255.75
5% raptor-tp6-instagram-firefox-cold fcp windows10-64-shippable-qr opt 205.00 -> 193.92
5% raptor-tp6-outlook-firefox-cold linux64-shippable opt 289.72 -> 274.66
5% raptor-tp6-bing-firefox-cold fcp linux64-shippable opt 248.42 -> 235.92
5% raptor-tp6-bing-firefox-cold linux64-shippable opt 252.92 -> 240.51
5% raptor-tp6-yahoo-mail-firefox-cold fcp linux64-shippable opt 282.67 -> 269.25
4% raptor-tp6-instagram-firefox-cold linux64-shippable opt 387.82 -> 370.73
4% raptor-tp6-instagram-firefox-cold fcp windows10-64-shippable opt 202.62 -> 194.08
4% raptor-tp6-bing-firefox-cold windows10-64-shippable-qr opt 237.41 -> 227.86
4% raptor-tp6-google-firefox-cold fcp linux64-shippable opt 308.96 -> 296.58
4% raptor-tp6-outlook-firefox-cold linux64-shippable-qr opt 287.59 -> 276.68
4% raptor-tp6-instagram-firefox-cold windows10-64-shippable-qr opt 375.85 -> 362.63
4% raptor-tp6-bing-firefox-cold linux64-shippable-qr opt 255.09 -> 246.18
3% raptor-tp6-reddit-firefox-cold linux64-shippable opt 732.97 -> 708.02
3% raptor-tp6-yahoo-mail-firefox-cold linux64-shippable opt 499.44 -> 482.60
3% raptor-tp6-google-firefox-cold linux64-shippable opt 411.53 -> 397.99
3% raptor-tp6-bing-firefox-cold fcp linux64-shippable-qr opt 261.42 -> 253.33
3% raptor-tp6-yandex-firefox-cold fcp linux64-shippable-qr opt 313.42 -> 304.58
3% raptor-tp6-yahoo-mail-firefox-cold linux64-shippable-qr opt 513.37 -> 500.41
2% raptor-tp6-amazon-firefox-mitm5-cold linux64-shippable opt 818.35 -> 798.34
2% raptor-tp6-bing-firefox-cold loadtime windows10-64-shippable-qr opt 315.29 -> 307.75
2% raptor-tp6-office-firefox-cold fcp windows10-64-shippable-qr opt 400.17 -> 391.50
2% raptor-tp6-office-firefox-cold windows10-64-shippable-qr opt 383.06 -> 374.91
2% raptor-tp6-youtube-firefox-cold linux64-shippable-qr opt 780.02 -> 763.68
2% raptor-tp6-yahoo-mail-firefox-cold fcp windows10-64-shippable-qr opt 320.25 -> 313.75
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25779
Comment 41•4 years ago
|
||
Hey Matt, there are lots of improvements from your patch but we also have two regressions for raptor-tp6-twitch.
Are these expected ?
Thanks!
Comment 42•4 years ago
|
||
I'm not sure I entirely believe the twitch regression. That test has been extremely volatile, and I suspect in this case we're just seeing a slight timing improvement knock it from one mode to another. Especially given that the last significant improvement seemed to come from a test-only change that seemed to affect a lot of raptor tests in unexpected ways.
Assignee | ||
Comment 43•4 years ago
|
||
Yeah, given that it seems to be the exact reverse of an unexplained improvement from a few days ago, then I don't think the regression is super meaningful.
Assignee | ||
Comment 44•4 years ago
|
||
I took a local profile of the twitch regression to try to understand it: https://bit.ly/3d1H4uI (sorry, xul symbolication didn't seem to work).
Looking at that it appears that the main document load of twitch returns, but we spend ~33ms waiting for the main-thread to become free, so that we can send it to the content process (which is roughly similar to the magnitude of the regression).
The main thread is busy in NeckoParent::RecvGetExtensionStream, and then we run a layout/paint cycle for the parent process content.
Looking at other profiles, it appears that have this same work on all pages, but twitch has an http response time of ~50ms, which is really quick and leads to it being ready before the other work completes. My local run of instagram had a response time of 180ms, so the main-thread was idle by the time we needed it.
I think this explains the regression, and why it comes and goes, since it wouldn't take much for the scheduling to work out better.
The extension work appears to be handling calls from raptor's 'postToControlServer' (I suspect the one inside updateTab, followed by the "status" message in nextCycle, but hard to be sure).
Henrik, is this something you've looked at?
I suspect reducing (or delaying) the number of postToControlServer calls during the time we're measuring might lead to better and more consistent results.
Comment 45•4 years ago
|
||
== Change summary for alert #25832 (as of Mon, 04 May 2020 07:14:14 GMT) ==
Improvements:
6% tabpaint linux64-shippable opt e10s stylo 53.41 -> 49.95
6% tabpaint linux64-shippable opt e10s stylo 53.42 -> 50.02
4% tsvg_static linux64-shippable opt e10s stylo 45.58 -> 43.81
3% tsvg_static linux64-shippable-qr opt e10s stylo 43.59 -> 42.20
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25832
Comment 46•4 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #44)
Henrik, is this something you've looked at?
I suspect reducing (or delaying) the number of postToControlServer calls during the time we're measuring might lead to better and more consistent results.
Thanks Matt! This is very useful information and hopefully helps us to get the performance regression on bug 1627621 investigated/solved. See that bug for further updates.
Comment 47•4 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #44)
I took a local profile of the twitch regression to try to understand it: https://bit.ly/3d1H4uI (sorry, xul symbolication didn't seem to work).
(This was probably due to bug 1635139.)
Description
•