[e10s] Media is unblocked while hovering an unfocused tab

VERIFIED FIXED in Firefox 57

Status

()

P1
major
VERIFIED FIXED
2 years ago
8 months ago

People

(Reporter: emilghitta, Assigned: mconley)

Tracking

({regression})

57 Branch
mozilla57
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-qa-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57+ verified)

Details

(Whiteboard: [photon-performance] [testcoverage])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8901868 [details]
Tab hovering.gif

[Affected versions]:
Firefox 57.0a1 (Build Id:20170828100127)

[Affected platforms]:
Windows 10 64bit.
Mac 10.11.6.
Ubuntu 16.04 64bit.

[Steps to reproduce]:
1. Launch Firefox with a clean profile.
2. Access the https://www.youtube.com/ webpage.
3. Open 1 random video in a new tab without focusing it.
4. Hover over the tab.

[Expected result]:
The media is still blocked for that tab since it was not visited nor the Play Tab button was pressed.

[Actual result]:
The media gets unblocked and the Play Tab button dissapears.

[Regression range]:
This is a recent regression:
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2cbd34e59b97a522498c5b5dbd22677935a025e5&tochange=3bbd8e25df3d92464b97bcb42413febd9041af6f

[Additional Information]
Please observe the attached video for further information regarding this issue.
(Reporter)

Comment 1

2 years ago
Hi Mike!

I see that you pushed some patches regarding tabs. I don't know if they are responsible for this behavior.

Can you please have a look into this ?

Thanks a lot!
Flags: needinfo?(mconley)
I CAN reproduce this issue with enable e10s, but CAN NOT reproduce it once disable e10s.

From the log, it seems the document would be incorrectly changed to visible when the cursor hovering over, so triggering the resume mechanism.
(Reporter)

Updated

2 years ago
Summary: Media is unblocked while hovering an unfocused tab → [e10s] Media is unblocked while hovering an unfocused tab
Duplicate of this bug: 1394118
Priority: -- → P1
Whiteboard: [testcoverage]
Duplicate of this bug: 1394443
moving tracking from duplicate bug 1394118
tracking-firefox57: --- → +
I'm seeing this not just on hover, but it seems to sometimes trigger playing background tabs when you are navigating tabs (ctrl-tab, ctrl-shift-tab, as well as opening and closing new tabs)

Updated

2 years ago
Blocks: 1385453
Duplicate of this bug: 1395944

Updated

2 years ago
Duplicate of this bug: 1396566

Comment 10

2 years ago
Alastor, as Julien mentioned this bug is a regression which could impact the release of block-autoply. could you help to see anything we could make the bug moving forward? Thanks.
Flags: needinfo?(alwu)
As I mentioned in comment2, this issue is caused by incorrect the document's visibility, it should be fixed by front end's solution.
Flags: needinfo?(alwu)
(Assignee)

Comment 12

2 years ago
Hello everyone.

I landed the patch that opened this bug. I've been on PTO and today is a holiday. Tomorrow I will be back at work and begin working on this.
(In reply to Bobby Chien [:bchien] from comment #10)
> Alastor, as Julien mentioned this bug is a regression which could impact the
> release of block-autoply. could you help to see anything we could make the
> bug moving forward? Thanks.

I wouldn't hold back block-autoplay for this. It needs to be fixed it nonetheless.

Bug 1385453 also likely duplicates the mouse-over restart functionality of background decoder shutdown.
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #13)
> (In reply to Bobby Chien [:bchien] from comment #10)
> > Alastor, as Julien mentioned this bug is a regression which could impact the
> > release of block-autoply. could you help to see anything we could make the
> > bug moving forward? Thanks.
> 
> I wouldn't hold back block-autoplay for this. It needs to be fixed it
> nonetheless.
> 
> Bug 1385453 also likely duplicates the mouse-over restart functionality of
> background decoder shutdown.
Which is bug 1274919.

Comment 16

2 years ago
Bug 1385453, comment 0 discusses using more than just tab hover for warming up tabs. So it would probably be good to rely on whatever mechanism their code is using (and will in the future).
(Assignee)

Updated

2 years ago
Assignee: nobody → mconley
(Assignee)

Comment 17

2 years ago
I've started studying this. This problem is due to the fact that when warming up the tabs, we set the DocShell active state to true, which updates the visibility state, which is what the media suspending stuff is waiting for to unblock playback.

I suspect this is also causing bug 1395735, since the web-exposed Page Visibility API will also be affected (ie, the page will think it is visible when it is only warmed up).

I am confident that a solution to this will also be a solution to bug 1395735.

I might be able to hook up some infrastructure that will allow us to cause the layers to paint and upload from the content process without actually activating the DocShell, which should avoid these problems. I'm not sure this is possible, but I'm starting there for investigation.
Flags: needinfo?(mconley)
See Also: → bug 1395735
(Assignee)

Comment 18

2 years ago
I'll note that if it's not possible to fix this bug before 57 needs to freeze / uplift, we can disable the warming API via this pref: http://searchfox.org/mozilla-central/rev/f2a1911ad310bf8651f342d719e4f4ca0a7b9bfb/browser/app/profile/firefox.js#1569

So we have a killswitch - the worst case scenario here is that we put off the performance improvements from bug 1385453 a release until we can get these things fixed.
Status: NEW → ASSIGNED
Iteration: --- → 57.3 - Sep 19
Flags: qe-verify+
Whiteboard: [testcoverage] → [photon-performance] [testcoverage]

Updated

2 years ago
Duplicate of this bug: 1397349
(Assignee)

Comment 20

2 years ago
I investigated this yesterday and today, and here are my findings.

The reason that the video is playing (and the Page Visibility API is telling the page that the tab is visible) is because we're setting the DocShell to be active. So we have to avoid doing that, and yet still need to get the layers for the page to the compositor.

I talked to billm, and as it turns out, this is actually possible. We need to first make the TabChild PuppetWidget visible, via TabChild::MakeVisible, then activate the PresShell, and then trigger a paint.

That appears to fix the problem, however, it puts us in a kind of confused state now - the way I've hacked up my PoC patch, the tab goes into STATE_LOADING without having set the docShell as active, and then when we select the warmed tab, we transition from STATE_LOADING to STATE_LOADED, and the docShell is never activated.

So we'll need to do some re-organizing here in the async tab switcher to account for this middle-ground warming state.

I really, really don't want to do that so close to uplift. The async tab switcher is pretty fragile code. I think the safest option, given the remaining time, is to disable tab warming for now, and then revisit this plan after 57 uplifts.

I'm pretty bummed out by that - the Telemetry showed that this gave a significant performance boost to tab switching. However, I think that the risks of attempting to jam this optimization in with slapped together regression fixes greatly outweigh the gains, considering what will be required.

So I think I'm going to post a patch disabling tab warming for now, and then file a new bug to change how the warming mechanism works.
Comment hidden (mozreview-request)

Comment 22

2 years ago
mozreview-review
Comment on attachment 8905223 [details]
Bug 1394455 - Disable tab warming until bug 1397426 is fixed. :(

https://reviewboard.mozilla.org/r/177012/#review182238

Sad r+.
Attachment #8905223 - Flags: review?(florian) → review+

Comment 23

2 years ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/515aab24cac4
Disable tab warming until bug 1397426 is fixed. :( r=florian
https://hg.mozilla.org/mozilla-central/rev/515aab24cac4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1395735
Comment 0 steps are not reproducible anymore, since comment 24 landed disabling tab warmup. 
Marking the bug as verified on: 57.0a1 20170910220126  - Windows 7x32, Mac OSX 10.12, Ubuntu 16.04
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+
(Reporter)

Updated

8 months ago
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.