DocShell currently requires mIsFrame to be set to consider thing not to be frames. This however doesn't seem to ever get set when in the OOP fission case.

Whilst working on Bug 1570243 I noticed that this was hapening for:
./mach test dom/security/test/csp/test_worker_src.html --enable-fission

An example crash whilst asserting for frames with no bc->GetParent:

 2:03.14 PASS allowing worker from https://example.com (service-worker-allowed)
 2:03.16 GECKO(6862) creating. load info
 2:03.16 GECKO(6862) got a doc: https://example.com/tests/dom/security/test/csp/file_worker_src_child_governs.html,
 2:03.16 GECKO(6862) mIsFrame false
 2:03.16 GECKO(6862) Assertion failure: !mBrowsingContext->GetParent(), at /home/jonathan/debugging/mozilla-unified/docshell/base/nsDocShell.cpp:10141
 2:07.14 GECKO(6862) #01: nsDocShell::DoURILoad(nsDocShellLoadState*, bool, nsIDocShell**, nsIRequest**) (/home/jonathan/debugging/mozilla-unified/docshell/base/nsDocShell.cpp:10141)
 2:07.15 GECKO(6862) #02: nsDocShell::InternalLoad(nsDocShellLoadState*, nsIDocShell**, nsIRequest**) (/home/jonathan/debugging/mozilla-unified/docshell/base/nsDocShell.cpp:9424)
 2:07.15 GECKO(6862) #03: nsDocShell::LoadURI(nsDocShellLoadState*, bool) (/home/jonathan/debugging/mozilla-unified/docshell/base/nsDocShell.cpp:796)
 2:07.16 GECKO(6862) #04: nsDocShell::LoadURI(nsTSubstring<char16_t> const&, mozilla::dom::LoadURIOptions const&) (/home/jonathan/debugging/mozilla-unified/docshell/base/nsDocShell.cpp:3675)
 2:07.24 GECKO(6862) #05: nsWebBrowser::LoadURI(nsTSubstring<char16_t> const&, mozilla::dom::LoadURIOptions const&) (/home/jonathan/debugging/mozilla-unified/toolkit/components/browser/nsWebBrowser.cpp:530)
 2:07.83 GECKO(6862) #06: mozilla::dom::BrowserChild::RecvLoadURL(nsTString<char> const&, mozilla::dom::ShowInfo const&) (/home/jonathan/debugging/mozilla-unified/dom/ipc/BrowserChild.cpp:1093)
 2:07.94 GECKO(6862) #07: mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) (/home/jonathan/debugging/mozilla-unified/obj-devedition/ipc/ipdl/PBrowserChild.cpp:4529)
 2:08.21 GECKO(6862) #08: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (/home/jonathan/debugging/mozilla-unified/obj-devedition/ipc/ipdl/PContentChild.cpp:8097)
 2:08.30 GECKO(6862) #09: mozilla::dom::ContentChild::OnMessageReceived(IPC::Message const&) (/home/jonathan/debugging/mozilla-unified/dom/ipc/ContentChild.cpp:3871)
 2:08.37 GECKO(6862) #10: mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) (/home/jonathan/debugging/mozilla-unified/ipc/glue/MessageChannel.cpp:2208)
 2:09.45 GECKO(6862) #11: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (Unified_cpp_ipc_glue1.cpp:?)
 2:09.47 GECKO(6862) #12: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) (Unified_cpp_ipc_glue1.cpp:?)
 2:09.48 GECKO(6862) #13: mozilla::ipc::MessageChannel::MessageTask::Run() (/home/jonathan/debugging/mozilla-unified/ipc/glue/MessageChannel.cpp:2004)
 2:09.52 GECKO(6862) #14: mozilla::SchedulerGroup::Runnable::Run() (/home/jonathan/debugging/mozilla-unified/xpcom/threads/SchedulerGroup.cpp:295)
 2:09.58 GECKO(6862) #15: nsThread::ProcessNextEvent(bool, bool*) (/home/jonathan/debugging/mozilla-unified/xpcom/threads/nsThread.cpp:1225)
 2:09.59 GECKO(6862) #16: NS_ProcessNextEvent(nsIThread*, bool) (/home/jonathan/debugging/mozilla-unified/xpcom/threads/nsThreadUtils.cpp:486)
 2:09.61 GECKO(6862) #17: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (/home/jonathan/debugging/mozilla-unified/ipc/glue/MessagePump.cpp:88)
 2:09.62 GECKO(6862) #18: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (/home/jonathan/debugging/mozilla-unified/ipc/glue/MessagePump.cpp:272)
 2:09.65 GECKO(6862) #19: MessageLoop::RunInternal() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:315)
 2:09.66 GECKO(6862) #20: MessageLoop::RunHandler() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:309)
 2:09.68 GECKO(6862) #21: MessageLoop::Run() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:290)
 2:09.71 GECKO(6862) #22: nsBaseAppShell::Run() (/home/jonathan/debugging/mozilla-unified/widget/nsBaseAppShell.cpp:139)
 2:09.72 GECKO(6862) #23: XRE_RunAppShell() (/home/jonathan/debugging/mozilla-unified/toolkit/xre/nsEmbedFunctions.cpp:934)
 2:09.73 GECKO(6862) #24: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (/home/jonathan/debugging/mozilla-unified/ipc/glue/MessagePump.cpp:238)
 2:09.73 GECKO(6862) #25: MessageLoop::RunInternal() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:315)
 2:09.73 GECKO(6862) #26: MessageLoop::RunHandler() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:309)
 2:09.73 GECKO(6862) #27: MessageLoop::Run() (/home/jonathan/debugging/mozilla-unified/ipc/chromium/src/base/message_loop.cc:290)
 2:09.75 GECKO(6862) #28: XRE_InitChildProcess(int, char**, XREChildData const*) (/home/jonathan/debugging/mozilla-unified/toolkit/xre/nsEmbedFunctions.cpp:773)
 2:09.76 GECKO(6862) #29: mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*) (/home/jonathan/debugging/mozilla-unified/toolkit/xre/Bootstrap.cpp:67)
 2:09.80 GECKO(6862) #30: content_process_main(mozilla::Bootstrap*, int, char**) (/home/jonathan/debugging/mozilla-unified/browser/app/../../ipc/contentproc/plugin-container.cpp:56)
 2:09.80 GECKO(6862) #31: main (/home/jonathan/debugging/mozilla-unified/browser/app/nsBrowserApp.cpp:272)
 2:11.47 GECKO(6862) #32: __libc_start_main (/build/glibc-KRRWSm/glibc-2.29/csu/../csu/libc-start.c:342)
 2:11.48 GECKO(6862) #33: _start (/home/jonathan/debugging/mozilla-unified/obj-devedition/dist/bin/firefox)
 2:11.48 GECKO(6862) #34: ??? (???:???)

The test is essentially just setting a cors URL to a frame with frame.src="<url>"

I added:

  // Must never have a parent as a document
  MOZ_ASSERT_IF(contentPolicyType == nsIContentPolicy::TYPE_DOCUMENT, !mBrowsingContext->GetParent());
  // Subdocuments must have a parent
  MOZ_ASSERT_IF(contentPolicyType == nsIContentPolicy::TYPE_SUBDOCUMENT, mBrowsingContext->GetParent());

To nsresult nsDocShell::DoURILoad just before we create the LoadInfo.

This fixes the content policy type for document loads in these frames.

Backed out changeset eaee8979fdc4 (bug 1594529) for Browser-chrome in toolkit/components/passwordmgr/test/browser/browser_doorhanger_crossframe.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274953536&repo=autoland&lineNumber=20878

Push with failure:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=eaee8979fdc423b979a8d1dc3915e45eea79a34d\

Backout:
https://hg.mozilla.org/integration/autoland/rev/f26294c01862f3631ef99237789ad82471c479e2

:dluca the latest push will likely cause a lot of test fails with the other patch backed out. Can we disable the test for fission only and add back in :kmag's patch? Either that or we should also back out mine.

Looks like we should just back out my patch too due to Bug 1594540 too.

Backed out changeset b2026c2b563f (bug 1594529) for Browser-chrome failures in workspace/build/src/docshell/base/nsDocShell. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274978920&repo=autoland&lineNumber=19152

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=b2026c2b563f20b49c672c58fb26a943cd7b184c

Backout:
https://hg.mozilla.org/integration/autoland/rev/1138923577823fbae40675932f07cd1a79730db1

:kmag I see a similar error that I saw in https://bugzilla.mozilla.org/show_bug.cgi?id=1593282

When the iframe changes it's src through iframeElement.src="http://other.com" the load never seems to trigger and instead remains on the existing domain.

[Parent 28260, Main Thread] WARNING: NS_ENSURE_TRUE(aCSP) failed: file /home/jonathan/debugging/mozilla-unified/dom/security/FramingChecker.cpp, line 176
[Child 28401, Main Thread] WARNING: Trying to request nsIHttpChannel from DocumentChannelChild, this is likely broken: file /home/jonathan/debugging/mozilla-unified/netwerk/ipc/DocumentChannelChild.cpp, line 44
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 created
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 ComputeIsSecureContext 
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 StartDocumentLoad http://example.com/badssl.com
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 ResetToURI http://example.com/badssl.com
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 ComputeIsSecureContext 
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 ComputeIsSecureContext http://example.com/badssl.com
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 ComputeIsSecureContext http://example.com/badssl.com
++DOMWINDOW == 11 (0x7faa3fecc400) [pid = 28401] [serial = 38] [outer = 0x7faa41abfd40]
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 with PressShell 0x7faa4276c000 and DocShell 0x7faa3b5249a0
[Child 28401: Main Thread]: D/nsSecureBrowserUI 0x7faa4463bb80 OnLocationChange: 0x7faa3b524828 0x7faa3fb28880 http://example.com/badssl.com 0
[Child 28401: Main Thread]: D/DocumentLeak DOCUMENT 0x7faa3fe3b000 UnblockDOMContentLoaded

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

I have confirmed that on try we are switching processes much more often with these fixes, which causes the inFlightProcessId to be a mismatch and cause exceptions that aren't reproducible locally. I changed the id to an array which didn't fix it: https://hg.mozilla.org/try/rev/a5bcd222899026535370c793ded0d227dc8e72da However preventing the id to be removed from the array fixed the issue.

The remaining test fails I have also seen that previous process messages are colliding with the following subtest. dom/security/test/general/test_same_site_cookies_iframe.html for example each subtest in isolation works fine. I can probably fix the test so they don't collide (using new frames perhaps) but that doesn't fix the underlying issue of messages still being received after the iframe has changed location.

Temporarily reassigning these DOM Security Fission bugs to ckerschb for re-triage.

Hi Anny, do you have any updates on this bug? ckerschb says this bug is the primary blocker for a bunch of MixedContentBlocker tests (meta bug 1584157) needed for Fission M4.1.

Hi Chris, I spoke with Nika today and discussed an alternative solution to Jonathan's patch which I implemented, but I am having lots of test failures still https://treeherder.mozilla.org/#/jobs?repo=try&revision=0df53257e1c176e2c1bf4966bc58f8794d691764

I figured I should update this bug with some findings in case someone else is following along. Currently, iframes are not being classified correctly as a type document, and with Fission enabled, one of the problems that this causes is that iframes show up in the list of recently visited windows and tabs. Kris' patch 'Bug 1594529: Infer nsDocShell::IsFrame from BrowsingContext. r=nika' fixes this, however, in fixing one issue, the patch exposes all sorts of other errant behaviour that we have not detected until now.

When we are loading something inside of an OOP iframe, with Kris' patch, we end up in https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/docshell/base/nsDocShell.cpp#9711-9731. However, because the current load is happening in an OOP iframe, from the process of the OOP iframe, we do not have access to the loadingNode of the iframe.

                process A
                      +
                      |
                      |
                      |
                      |
                +-------------------------------------------------------------+
                |     v                                                       |
                |                                                             |
                |   loadingNode/loadingContext is of type HTMLIFrameElement   |
                |                                                             |
                |   +-----------------------------------------+               |
                |   |                                         |               |
                |   |  Docshell for the remote iframe         |               |
                |   |  `loadingNode==nullptr` in this process |               |
                |   |                                         |               |
                |   |    +--------------------------------+   |               |
                |   |    |                                |   |               |
        +------------->  |                                |   |               |
process B       |   |    | Document of the remote iframe  |   |               |
        +--------------------->                           |   |               |
                |   |    |                                |   |               |
                |   |    +--------------------------------+   |               |
                |   |                                         |               |
                |   +-----------------------------------------+               |
                |                                                             |
                |                                                             |
                |                                                             |
                |                                                             |
                +-------------------------------------------------------------+

Not having access to loadingNode is a problem because then we end up here https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/docshell/base/nsDocShell.cpp#9726-9730 and we are unable to load another document inside of the OOP iframe. To combat this, in the third case we can check if we are currently being destroyed, and if not, we can continue carrying on with the load.

Later on in the function, we encounter another obstacle during creation of LoadInfo object https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/docshell/base/nsDocShell.cpp#9796-9800. Since we do not have loadingContext, a lot of information about the load does not get filled out for LoadInfo class https://searchfox.org/mozilla-central/rev/96f1457323cc598a36f5701f8e67aedaf97acfcf/netwerk/base/LoadInfo.cpp#148-286.

To fix this in cases when we are using a document channel to load URIs, we can attempt to fill out the information inside of DocumentLoadListener::Open( in the parent process as eventually this is where we end up sometime in our document loading pipeline.

In order to fill out information on a LoadInfo object that is used for loading subdocuments, we want to save the parent's LoadInfo object on the WindowGlobalParent object that corresponds to the BrowsingContext. Later we can retrieve parent's LoadInfo object when filling out LoadInfo object for subdocuments in DocumentLoadListener::Open and inherit some of the fields.

However, there are cases when just as we are about to save the parent's LoadInfo on WindowGlobalParent, we don't have a channel and thus we are unable to save any LoadInfo for that load. So far I have observed that this happens when the initial load is "about:blank". In this case, Nika suggested that we can attempt to fill out the fields by retrieving appropriate info using top-level document or the opener document.

TODO come up with a longer description

Nika recommends Anny lands this patch because it is an unambiguous improvement, even though it breaks some CSP stuff. We can fix that in follow up work.

Bugbug thinks this bug is a regression, but please revert this change in case of error.

https://hg.mozilla.org/mozilla-central/rev/4ba9a230586a
https://hg.mozilla.org/mozilla-central/rev/fbf55a44d7fb
https://hg.mozilla.org/mozilla-central/rev/16bfa5c1f2ca

Backed out 2 changesets (Bug 1594529) for causing perma wpt2 with ValueError: badly formed hexadecimal UUID string in /cookies/samesite/iframe-reload.https.html CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=3009224928c0c0a2f7b4829c94a7c335fddb7652

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=linux%2C18.04%2Cx64%2Cquantumrender%2Cdebug%2Cweb%2Cplatform%2Ctests%2Cwith%2Cfission%2Cenabled%2Ctest-linux1804-64-qr%2Fdebug-web-platform-tests-fis-e10s-2%2Cw-fis%28wpt2%29&tochange=b1309f9133e81532fc96d9046ffc282400abdef4&fromchange=c522095c6db2e2c3100c855670b39c7f23feade9&selectedJob=297640184

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297640184&repo=autoland&lineNumber=5796

We only backed out the first 2 revisions and missing out the third one that caused assertion failures nsDocShell.cpp
We also backed out the 3rd revision and that fixed the assertion failures

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=3d4ff35ef9a990bc63de39130a1bb4642e471c7c

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=3009224928c0c0a2f7b4829c94a7c335fddb7652&selectedJob=297687059https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=3009224928c0c0a2f7b4829c94a7c335fddb7652&selectedJob=297687059

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=297687059&repo=autoland&lineNumber=3521

Backouts merged:
https://hg.mozilla.org/mozilla-central/rev/3009224928c0
https://hg.mozilla.org/mozilla-central/rev/3d4ff35ef9a9

https://hg.mozilla.org/mozilla-central/rev/9859407d505e
https://hg.mozilla.org/mozilla-central/rev/29ff609063c2
https://hg.mozilla.org/mozilla-central/rev/bfe21f274ed5
https://hg.mozilla.org/mozilla-central/rev/d3d9cc11fee5

(In reply to Pulsebot from comment #42)

Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1589fcd0929c
Replace MOZ_MUST_USE with [[nodiscard]] in dom/media/webaudio. r=padenot
https://hg.mozilla.org/integration/autoland/rev/63a755d0e9b1
Replace MOZ_MUST_USE with [[nodiscard]] in dom/media. r=bryce

Oops. These patches are unrelated to this bug and should have linked to bug 1633286.