1587423 - Audit usage of nsIDocShellTreeItem in nsFocusManager::WindowRaised

Status: RESOLVED INVALID

(Core :: DOM: Navigation, defect, P2)

Description

[Kannan Vijayan [:djvj]]

https://searchfox.org/mozilla-central/rev/7cc0f0e89cb40e43bf5c96906f13d44705401042/dom/base/nsFocusManager.cpp#662

This code sets the visibility of the window associated with the tree owner, which may be out of process. I spent some time trying to verify that this always get called in contexts where the base window is in-process, but it doesn't seem like that's the case.

It seems like IPC at this point might be the best option for now. Calls into this code should not be frequent (windows raising and being lowered), and the callers don't seem to be easily amenable to lifting this logic (set visibility on base window) into a different location - at the very least it would be confusing.

Comment 1

[Chris Peterson [:cpeterson]]

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

Comment 2

[Chris Peterson [:cpeterson]]

We need to 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

[Chris Peterson [:cpeterson]]

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

Comment 4

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

In general, getting the tree owner is probably fine, even with Fission enabled, so closing.