1602318 - Create DocumentChannel directly in the parent process for that initiate there

Status: RESOLVED FIXED

(Core :: Networking, task, P2)

Description

[Matt Woodrow (:mattwoodrow)]

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.

Comment 1

[Chris Peterson [:cpeterson]]

This bug is not a Fission MVP blocker.

Comment 2

[Matt Woodrow (:mattwoodrow)]

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.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Associate a current DocumentLoadListener with CanonicalBrowsingContext. r?nika

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Initialize mixed content channel for process-switches. r?nika

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

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Defer nsIRemoteWindowContext load requests to avoid re-entrancy. r?nika

Depends on D67095

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Remove incorrect assertion. r?jya

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

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Change search engine tests to listen for, and block loads in the parent. r?kmag

Depends on D67097

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make sure we wait for the right load to finish. r?kmag

r.js

Depends on D67098

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make download tests wait for the STOP progress event instead of load. r?kmag

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 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Remove nsILoadContext from DocumentLoadListener. r?nika

Depends on D67100

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Fix unified build issues. r?jya

Depends on D70618

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make sure rv is initialized correctly in all branches of OpenInitializedChannel. r?nika

Depends on D70620

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Fix race in browser_ext_tabs_onUpdated by installing the listener before adding the new tab. r?zombie

Depends on D70621

Comment 14

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make xpihandler tests wait for load before starting the test. r?kmag

Depends on D70622

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make some browser tests wait on the test uri to load instead of any uri. r?kmag

Depends on D70623

Comment 16

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Allow less a smaller number of pref checks. r?mconley

Depends on D70624

Comment 17

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Make RemoteWebProgress:stop also stop any current loads in the parent process. r?nika

Depends on D70625

Comment 18

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Expose some new functionality on the ADocumentChannelBridge/DocumentLoadListener boundary. r?jya

Depends on D70626

Comment 19

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Start loads directly from CanonicalBrowsingContext when possible. r?nika,jya

Depends on D70627

Comment 20

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Expose LoadContext on BrowsingContext webidl. r?nika,kmag

Depends on D70629

Comment 21

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Disable parent-initiated loads when using devtools. r?nika,ochameau

Depends on D71135

Comment 22

[Pulsebot]

Pushed by [e-mail]:
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

[Noemi Erli[:noemi_erli]]

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

Comment 24

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Add load identifier and copy-ctor to nsDocShellLoadState. r?nika

Depends on D71135

Comment 25

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Simplify DocumentChannelParent construction so that everything happens in Init. r?jya

Depends on D72110

Comment 26

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Initiate document loads in the parent process in parallel with setting up the content process side. r?nika!,jya!

Depends on D72111

Comment 27

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1602318 - Enable parent initiated loads pref. r?jya

Depends on D71136

Comment 28

[Pulsebot]

Pushed by [e-mail]:
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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f71e3eff7a8c
Enable parent initiated loads pref. r=jya

Comment 30

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab38fb8e945d
Backed out 20 changesets for causing multiple types of failures. CLOSED TREE

Comment 31

[Cosmin Sabou [:CosminS]]

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

[Pulsebot]

Pushed by [e-mail]:
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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d61dbc091c36
Enable parent initiated loads pref. r=jya

Comment 34

[Cristian Brindusan [:cbrindusan]]

Backed out changeset d61dbc091c36 (Bug 1602318) for geckoview failures.
https://hg.mozilla.org/integration/autoland/rev/21659f178a1285e10dec0579230e52eac00d05ae

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=d61dbc091c360277b7dacd037c20d4835c3505e7&selectedJob=299448580

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299448580&repo=autoland&lineNumber=6935

Comment 35

[Cristian Brindusan [:cbrindusan]]

There are also bc asan perma failures like:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299448604&repo=autoland&lineNumber=2996

Comment 36

[Narcis Beleuzu [:NarcisB]]

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

Comment 37

[Matt Woodrow (:mattwoodrow)]

The pref flip patch got backed out, so reopening until that sticks.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f45a8fc88cb
Enable parent initiated loads pref. r=jya

Comment 39

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/2f45a8fc88cb

Comment 40

[Marian Raiciof [:marauder]]

== 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

[Marian Raiciof [:marauder]]

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

[Kris Maglione [:kmag]]

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.

Comment 43

[Matt Woodrow (:mattwoodrow)]

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.

Comment 44

[Matt Woodrow (:mattwoodrow)]

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

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== 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

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(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

[Markus Stange [:mstange]]

(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.)