Closed Bug 1570406 Opened 5 years ago Closed 5 years ago

Picture-in-Picture toggle mouse event listeners never removed on pagehide

Categories

(Toolkit :: Video/Audio Controls, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

Details

Attachments

(1 file)

In bug 1545296, I added code here:

https://searchfox.org/mozilla-central/rev/b38e3beb658b80e1ed03e0fdf64d225bd4a40327/toolkit/actors/PictureInPictureChild.jsm#139-143

to remove the mouse event listeners on pagehide. We check to ensure that the event is firing for the top-level document rather than any subframes.

Unfortunately, that check is broken because a pagehide event's target is a document, and document.top doesn't exist. So we keep bailing out of this pagehide event listener.

What we should probably be doing is checking if the event.target.ownerGlobal.top is equal to event.target.ownerGlobal. That way, we detach the event listeners on pagehide.

This is probably a bit of a leak here - we're just slowly accumulating event listeners (which are mostly inert) and never removing them.

We were using the pagehide event before incorrectly: the pagehide event target
was being inspected to see if it was the top-level frame, and only then, would
we remove the mouse event handlers on the window root.

The problem is that event.target.top on a pagehide event is undefined, since the
event target is a Document.

Further, we probably want to remove any window root event listeners that were
registered for subframes as well. I'm not sure why I decided to try to filter
out subframes, but that definitely can keep windows alive.

So instead, I'm using the "cleanup" method that ActorManagerChild calls if
defined on an ActorChild, which runs when the associated frame unloads. This
way, if a subframe happened to have registered a window root event listener,
it'll get unregistered.

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6bfccacf0bb0
Ensure that we properly detach window root event listeners in PictureInPictureToggleChild. r=JSON_voorhees
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: