1543251 - Move hasBeforeUnload from nsITabParent to CanonicalBrowsingContext

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, task, P2)

Description

[Mike Conley (:mconley) (:⚙️)]

We added hasBeforeUnload to nsITabParent in bug 1336763 so that we could know whether or not we needed to run permitUnload for the frames in a tab that was closing, or if we could just close it right away.

In a Fission-world, it makes sense, I think, to either put this hasBeforeUnload property on the top-most CanonicalBrowsingContext of a tree in the chrome process, or the associated WindowGlobalParent. Either is fine with me.

Here's where we increment and decrement (two places) the beforeunload event listener counts in the content process.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

Comment 2

[Neha Kochar [:neha]]

ni for Andreas for thoughts on this. Candidate for M4?

Comment 3

[Andreas Farre [:farre]]

Sounds perfectly doable. M4, sure.

Comment 4

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

It might make more sense to keep track of this on WindowGlobalParent, given that these are recorded per-inner-window already. That way we'd get clearing the count upon changing the active global from navigations for free.

We'd probably put a flag on WindowGlobalParent, and perhaps move the count from nsGlobalWindowInner to WindowGlobalChild. Whenever the number on the WindowGlobalChild changes from or to 0, we'd send a message over the WindowGlobal actor (e.g. SendAddedBeforeUnloadListener or SendRemovedBeforeUnloadListener) which would flip the bit in the parent process.

Comment 5

[u608768]

Attachment: Bug 1543251 - Move hasBeforeUnload from PBrowser to PWindowGlobal, r=nika

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/405100db6c45
Move hasBeforeUnload from PBrowser to PWindowGlobal, r=nika

Comment 7

[Noemi Erli[:noemi_erli]]

Backed out changeset 405100db6c45 (Bug 1543251) for failing in nsGlobalWindowInner.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=255835862&resultStatus=testfailed%2Cbusted%2Cexception&revision=405100db6c45f0463286067df5b491f74790d9aa

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255831134&repo=autoland&lineNumber=1094
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255831085&repo=autoland&lineNumber=14836

Backout: https://hg.mozilla.org/integration/autoland/rev/0560e58effb76deac1fc7433deea7610fe4a5bc7

Comment 8

[u608768]

Updated patch is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4223ae04b28deb052b19eb6c3c499f5d7ce833f4

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6709a3bc1a1c
Move hasBeforeUnload from PBrowser to PWindowGlobal, r=nika

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6709a3bc1a1c