1325049 - Global sharing indicator doesn't work correctly with e10s multi

Status: VERIFIED FIXED

(Firefox :: Site Permissions, defect, P1)

Description

[Florian Quèze [:florian]]

Steps to reproduce:
1. Enable e10s multi (set dom.ipc.processCount to 2 in about:config)
2. Load https://people-mozilla.org/~fqueze2/webrtc/ and share the camera.
3. Load http://mozilla.github.io/webrtc-landing/gum_test.html in a second tab, and share the camera.
4. Click the "Stop" button.
5. (Mac) Click the Camera icon in the menubar.

Expected result:
The menu should offer to control sharing for the first tab.

Actual result:
The menu has only one disable item, saying "Sharing Camera with 0 tabs".


This is because when receiving the "webrtc:UpdatingIndicators" message from any content process, webrtcUI.jsm clears the webrtcUI._streams array that contained info about active streams from all processes: http://searchfox.org/mozilla-central/rev/cc2a84852bd4e6f6d8d4d5b17b8382bb5d005749/browser/modules/webrtcUI.jsm#217

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

E10s multi is on by default in Nightly.  Who can work on this?  Thanks

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

Florian is already looking at it, but we forgot to assign it.

Comment 3

[Florian Quèze [:florian]]

Attachment: Fix

Comment 4

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9b278db557d

Comment 5

[Florian Quèze [:florian]]

(In reply to Randell Jesup [:jesup] from comment #1)
> E10s multi is on by default in Nightly.  Who can work on this?  Thanks

It took me more time than I expected to attach a 'working' patch, because it took me a while to figure out how to reliably have 2 different content processes within my test.

From seeing the try results from comment 4, I think I'll need help to figure out the debug-only failures of my new test:
Assertion failure: [GFX1]: Texture deallocated too late during shutdown, at /home/worker/workspace/build/src/gfx/2d/Logging.h:518

Comment 6

[Florian Quèze [:florian]]

Attachment: Fix v2

Fixed the eslint error, and disabled the test on e10s && debug (filed bug 1347625 to cover this assertion).

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8847597 [details] [diff] [review]
Fix

Review of attachment 8847597 [details] [diff] [review]:
-----------------------------------------------------------------

(There should probably be a helper function to write tests that load tabs in different processes..)

Comment 8

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=65825cc1ac5c

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8847597 [details] [diff] [review]
Fix

(gah, mid-air)

Comment 10

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4e0318c18c37fc2fdf89cdd7cdf2398af0c7b56
Bug 1325049 - Fix the global webrtc sharing indicator to work with multiple content processes, r=felipe.

Comment 11

[Carsten Book [:Tomcat]]

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

Comment 12

[Florian Quèze [:florian]]

Comment on attachment 8847680 [details] [diff] [review]
Fix v2

I heard that we may turn on e10s-multi for some users in 54, if so I think we should uplift this fix for them.

Approval Request Comment
[Feature/Bug causing the regression]: e10s-multi
[User impact if declined]: WebRTC sharing indicators won't be reliable for users sharing devices in tabs running in different content processes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: Not really required, but would be nice if QE could do some exploration around this. STRs are in the initial bug description.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: Acceptable for aurora in my option.
[Why is the change risky/not risky?]: The risk is reduced by relatively extensive test coverage for this area of the code.
[String changes made/needed]: none

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8847680 [details] [diff] [review]
Fix v2

Support e10s-multi experiment. Fix a WebRTC sharing indicator issue. Aurora54+.

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

Comment 14

[ovidiu boca[:Ovidiu]]

Attachment: Recording #7.mp4

Hi, I tested this issue on Mac OS X 10.12 with FF Nightly 55.0a1(2017-03-19) and following the scenario from the description the issue is no longer reproducible. 

I did some exploratory and I found a bug following this scenario: 

You need to have 2 monitors to reproduce this. 

STR:
1.  Open Firefox on screen no.1
2. Load https://people-mozilla.org/~fqueze2/webrtc/ and share the camera.
3. Load http://mozilla.github.io/webrtc-landing/gum_test.html in a second tab, and share the camera.
4. Click the "Stop" button.
5. Click the Camera icon in the menubar.(from the second screen)

Expected result:
The menu should offer to control sharing for the first tab.

Actual result:

The control sharing for the first tab is open for ~1 sec and after that disappears. 

Please see the video recorder for a better understanding. 

Florian, please take a look at this, thanks.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d9a7778fd9ab

Comment 16

[Florian Quèze [:florian]]

(In reply to ovidiu boca[:Ovidiu] from comment #14)
> Created attachment 8849018 [details]
> Recording #7.mp4
> 
> Hi, I tested this issue on Mac OS X 10.12 with FF Nightly 55.0a1(2017-03-19)
> and following the scenario from the description the issue is no longer
> reproducible. 
> 
> I did some exploratory and I found a bug following this scenario: 
> 
> You need to have 2 monitors to reproduce this. 
> 
> STR:
> 1.  Open Firefox on screen no.1
> 2. Load https://people-mozilla.org/~fqueze2/webrtc/ and share the camera.
> 3. Load http://mozilla.github.io/webrtc-landing/gum_test.html in a second
> tab, and share the camera.
> 4. Click the "Stop" button.
> 5. Click the Camera icon in the menubar.(from the second screen)
> 
> Expected result:
> The menu should offer to control sharing for the first tab.
> 
> Actual result:
> 
> The control sharing for the first tab is open for ~1 sec and after that
> disappears. 
> 
> Please see the video recorder for a better understanding. 
> 
> Florian, please take a look at this, thanks.

Please file a new bug for this, thanks!

From looking at the recording, it seems like the issue here is caused by opening the sharing indicator from the menu bar in the second screen. Can you reproduce if in the second tab you load any other page (ie. don't start a video stream from a second tab)?

Comment 17

[ovidiu boca[:Ovidiu]]

Hi Florian, 
I filled a bug like you suggested, please see bug 1349141.

"Can you reproduce if in the second tab you load any other page (ie. don't start a video stream from a second tab)?"

I tried this scenario and I'm still able to reproduce it. 

What I also saw is that this issue is reproducible only on Mac OS X 10.12. I tried to reproduce the scenario from bug description, scenario from bug 1349141 and I couldn't.

Comment 18

[Andrei Vaida [:avaida]]

Let's make sure this works as expected on Beta 54.

Comment 19

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 52.0a2 (2016.12.21) on Mac OS 10.11.
I can confirm this issue is fixed, I verified using Firefox 54.0b4 and 55.0a1 on Mac OS X 10.11.