Closed Bug 1646567 Opened 2 years ago Closed 2 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.
Severity: -- → S3
Priority: -- → P3

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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.