1597467 - [meta] Audit and fix uses of Document::mSubDocuments (pre: nsIDocShellTreeItem usage in nsDocumentViewer::SyncParentSubDocMap in layout/base/nsDocumentViewer.cpp)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, task, P2)

Description

[Kannan Vijayan [:djvj]]

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

[Chris Peterson [:cpeterson]]

Kannan says replacing nsIDocShellTreeItem calls should block enabling Fission in Nightly (M6).

Comment 2

[Chris Peterson [:cpeterson]]

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

[BugBot [:suhaib / :marco/ :calixte]]

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

[Chris Peterson [:cpeterson]]

Auditing whether this use of nsIDocShellTreeItem breaks when Fission is enabled blocks Fission Nightly.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

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

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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

[Hiroyuki Ikezoe (:hiro)]

Moving to DOM:Core (which is an appropriate one what I think).

Comment 9

[Neha Kochar [:neha]]

Edgar, can you take this one up?

Comment 10

[Neha Kochar [:neha]]

Moving this meta bug to MVP as one of the dependencies is tracked in MVP.

Comment 11

[Neha Kochar [:neha]]

Steven, please confirm that the auditing is complete for this API.

Comment 12

[Neha Kochar [:neha]]

All dependencies expected to be complete latest by M8.

Comment 13

[Neha Kochar [:neha]]

The audit is complete and the one remaining bug is in M8 and assigned to Henri. Closing this.