[meta] Audit and fix uses of Document::mSubDocuments (pre: nsIDocShellTreeItem usage in nsDocumentViewer::SyncParentSubDocMap in layout/base/nsDocumentViewer.cpp)
Categories
(Core :: DOM: Core & HTML, task, P2)
Tracking
()
Fission Milestone | M8 |
People
(Reporter: djvj, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: meta, Whiteboard: [rm-docshell-tree-item:hard])
In file layout/base/nsDocumentViewer.cpp
This is more a generic bug relating to SubDocuments (mSubDocuments in mozilla::dom::Document). Point other functions related to sub-documents to this bug.
Related bug (17 days ago, dholbert): https://bugzilla.mozilla.org/show_bug.cgi?id=1591156
mSubDocuments can no longer be a hashtable mapping to sub-documents, as the sub-documents may not exist in-process.
Change mSubDocuments on Document to map elements to BrowsingContexts instead.
Change all writers of mSubDocuments (including indirect users) to insert BrowsingContexts instead of Documents in the hashtable. Change all readers of mSubDocuments to deal with BrowsingContexts for out-of-process docshells.
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 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•5 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
|
||
In general, we probably need to remove almost every use of Document::mSubDocuments
, as they're all based on a flawed model of how subdocuments work.
Perhaps this should be turned into a metabug for fixing the relevant callers, including: FindContentForSubDocument
, EnumerateSubDocuments
, CollectDescendantDocuments
, CanSavePresentation
, and ReportUseCounters
.
Updated•4 years ago
|
Comment 6•4 years ago
|
||
Filed, and changing this bug as a meta bug for auditing the uses of Document::mSubDocuments
- FindContentForSubDocument (bug 1656107)
- EnumerateSubDocuments (bug 1656105)
- CanSavePresentation (bug 1656111)
- ReportUseCounters (bug 1656114)
- CollectDescendantDocuments (bug 1656108)
(Though I've already audited the last one and closed)
Hsin-Yi, would you mind handling these bugs in your team?
Comment 7•4 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
Filed, and changing this bug as a meta bug for auditing the uses of
Document::mSubDocuments
- FindContentForSubDocument (bug 1656107)
- EnumerateSubDocuments (bug 1656105)
- CanSavePresentation (bug 1656111)
- ReportUseCounters (bug 1656114)
- CollectDescendantDocuments (bug 1656108)
(Though I've already audited the last one and closed)
Hsin-Yi, would you mind handling these bugs in your team?
Commented on each bug. Thanks, hiro!
Comment 8•4 years ago
|
||
Moving to DOM:Core (which is an appropriate one what I think).
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 10•4 years ago
|
||
Moving this meta bug to MVP as one of the dependencies is tracked in MVP.
Comment 11•4 years ago
|
||
Steven, please confirm that the auditing is complete for this API.
Updated•4 years ago
|
Comment 12•4 years ago
|
||
All dependencies expected to be complete latest by M8.
Comment 13•3 years ago
|
||
The audit is complete and the one remaining bug is in M8 and assigned to Henri. Closing this.
Description
•