Closed Bug 1607984 Opened 4 years ago Closed 4 years ago

Use DocumentChannel for docshells in the parent process

Categories

(Core :: Networking, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: mattwoodrow, Assigned: jya)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [necko-triaged])

Attachments

(20 files, 2 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

We currently only create a DocumentChannel for loads that initiate within a content process, but it would be nicer to always create them and unify our codepaths more.

Depends on: 1607987

Matt, this sounds to me as a P3 only, but please feel free to raise the priority.

Priority: -- → P3
Whiteboard: [necko-triaged]
Blocks: 1399574
Depends on: 1616171
No longer depends on: 1616171
Assignee: nobody → jyavenard
Depends on: 1616353

This also removes the need to call SerializeRedirectData to set mRedirectChannelId to the proper value and register the channel.

If we were to open the window to close it immediately; we would leak a nsHtml5Parser object.

Depends on D69999

Regressiong from bug 1607987. This prevented from QueryInterface(Ci.nsIIdentChannel) in JS on the channel.

Depends on D70001

This gets triggered with DocumentChannel is enabled in the parent process. Not sure how it worked in the past.

Depends on D70002

Attached file Bug 1607984 - P8. Fix test. r?ehsan (obsolete) —

The test waited one event loop before testing if the attributes could be changed.
There wasn't any guarantees that the load could have actually started by then being asynchronous in nature.

The use of DocumentChannel introduces some extra hops when starting a load, and we can not longer simply wait for one event loop.
So instead we wait for the nsIWebProgress event that notifies that the docShell has actually started the load.

Depends on D70004

The DocumentLoadListener is setting up a ParentChannelListener to go in between the normal listener->channel chain.
ParentChannelListener not implementing nsIThreadRetargetableStreamListener would prevent a nsHtml5StreamParser settings things up so that OnDataAvailable could be sent to a html parser thread off the main thread; improving performance.

Depends on D70005

The code assumed that nsJARChannel::RetargetDeliveryTo would have been called synchronously from nsJARChannel::OnStartRequest, which would be true if we weren't using a DocumentChannel.

The DocumentLoadListener queue the calls to OnStartRequest until the final redirect.
nsJARChannel::RetargetDelivery mRequest member is be set to forward the call.
So we need to only reset it once OnStopRequest has been received.

Depends on D70006

We must wait for the iframe created by extension1 to be fully loaded before we can attempt to send a message to it.
So we send a message when it is ready, and suspend the execution until this message is received.

Depends on D70007

Add ParentProcessDocumentChannel object. This object is a DocumentChannel that will start a channel load from the parent process via a DocumentChannel.

The aim of this task is two-fold.
1- Be consistent on how we handle redirects before continuing the load on the final channel.
2- Prepare to initiate a process switch when needed without having to go via an intermediary content process, saving a process switch. This task will be done in a follow-up task.

The behaviour of the ParentProcessDocumentChannel is similar in logic to the DocumentChannelChild/DocumentChannelParent pair.
The ParentProcessDocumentChannel sets up a DocumentLoadListener, have it handle the redirects and upon completion continue the load on the final channel.

Depends on D70008

Some tests rely on this event to start action. The DocumentChannel had no equivalent. We make the ParentProcessDocumentChannel listen to this event and if it matches the nsIChannel currently in use in the DocumentLoadListener than we emit a similar document-on-modify-request event on the DocumentChannel.

Depends on D70009

When starting a load via the ParentProcessDocumentChannel, the event http-on-modify-request will be fired before the DocumentLoadListener has a chance to set the notificationCallback attribute.
When using a DocumentChannel, this test will not trigger the expected codepath as the DOMWindowCreated event will be fired once the channel is fully up and running; which in effect is also a fix of the original bug 1339722

Instead we use the document-on-modify-request event when the DocumentChannel is enabled.

Depends on D70010

Attachment #9138823 - Attachment description: Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin → Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. r?valentin
Attachment #9138821 - Attachment is obsolete: true
See Also: → 1629254
Attachment #9138823 - Attachment description: Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. r?valentin → Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin

addA11yLoadEvent gets the contentWindow and wait for the document from that window to fire the load event.

Enabling the DocumentChannel for parent process load (or here in non-e10s mode) we have one extra event loop before the load starts.
So the window passed to addAllyLoadEvent would have been of the about:blank page.

The current code was based on an observable behaviour which was that the load was occuring synchronously.

DocumentChannel broke that assumption.

This file will be removed in bug 1628752 anyway.

Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c499fa55d0f2
P1. Make SerializeRedirectData const. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/e11f4d334dc7
P2. Add Redirects/LastVisitInfo getters. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/1cfc8aad978c
P3. Fix leak in nsDSURIContentListener. r=smaug
https://hg.mozilla.org/integration/autoland/rev/3d6824e92c38
P4. Expose SetClassificationFlagsHelper. r=valentin
https://hg.mozilla.org/integration/autoland/rev/348da0a8dd00
P5. Add missing nsIIdentChannel interface. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/47beda24613f
P6. Fix test. r=MattN
https://hg.mozilla.org/integration/autoland/rev/4c661ed81cf2
P7. Fix compilation failure in non-unified mode. r=valentin
https://hg.mozilla.org/integration/autoland/rev/0fa85c9199a9
P9. Implement nsIThreadRetargetableStreamListener in ParentChannelListerner. r=valentin
https://hg.mozilla.org/integration/autoland/rev/0b71b61415d5
P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin r=valentin
https://hg.mozilla.org/integration/autoland/rev/9f216e9bd32e
P11. Don't assume the page will be loaded synchronously. r=zombie
https://hg.mozilla.org/integration/autoland/rev/a3a28be516f9
P12. Start parent load via DocumentChannel. r=mayhemer,nika,mattwoodrow
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/718d89be09d2
P1. Make SerializeRedirectData const. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/07ce7b11fee9
P2. Add Redirects/LastVisitInfo getters. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/05d2032060db
P3. Fix leak in nsDSURIContentListener. r=smaug
https://hg.mozilla.org/integration/autoland/rev/fe84e5c64b4e
P4. Expose SetClassificationFlagsHelper. r=valentin
https://hg.mozilla.org/integration/autoland/rev/f554d4ce6718
P5. Add missing nsIIdentChannel interface. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/979d99eb12d1
P6. Fix test. r=MattN
https://hg.mozilla.org/integration/autoland/rev/e51f6b7a745c
P7. Fix compilation failure in non-unified mode. r=valentin
https://hg.mozilla.org/integration/autoland/rev/8e7c95d322e8
P9. Implement nsIThreadRetargetableStreamListener in ParentChannelListerner. r=valentin
https://hg.mozilla.org/integration/autoland/rev/c7edd070b2f3
P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin r=valentin
https://hg.mozilla.org/integration/autoland/rev/025af7792f2a
P11. Don't assume the page will be loaded synchronously. r=zombie
https://hg.mozilla.org/integration/autoland/rev/8f9058eb821d
P12. Start parent load via DocumentChannel. r=mayhemer,nika,mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/5f9fe17e46b8
P13. Proxy the first http-on-opening-request event to the DocumentChannel. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/fdacabac2c54
P14. Fix test_bug1339722.html when using PPDC. r=valentin
https://hg.mozilla.org/integration/autoland/rev/71dffa590c10
P15. Wait for the load to start before setting the event handler. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/5497c90b03de
P16. Always whitelist file. r=markh

4 patches didn't get pushed for some reasons.

Flags: needinfo?(jyavenard)
Regressions: 1500977
Regressions: 1534242
No longer regressions: 1500977
No longer regressions: 1534242
Regressions: 1630211
Regressions: 1630209
Regressions: 1630192

the window may be showed before the page has loaded. In which case, setting a listener for the pageshow event would never be triggered as it would be set too late.

We disable it on Android for now due to unexplained reftest start failures.

Blocks: 1632098
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/09a651daf344
P1. Make SerializeRedirectData const. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/dc8b37e10dc7
P2. Add Redirects/LastVisitInfo getters. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/c173bde5106b
P3. Fix leak in nsDSURIContentListener. r=smaug
https://hg.mozilla.org/integration/autoland/rev/7a2ef225a41e
P4. Expose SetClassificationFlagsHelper. r=valentin
https://hg.mozilla.org/integration/autoland/rev/e02b12515d60
P5. Add missing nsIIdentChannel interface. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/abe692da4556
P6. Fix test. r=MattN
https://hg.mozilla.org/integration/autoland/rev/bed3f6bee79e
P7. Fix compilation failure in non-unified mode. r=valentin
https://hg.mozilla.org/integration/autoland/rev/e11b1b0ecfbf
P9. Implement nsIThreadRetargetableStreamListener in ParentChannelListerner. r=valentin
https://hg.mozilla.org/integration/autoland/rev/d7600d4d3528
P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin r=valentin
https://hg.mozilla.org/integration/autoland/rev/107be8f3737d
P11. Don't assume the page will be loaded synchronously. r=zombie
https://hg.mozilla.org/integration/autoland/rev/63175f596762
P12. Start parent load via DocumentChannel. r=mayhemer,nika,mattwoodrow,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/ce527a6ffba4
P13. Proxy the first http-on-opening-request event to the DocumentChannel. r=mayhemer,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/0c0169ed4f04
P14. Fix test_bug1339722.html when using PPDC. r=valentin
https://hg.mozilla.org/integration/autoland/rev/4e5d89f68293
P15. Wait for the load to start before setting the event handler. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/0cb21bedf65f
P16. Always whitelist file. r=markh
https://hg.mozilla.org/integration/autoland/rev/4509808243f5
P17. Put ParentProcessDocumentChannel behind a pref. r=necko-reviewers,mattwoodrow
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9fc1a237829c
P1. Make SerializeRedirectData const. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/244d3cb006be
P2. Add Redirects/LastVisitInfo getters. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/b8c6277207d8
P3. Fix leak in nsDSURIContentListener. r=smaug
https://hg.mozilla.org/integration/autoland/rev/35f22d0817e1
P4. Expose SetClassificationFlagsHelper. r=valentin
https://hg.mozilla.org/integration/autoland/rev/1ab8758802a6
P5. Add missing nsIIdentChannel interface. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/7657023a763b
P6. Fix test. r=MattN
https://hg.mozilla.org/integration/autoland/rev/12b82a39c910
P7. Fix compilation failure in non-unified mode. r=valentin
https://hg.mozilla.org/integration/autoland/rev/980067f3ac1d
P9. Implement nsIThreadRetargetableStreamListener in ParentChannelListerner. r=valentin
https://hg.mozilla.org/integration/autoland/rev/77fda525ee12
P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin r=valentin
https://hg.mozilla.org/integration/autoland/rev/108e2cb6b2a9
P11. Don't assume the page will be loaded synchronously. r=zombie
https://hg.mozilla.org/integration/autoland/rev/142148a95181
P12. Start parent load via DocumentChannel. r=mayhemer,nika,mattwoodrow,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/52566b3564ba
P13. Proxy the first http-on-opening-request event to the DocumentChannel. r=mayhemer,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/9a15a605f91a
P14. Fix test_bug1339722.html when using PPDC. r=valentin
https://hg.mozilla.org/integration/autoland/rev/28af6418ac16
P15. Wait for the load to start before setting the event handler. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/e973911e67e6
P16. Always whitelist file. r=markh
https://hg.mozilla.org/integration/autoland/rev/6c6ffa908c06
P17. Put ParentProcessDocumentChannel behind a pref. r=necko-reviewers,mattwoodrow
See Also: → 1632400
Flags: needinfo?(jyavenard)

Backed out for leaks on browser_ext_webRequest.js

backout: https://hg.mozilla.org/integration/autoland/rev/ed118e721ff6df179e51bba78c90d4e748d64013

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=298974407&group_state=expanded&revision=6c6ffa908c06ddd48314ab276b9602e7822c9a7b&searchStr=browser-chrome

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=298974407&repo=autoland&lineNumber=41599

[task 2020-04-23T07:27:03.519Z] 07:27:03 INFO - GECKO(4768) | [Parent 3192, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/netwerk/base/nsNetUtil.cpp, line 2323
[task 2020-04-23T07:27:03.519Z] 07:27:03 INFO - GECKO(4768) | [Parent 3192, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/caps/ContentPrincipal.cpp, line 398
[task 2020-04-23T07:27:03.519Z] 07:27:03 INFO - GECKO(4768) | [Parent 3192, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /builds/worker/checkouts/gecko/caps/ContentPrincipal.cpp, line 423
[task 2020-04-23T07:27:03.562Z] 07:27:03 INFO - GECKO(4768) | [Parent 3192, Main Thread] WARNING: Extra shutdown CC: 'i < NORMAL_SHUTDOWN_COLLECTIONS', file /builds/worker/checkouts/gecko/xpcom/base/nsCycleCollector.cpp, line 3359
[task 2020-04-23T07:27:03.817Z] 07:27:03 INFO - TEST-INFO | Main app process: exit 0
[task 2020-04-23T07:27:03.817Z] 07:27:03 INFO - TEST-INFO | Confirming we saw 2475 DOCSHELL created and 2472 destroyed log strings.
[task 2020-04-23T07:27:03.817Z] 07:27:03 INFO - TEST-INFO | Confirming we saw 6722 DOMWINDOW created and 6707 destroyed log strings.
[task 2020-04-23T07:27:03.818Z] 07:27:03 ERROR - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_webRequest.js | leaked 2 window(s) until shutdown [url = chrome://global/content/win.xhtml]
[task 2020-04-23T07:27:03.818Z] 07:27:03 ERROR - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_webRequest.js | leaked 1 window(s) until shutdown [url = chrome://browser/content/browser.xhtml]
[task 2020-04-23T07:27:03.818Z] 07:27:03 ERROR - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_webRequest.js | leaked 1 window(s) until shutdown [url = about:blank]
[task 2020-04-23T07:27:03.819Z] 07:27:03 INFO - TEST-INFO | browser/components/extensions/test/browser/browser_ext_webRequest.js | windows(s) leaked: [pid = 3192] [serial = 1178], [pid = 3192] [serial = 1179], [pid = 3192] [serial = 1182], [pid = 3192] [serial = 1185]
[task 2020-04-23T07:27:03.819Z] 07:27:03 INFO - TEST-INFO | browser/components/extensions/test/browser/browser_ext_webRequest.js | This test created 1 hidden window(s)

Flags: needinfo?(jyavenard)

Can't reproduce the leak on a fresh try run based in central.

Seems to have been due to specific and local autoland issue.

Flags: needinfo?(jyavenard)
Attachment #9138823 - Attachment description: Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. ?valentin → Bug 1607984 - P10. Wait until OnStopRequest has been called to clear mRequest. r?valentin

Move DocumentChannelChild methods to the base class as they will be needed with the ParentProcessDocumentChannel

Depends on D70008

This prepares the code so that it doesn't always have to deal with dealing with another process as is the case when using DocumentChannelChild/DocumentChannelParent

Depends on D72271

Attachment #9138825 - Attachment description: Bug 1607984 - P12. Start parent load via DocumentChannel. r?mayhemer,nika → Bug 1607984 - P12-4. Start parent load via DocumentChannel. r?mayhemer,nika
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87ff410d6d5d
P1. Make SerializeRedirectData const. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/e73b1ad10587
P2. Add Redirects/LastVisitInfo getters. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/dac4a7633252
P3. Fix leak in nsDSURIContentListener. r=smaug
https://hg.mozilla.org/integration/autoland/rev/9f2a48ab3d51
P4. Expose SetClassificationFlagsHelper. r=valentin
https://hg.mozilla.org/integration/autoland/rev/2eaf4a867ed7
P5. Add missing nsIIdentChannel interface. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/5883b99090e5
P6. Fix test. r=MattN
https://hg.mozilla.org/integration/autoland/rev/0db8c5437f96
P7. Fix compilation failure in non-unified mode. r=valentin
https://hg.mozilla.org/integration/autoland/rev/4d0b7005e95e
P9. Implement nsIThreadRetargetableStreamListener in ParentChannelListerner. r=valentin
https://hg.mozilla.org/integration/autoland/rev/4685da63cb65
P10. Wait until OnStopRequest has been called to clear mRequest. r=valentin
https://hg.mozilla.org/integration/autoland/rev/c75b3ce009e8
P11. Don't assume the page will be loaded synchronously. r=zombie
https://hg.mozilla.org/integration/autoland/rev/ce44570cce1d
P12-1. Move methods to base class. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/277d3fd86386
P12-2. Refactor DocumentLoadListener. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/2896323daed4
P12-3. Let DocumentChannel decides when it can be used. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/e27e24312c27
P12-4. Start parent load via DocumentChannel. r=mayhemer,nika,mattwoodrow,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/b507b6482b9d
P13. Proxy the first http-on-opening-request event to the DocumentChannel. r=mayhemer,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/d77b869fb6b2
P14. Fix test_bug1339722.html when using PPDC. r=valentin
https://hg.mozilla.org/integration/autoland/rev/383252c721a1
P15. Wait for the load to start before setting the event handler. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/baba9f0949b1
P16. Always whitelist file. r=markh
https://hg.mozilla.org/integration/autoland/rev/a6ae63b01821
P17. Put ParentProcessDocumentChannel behind a pref. r=necko-reviewers,mattwoodrow

The test became invalid with the preference introduced in P17.

Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/598a2ed6fac1
P14-2. Correct preference check. r=mattwoodrow
Attachment #9141185 - Attachment is obsolete: true
No longer regressions: 1630211
You need to log in before you can comment on or make changes to this bug.