Open Bug 1597474 Opened 2 years ago Updated 6 months ago

[meta] Fix uses of Fission-incompatible `nsDocumentViewer::CallChildren` method


(Core :: Layout, task, P2)




Fission Milestone Future


(Reporter: djvj, Unassigned)


(Depends on 1 open bug, Blocks 1 open bug)


(Keywords: meta, Whiteboard: [rm-docshell-tree-item:hard])

In file layout/base/nsDocumentViewer.cpp

Helper method to call a closure on all docshell tree children.

Used by various methods:

  • Setting force/hint charsets.
  • Pausing painting.
  • Etc.

Change to using BrowsingContext.

Callers of each of these will need to be fixed to make sure that they deal with the fact that only in-process children will be seen. Audit of callers needs to ensure one of two conditions:

  • Caller directly handles out-of-process children.
  • Caller skips out-of-process children and the indirect callers of that code is modified to dispatch a call to the same method across the various relevant content processes.

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

Fission Milestone: --- → M6
Priority: -- → P3

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:

:farre's presentation with examples of replacing nsIDocShellTreeItem with BrowsingContext, WindowContext, SyncedContexts, and BrowsingContextGroup:

Component: DOM: Navigation → Layout
Priority: P3 → --
Summary: Fix uses of nsDocumentViewer::CallChildren in layout/base/nsDocumentViewer.cpp → Audit nsIDocShellTreeItem usage in nsDocumentViewer::CallChildren in layout/base/nsDocumentViewer.cpp
Whiteboard: [rm-docshell-tree-item:hard] → [rm-docshell-tree-item:hard] [layout:triage-discuss]

The priority flag is not set for this bug.
:jwatt, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jwatt)

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

Fission Milestone: M6 → M6c

Almost every caller of CallChildren probably needs to be individually changed, likely to use a synced field on WindowContext or something similar instead. CallChildren itself as a method cannot be changed to work with Fission.

Whiteboard: [rm-docshell-tree-item:hard] [layout:triage-discuss] → [rm-docshell-tree-item:hard]

TYLin: Would you be able to look into this one?

Flags: needinfo?(jwatt) → needinfo?(aethanyc)
Severity: normal → N/A
Type: defect → task
Priority: -- → P2
Summary: Audit nsIDocShellTreeItem usage in nsDocumentViewer::CallChildren in layout/base/nsDocumentViewer.cpp → Fix uses of Fission-incompatible `nsDocumentViewer::CallChildren` method
Fission Milestone: M6c → M6b
Flags: needinfo?(kmaglione+bmo)
Depends on: 1662838
Depends on: 1662839
Depends on: 1662840

Dependencies filed.

Flags: needinfo?(kmaglione+bmo)
Assignee: nobody → cam

Cameron, can you review the dependencies and assess their effect so they can be appropriately triaged into M6b or M6c? For the M6b bugs, please also provide an estimate date for the patch in the Whiteboard field as M6b is nearing completion.

Flags: needinfo?(cam)
Depends on: 1662841
Flags: needinfo?(aethanyc)
  • PausePainting / ResumePainting (bug 1662838) -- only used in a single test, and there are no OOP subdocuments involved. Nothing to do for now.
  • SetHintCharset / SetHintCharacterSetSource / SetAuthorStyleDisabled (bug 1662839) -- I think the charset things are Thunderbird-only, but Henri will confirm. The AuthorStyleDisabled I am working on a patch for now.
  • SetOverrideDPPX / EmulateMediumInternal / EmulatePrefersColorSchemeInternal (bug 1662840) -- these are devtools related, so let's do them later.
Flags: needinfo?(cam)

Moving this meta to M7 as the dependent bugs are in M6c or M7.

Fission Milestone: M6b → M7

The remaining uses are bug 1662838 (nothing broken with Fission) and bug 1662840 (Devtools does not consider this a Beta blocker). So, removing this meta for M7 as there are no Beta blocking tasks here.

Fission Milestone: M7 → ---

Unassigning heycam because he is no longer working on Firefox.

Assignee: cam → nobody
Fission Milestone: --- → ?

MVP so we can revisit and cleanup before shipping everywhere by default.

Fission Milestone: ? → MVP
Depends on: 1690100

The only remaining Fission blocking work is in bug 1690100 for devtools. We expect this to be complete in M8.

Fission Milestone: MVP → M8

The only remaining Fission blocking work is in bug 1690100 for devtools, which is already tracked for M8. Removing the Fission milestone from this meta.

Fission Milestone: M8 → ---
Depends on: 1701765

Tracking this meta bug for M8 since blocking bug 1690100 for devtools is tracked for M8.

Blocking bug 1701765 about refactoring HintCharacterSet code doesn't need to block Fission.

Fission Milestone: --- → M8
Closed: 8 months ago
Resolution: --- → FIXED
Resolution: FIXED → ---
Depends on: 1701771
Summary: Fix uses of Fission-incompatible `nsDocumentViewer::CallChildren` method → [meta] Fix uses of Fission-incompatible `nsDocumentViewer::CallChildren` method

This meta bug has only one remaining blocking bug (bug 1690100) and it is a DevTools bug. So this meta bug no longer needs to block Fission MVP.

Fission Milestone: M8 → Future
You need to log in before you can comment on or make changes to this bug.