Audit nsIDocShellTreeItem usage in nsDocumentViewer::PermitUnloadInternal in layout/base/nsDocumentViewer.cpp
Categories
(Core :: Layout, defect)
Tracking
()
| Fission Milestone | M6c |
People
(Reporter: djvj, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [rm-docshell-tree-item:hard])
In file layout/base/nsDocumentViewer.cpp
Called from nsIContentViewer::permitUnload XPCOM API
Used for firing beforeUnload handlers for unloading docs.
Iterates over all children of existing DocShell, finding child ContentViewers for each child doc.
Calls PermitUnloadInternal on them.
PermitUnload is a pretty involved method that can run script, dispatch events, etc. on the child document.
beforeUnload handling is special and does not care too much about performance.
IPC should be fine here when children cross process boundaries.
Sync IPC is easiest to do because this is conceptually a synchronous action (traverse entire tree and fire any necessary beforeUnload events).
|
||
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.
|
||
Updated•4 years ago
|
|
||
Comment 5•4 years ago
|
||
I've confirmed that beforeunload event (which is the only one purpose of nsDocumentViewer::PermitUnloadInternal) is properly handled in OOP iframe (I believe it's been handled properly since bug 1533949, thanks to mconley!).
I found an issue that the event is not fired when the document is reloaded (bug 1655866), but as far as I can tell the case should also be handled in the parent process, so nothing to be changed in nsDocumentViewer::PermitUnloadInternal, I think.
Closing WORKSFORME.
Description
•