1445732 - Muting tab causes lazy browser premature insertion

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Oriol Brufau [:Oriol]]

1. Go to https://demo.bitmovin.com/public/firefox/av1/
2. Play the video and mute the tab
3. Switch to some other tab
4. Make sure automatic session restore is enabled
5. Restart the browser

Result:

[bug 1345098] Lazy browser prematurely inserted via 'mute' property access:
getter@chrome://browser/content/tabbrowser.js:1999:37
toggleMuteAudio@chrome://browser/content/tabbrowser.xml:1932:15
restoreTab@resource:///modules/sessionstore/SessionStore.jsm:3744:7
restoreTabs@resource:///modules/sessionstore/SessionStore.jsm:3686:9
ssi_restoreWindow@resource:///modules/sessionstore/SessionStore.jsm:3503:7
ssi_restoreWindows@resource:///modules/sessionstore/SessionStore.jsm:3627:5
initializeWindow@resource:///modules/sessionstore/SessionStore.jsm:1178:11
onBeforeBrowserWindowShown/<@resource:///modules/sessionstore/SessionStore.jsm:1329:9
promise callback*onBeforeBrowserWindowShown@resource:///modules/sessionstore/SessionStore.jsm:1314:5
ssi_observe@resource:///modules/sessionstore/SessionStore.jsm:765:9
onLoad@chrome://browser/content/browser.js:1326:5
onload@chrome://browser/content/browser.xul:1:1

Premature insertions are not awful for performance, they also cause various kinds of problems like bug 1418009 and bug 1418038.

Regression window: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=be7a5a989cb3dbabce6a1bb78aad2d05baa3589a&tochange=7a630a4add1f5c974c95ab829658d49653535c36

I guess it's bug 1387976.

Comment 1

[Oriol Brufau [:Oriol]]

I meant "not *only* awful".

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Abdoulaye, can you look in to this?

Comment 3

[Oriol Brufau [:Oriol]]

I believe this is caused by a merge conflict between bug 1387976 and bug 1443849.

In https://hg.mozilla.org/integration/autoland/rev/de798abb5ac7 from bug 1387976 there was a this.linkedPanel check that avoided lazy browser premature insertions, but this was lost in https://hg.mozilla.org/integration/autoland/rev/7a630a4add1f from bug 1443849.

Comment 4

[Abdoulaye O. LY]

Thank you Oriol for pointing this out. It was indeed a merge conflitct. 

Yes Jared, Please assign me this. I will quickly land a patch for this.

Comment 5

[Dão Gottwald [:dao]]

(In reply to Oriol Brufau [:Oriol] from comment #3)
> In https://hg.mozilla.org/integration/autoland/rev/de798abb5ac7 from bug
> 1387976 there was a this.linkedPanel check that avoided lazy browser
> premature insertions, but this was lost in
> https://hg.mozilla.org/integration/autoland/rev/7a630a4add1f from bug
> 1443849.

That's not "from bug 1443849", the sheriffs did that. No idea why this is labeled "part 4" of my patch series which it clearly wasn't. Also not sure why this was a conflict in the first place. My patches didn't even touch toggleMuteAudio. :|

Comment 6

[Abdoulaye O. LY]

> Also not sure why this was a conflict in the first place. My patches didn't even touch toggleMuteAudio

By looking closely to the issue, I am a little bit confused too. I can't see any conflicts created from your patches. In addition, all the code from my patch disappeared.

Comment 7

[Oriol Brufau [:Oriol]]

I have no idea other than https://hg.mozilla.org/integration/autoland/rev/7a630a4add1f says this:

    Bug 1443849 - Part 4: Fix broken tabbrowser.xml after merge conflict. CLOSED TREE

The author is Sebastian Hengst so I guess you can ask him.

But I don't really see how this matters, wouldn't fixing tabbrowser.xml so that it behaves like it was supposed to in bug 1387976 do the trick?

Comment 8

[Abdoulaye O. LY]

Attachment: Reapplied changsets de798abb5ac7 and a049e86626d4 (bug 1387976) for lazy-tab muted state to persist across browser restarts

Comment 9

[Oriol Brufau [:Oriol]]

Abdoulaye, it seems that most parts of your patch already landed, e.g. see your test:
https://searchfox.org/mozilla-central/source/browser/base/content/test/tabs/browser_bug_1387976_restore_lazy_tab_browser_muted_state.js

So you should use "hg pull" and update, and then attach a patch only with the parts that magically disappeared.

Comment 10

[Abdoulaye O. LY]

(In reply to Oriol Brufau [:Oriol] from comment #9)
> Abdoulaye, it seems that most parts of your patch already landed, e.g. see
> your test:
> https://searchfox.org/mozilla-central/source/browser/base/content/test/tabs/
> browser_bug_1387976_restore_lazy_tab_browser_muted_state.js
> 
> So you should use "hg pull" and update, and then attach a patch only with
> the parts that magically disappeared.

Oops I forgot to update after a pull the last time (too used to git :)).

Thanks

Comment 11

[Abdoulaye O. LY]

Attachment: Avoid invoking mute and unmute methods for lazy-browsers to prevent premature insertion

Comment 12

[Oriol Brufau [:Oriol]]

Comment on attachment 8959903 [details] [diff] [review]
Avoid invoking mute and unmute methods for lazy-browsers to prevent premature insertion

Thanks! I guess :jaws should review this

Comment 13

[Oriol Brufau [:Oriol]]

Comment on attachment 8959351 [details] [diff] [review]
Reapplied changsets de798abb5ac7 and a049e86626d4 (bug 1387976) for lazy-tab muted state to persist across browser restarts

and this previous patch is obsolete.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/006ec77ea08a
Avoid invoking mute and unmute methods for lazy-browsers to prevent premature insertion. r=jaws

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/006ec77ea08a

Comment 16

[Sajedul Islam]

I have reproduced this bug with Nightly 61.0a1 (2018-03-14) on Windows 10 , 64 Bit ! 

This bug's fix is Verified with latest Nightly !

Build   ID    20180327105613
User Agent    Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0

[bugday-20180321]