Audit nsIDocShellTreeItem usage in nsDocumentViewer::LoadComplete in layout/base/nsDocumentViewer.cpp
Categories
(Core :: Layout, defect)
Tracking
()
Fission Milestone | Future |
People
(Reporter: djvj, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [rm-docshell-tree-item:hard])
In file layout/base/nsDocumentViewer.cpp
Finds all child documents (via DocShell tree) that have not yet completed loading, and invokes load events for the element (in the current document) that contains them.
Guarding on child docs accesses:
- TreatAsBackgroundLoad on DocShell
- Sync to BrowsingContext?
- DocShell->Document->GetReadyStateEnum()
- Sync to BrowsingContext? (should be infrequently updated, no sensitive information).
- DocShell->Window->GetFrameElementInternal
- Can this use mSubDocuments (modified to map to BrowsingContext instead of Documents), using the BrowsingContext to find the corresponding element in the current document?
That should eliminate all need for cross-process checks at this event.
Comment 1•5 years ago
|
||
Kannan says replacing nsIDocShellTreeItem calls should block enabling Fission in Nightly (M6).
Updated•5 years ago
|
Comment 2•4 years ago
•
|
||
Moving to Layout.
Please audit this use of the nsIDocShellTreeItem interface. With Fission enabled, Documents and nsDocShells for related frames, such as subframes and parent documents, may not be available within the current process and the corresponding nsIDocShellTreeItem methods will return null
If this code works as-is with Fission, we don't need to remove this usage of nsIDocShellTreeItem until when we remove nsIDocShellTreeItem entirely (bug 1607591) after we ship Fission MVP.
Fission documentation about replacing nsIDocShellTree Item:
https://wiki.mozilla.org/Project_Fission/DocShell_Tree_Replace
:farre's presentation with examples of replacing nsIDocShellTreeItem with BrowsingContext, WindowContext, SyncedContexts, and BrowsingContextGroup:
https://docs.google.com/presentation/d/1K4j6ngty64TZjJNS5qH-MBoOm3TI2dJedVsbH8jUhKE/edit#slide=id.g6e35225e5d_1_264
Comment 3•4 years ago
|
||
The priority flag is not set for this bug.
:jwatt, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 4•4 years ago
|
||
Auditing whether this use of nsIDocShellTreeItem breaks when Fission is enabled blocks Fission Nightly.
Comment 5•4 years ago
|
||
This code seems to be specifically part of the DocGroup::TryToLoadIframesInBackground()
code, which I don't think was ever made Fission-compatible. https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/layout/base/nsDocumentViewer.cpp#1047-1089
I don't think this needs to block Fission, as that change is not on-route to ship before it right now afaik. ni? :smaug to move it back if this is important to get working now.
Comment 6•4 years ago
|
||
Yeah, this is not important. We might want to remove that code, or just make it work with Fission later.
(the code seems to be working quite fine on non-Fission, and it speeds up page loads)
Updated•4 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•