Audit nsIDocShellTreeItem usage in mozilla::widget::WidgetUtils::DOMWindowToWidget in widget/SharedWidgetUtils.cpp
Categories
(Core :: Widget, defect)
Tracking
()
Fission Milestone | M6c |
People
(Reporter: djvj, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [rm-docshell-tree-item:simple])
In file widget/SharedWidgetUtils.cpp
Walks up the docshell tree, looking for a window with an associated widget.
A cursory audit of the users seems to indicate that this should only be invoked in either chrome code, or situations where the top-level window is in-process.
Should be fine to change this to using BrowsingContext to walk up the tree, asserting that all items along the way are in-process, and otherwise leaving the logic the same.
Comment 1•5 years ago
|
||
Kannan says replacing nsIDocShellTreeItem calls should block enabling Fission in Nightly (M6).
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Moving to Widget.
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•5 years ago
|
||
Jim, someone with Widget experience will need to triage this bug and audit this code.
Comment 4•5 years ago
|
||
Auditing whether this use of nsIDocShellTreeItem breaks when Fission is enabled blocks Fission Nightly.
Updated•4 years ago
|
Comment 5•4 years ago
|
||
Each BrowserChild has it's own PuppetWidget, meaning this probably works OK as-is so needs no changes for Fission. Jim, can you confirm?
Comment 6•4 years ago
|
||
This should work OK as-is for Fission, so we shouldn't need to change anything.
Description
•