1647442 - Fix usage of GetFrameElementInternal and IsInclusiveDescendantOf in Document::AdoptNode

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Kris Maglione [:kmag]]

It's meant to prevent us from adopting a frame element into a descendant document, but it doesn't handle cases where there are cross-process interstitials.

Comment 1

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

The calls to get the frame element are probably fine here, but we should probably do them off of the BrowsingContext as we climb the tree, and ignore missing ones, so that we handle situations like A1 -> B -> A2 adopting the frame from A1 into A2. https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/dom/base/Document.cpp#9359-9372

Comment 2

[Steven MacLeod [:smacleod]]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #1)

The calls to get the frame element are probably fine here, but we should probably do them off of the BrowsingContext as we climb the tree, and ignore missing ones, so that we handle situations like A1 -> B -> A2 adopting the frame from A1 into A2. https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/dom/base/Document.cpp#9359-9372

When Document::AdoptNode() is called for a document that was just created with document.implementation.createDocument(...), Document::GetBrowsingContext() is null. Do we want to try and fallback to the document tree for these cases? or is there some other behaviour you'd prefer?

Comment 3

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

We discussed this a bit on Matrix, but this should work OK if we first get the window using the old mechanism before getting the BrowsingContext, rather than directly getting the BrowsingContext, as a Document may not be loaded in a container (like in this case where we have a data document).

Comment 4

[Steven MacLeod [:smacleod]]

Attachment: Bug 1647442 - handle OOP ancestors in Document::AdoptNode. r=nika!

We now use BrowsingContext to climb the tree and grab frame elements so
we skip over any OOP ancestors.

Document::GetBrowsingContext() may be null when the document was
just created with document.implementation.createDocument.
Document::GetWindow() will also return null in this case, so we
assume the document isn't a descendant of the adoptedNode, preserving
the old behaviour.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e07dcc1c6c3
handle OOP ancestors in Document::AdoptNode. r=nika

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0e07dcc1c6c3