1681796 - Prevent superfluous PictureInPictureParent actors from being associated with each tab

Status: RESOLVED FIXED

(Firefox :: Untriaged, defect)

Description

[Hunter Jones]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Comment 1

[:Gijs (out for now; he/him)]

So I think this is due to the code at https://searchfox.org/mozilla-central/rev/4415bec7a49c50a338167d9c8934527b9cae59d0/toolkit/components/pictureinpicture/PictureInPicture.jsm#323-325 which will get the parent actor when handling a request to enter PiP.

This is odd because AFAICT we only use the ref for a check for where to position things (via a map) -- but it's not the right actor for that anyway, as it doesn't correspond to the player window. But it's quite possible I'm misunderstanding that part...

Comment 2

[Hunter Jones]

Attachment: Bug 1681796: Prevent superfluous PictureInPictureParent actors from being associated with each tab r?mconley,gijs

Comment 3

[Hunter Jones]

(In reply to :Gijs (he/him) from comment #1)

So I think this is due to the code at https://searchfox.org/mozilla-central/rev/4415bec7a49c50a338167d9c8934527b9cae59d0/toolkit/components/pictureinpicture/PictureInPicture.jsm#323-325 which will get the parent actor when handling a request to enter PiP.

This is odd because AFAICT we only use the ref for a check for where to position things (via a map) -- but it's not the right actor for that anyway, as it doesn't correspond to the player window. But it's quite possible I'm misunderstanding that part...

This is correct, I erroneously added this in a previous patch to keep the code as similar as possible to what it already was. I plan to remove this now.

Comment 4

[Virginia (please send need info request to Brindusa Tot)]

Hi Hunter,

As per your last comment I see that the issue has been solved. For this reason I'll mark this ticket as Resolved - Works for me.

If the issue is still reproducible with the latest Firefox version, feel free to reopen the bug with more information.

Regards,
Virginia

Comment 5

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

I think Hunter was referring to a line of code that had been added in a previous patch (one in bug 1589680). I think this bug is still valid - I'm going to re-open.

Comment 6

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

Hey Hunter, there's a small nit left on the patch that we hope can be addressed before it lands. Got time for it?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f7fd3b38dfff
Prevent superfluous PictureInPictureParent actors from being associated with each tab r=Gijs

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f7fd3b38dfff