Open Bug 1587414 Opened 3 months ago Updated 2 months ago

Fix usage of nsIDocShellTreeItem in nsContentUtils::GetPresentationUrl

Categories

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

defect

Tracking

()

Fission Milestone M6

People

(Reporter: djvj, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [rm-docshell-tree-item:simple])

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

Simple fix. Checks whether the same-type root of the document tree is also the actual root. Change to use BrowsingContext.

Whiteboard: [rm-docshell-tree-item:simple]
Fission Milestone: --- → M5
Priority: -- → P2
Fission Milestone: M5 → Future

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

Fission Milestone: Future → M6

@kmag I had a question about this one. You informed me at some point that our BrowsingContext tree are all going to be the same-type up the tree. I didn't know that when I wrote this summary. Does this mean that the root and same-type-root in this instance are definitely always the same, and the conditional here is perfunctory?

Flags: needinfo?(kmaglione+bmo)

(In reply to Kannan Vijayan [:djvj] from comment #2)

@kmag I had a question about this one. You informed me at some point that our BrowsingContext tree are all going to be the same-type up the tree. I didn't know that when I wrote this summary. Does this mean that the root and same-type-root in this instance are definitely always the same, and the conditional here is perfunctory?

Well, there's a lot going on here...

In a BrowsingContext tree, all items will always be the same type. That isn't true for nsIDocShellTreeItem, though. DocShell tree items are also connected to parents of a different type.

However, in a content process, we are only allowed to have content DocShells, and the branch in question is only taken in content processes, which means that in this case they definitely are always going to be the same either way.

That said,

  1. I have no idea why the comment mentions <iframe mozbrowser>, and,
  2. If we change this to use BrowsingContext and the root tree item is in a different process, there will be no DocShell for it, and no BrowserChild. But the code would probably be less wrong that way than it is currently, where it gets the presentation URL from something other than the same-type root...
Flags: needinfo?(kmaglione+bmo)
You need to log in before you can comment on or make changes to this bug.