Closed Bug 1646567 Opened 5 years ago Closed 5 years ago

Fix GetInProcessParentDocshell usage in nsDocShell::HasUnloadedParent

Categories

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

defect

Tracking

()

RESOLVED FIXED
81 Branch
Fission Milestone M6b
Tracking Status
firefox81 --- fixed

People

(Reporter: kmag, Assigned: smacleod)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.

Should be possible to recursively check if our parent WindowContext is the current on in it's BrowsingContext up the tree, only checking the docshell if it's present.

Assignee: nobody → smacleod
Status: NEW → ASSIGNED
Fission Milestone: --- → M6b

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

... only checking the docshell if it's present.

If I understand correctly, this means we'd only be checking same origin docshells up the tree. So if we had the situation a.com <- b.com <- a.com, e.g.:

<!-- http://a.com -->
<iframe src="b.com"></iframe>

<!-- http://b.com -->
<iframe src="http://a.com/frame.html"></iframe>

If nsDocShell::HasUnloadedParent is called on the http://a.com/frame.html docshell, we'd end up not checking if the docshell for b.com is unloaded. Is this okay because if it was unloaded http://a.com/frame.html's parent WindowContext (http://b.com) wouldn't be the current WindowContext of the BrowsingContext, so that's how we're checking it? or is there some other reason we can skip the check for OOP docshells in the tree?

Flags: needinfo?(nika)

Reading my last comment back it's a little less understandable than I was hoping. Basically I'm trying verify my understanding that checking if the parent WindowContext is the current WindowContext of its BrowsingContext is a sufficient proxy for checking if the OOP docshell is unloaded.

Yeah, that's pretty much right. If it was unloaded, the parent WindowContext wouldn't be current anymore (and/or any of our ancestors would be discarded). This doesn't handle the case where the cross-process frame is currently running unload event handlers, but that is only relevant for in-process frames, as running unload is synchronous(*)

*: JS can spin a nested event loop, but it's still conceptually synchronous.

Flags: needinfo?(nika)

With fission enabled, walking the docshell tree to find unloaded
parents would stop at the first OOP parent. Instead we now walk
the BrowsingContext/WindowContext tree. To still check parents
which are OOP, we take advantage of the fact that a parent
WindowContext will no longer be the current WindowContext if
the parent was unloaded. For in process docshell's we additionally
check the docshell directly.

Pushed by smacleod@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6164e09df60e fix nsDocShell::HasUnloadedParent to work for OOP frames. r=nika

(In reply to Atila Butkovits from comment #7)

The push caused mass failures.

Woops, sorry about that!

Will land the fixed up patch after a try run.

Flags: needinfo?(smacleod)
Pushed by smacleod@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0d06bfd1fa94 fix nsDocShell::HasUnloadedParent to work for OOP frames. r=nika
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: