1597437 - Audit nsIDocShellTreeItem usage in nsGlobalWindowOuter::GetContentInternal in dom/base/nsGlobalWindowOuter.cpp

Status: RESOLVED FIXED

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

Description

[Kannan Vijayan [:djvj]]

In file dom/base/nsGlobalWindowOuter.cpp

Uses DocShellTree to get the DOMWindowOuter for the same-type root document.

Only works if root document is in-process.

Is called by JS-API “Window.content”, so invokeable from within content.

Must use IPC here or in caller to handle cases where the root is out-of-process.

Handling for in-process root should be changed to use BrowsingContext.

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)]

This getter is deprecated, and we had plans a few years ago to make it chrome-only. We may be able to get away with also making it parent-process-only after those changes. (bug 1400140)

My attempt to look at the usage counter information suggested that this property is accessed in ~0.001% of documents on beta 74 (though I may be interpreting the data wrong), meaning that it's likely we can get rid of it sooner rather than later. :bzbarsky, would this be reasonable to get rid of soon?

Comment 3

[Boris Zbarsky [:bzbarsky]]

It's accessed in .01% of documents and .07% of pageloads (most pageloads involve multiple documents) on beta 74, if I am not misreading the data.

That said, we have had no regression reports after disabling it in nightly. I would be quite happy to put it behind a pref and let that ride the trains to release.... If it turns out there's a problem we completely missed, we can flip the pref at that point, but I don't really expect there to be any problems.

Comment 4

[Chris Peterson [:cpeterson]]

We need to audit this use of the nsIDocShellTreeItem interface or just remove it, as bz suggested in comment 3.

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 5

[Chris Peterson [:cpeterson]]

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

Comment 6

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

This can be worked around for web content without removing the getter by changing nsGlobalWindowOuter::GetContentOuter to call GetContentInternal only if XRE_IsParentProcess(), and otherwise call and wrap the return value from GetTopOuter.

Comment 7

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

Attachment: Bug 1597437 - Handle 'window.content' legacy getter better with Fission,

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/475d5cc76937
Handle 'window.content' legacy getter better with Fission, r=farre,Jamie

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/475d5cc76937