1597440 - Audit nsIDocShellTreeItem usage in nsGlobalWindowOuter::FocusOuter in dom/base/nsGlobalWindowOuter.cpp

Status: NEW

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

Description

[Kannan Vijayan [:djvj]]

In file dom/base/nsGlobalWindowOuter.cpp

Gets the root DocShell tree item, and its outer window, and checks if it’s the same as the active DOM window.

Retrieves parent window and sets its focus target to the element holding this window, if a parent document exists.

Needs change to user code to handle case where parent is not in process, or to send IPC to parent.

Most efficient approach is to send IPC to chrome which dispatches appropriate sub-IPCs to all relevant processes.

Existing implementation should be moved to using BrowsingContext and WindowContext.

Comment 1

[Chris Peterson [:cpeterson]]

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

Comment 2

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

:hsivonen, this appears to be related to focus management, which you've been working on porting to Fission. Could you look into making nsGlobalWindowOuter::FocusOuter work when fission is enabled?

Comment 3

[Henri Sivonen (:hsivonen)]

I need do deal with bug 1614297 and bug 1615548 first. I'll take a look at this after unless someone else gets to this more quickly.

Comment 4

[Henri Sivonen (:hsivonen)]

It turns out that I want to deal with content-process uses of mActiveWindow first.

Comment 5

[Henri Sivonen (:hsivonen)]

Hmm. This is broader in scope than that. Will spin off a bug blocking this one.

Comment 6

[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 is broken with Fission, fixing it blocks enabling Fission is Nightly.

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 7

[Chris Peterson [:cpeterson]]

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

Comment 8

[Henri Sivonen (:hsivonen)]

This should work fine when Fission is enabled. Could be written without nsIDocShellTreeItem using bug 1633723.

Comment 9

[Chris Peterson [:cpeterson]]

(In reply to Henri Sivonen (:hsivonen) from comment #8)

This should work fine when Fission is enabled. Could be written without nsIDocShellTreeItem using bug 1633723.

Thanks. In that case, this bug doesn't need to block shipping Fission MVP. I'll move this bug to a future milestone when we remove the nsIDocShellTreeItem interface entirely (someday).