1649032 - If the user closes the WebRTC sharing indicator, all streams should close

Status: RESOLVED FIXED

(Firefox :: Site Permissions, task, P2)

Description

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

The indicator isn't easy to close, but it can occur (mainly via OS controls for the indicator window). If that occurs, we should stop all of the ongoing streams. This is consistent with Chromium.

Comment 1

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

Attachment: Bug 1649032 - If the user manages to close the WebRTC indicator, close all of the active streams. r?pbz!

Comment 2

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

Attachment: Bug 1649032 - Add a test that ensures that we stop all streams if the WebRTC indicator is closed. r?pbz!

Depends on D82989

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8462c37bef1
If the user manages to close the WebRTC indicator, close all of the active streams. r=pbz
https://hg.mozilla.org/integration/autoland/rev/ef80a597db37
Add a test that ensures that we stop all streams if the WebRTC indicator is closed. r=pbz

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (bug 1649032) for leakcheck failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&fromchange=bc20566bf7670cd7ecc1956af3cc03cd98abf91e&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Ctest-macosx1014-64%2Fdebug-mochitest-browser-chrome-e10s%2Cbc12&tochange=162a3ca59e61c6e1638a67f7454fd5d1099560a7&selectedTaskRun=dfLYW7bZRBO_DRmoZ0-D2Q.0

Backout link: https://hg.mozilla.org/integration/autoland/rev/162a3ca59e61c6e1638a67f7454fd5d1099560a7

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309902802&repo=autoland&lineNumber=15600

...
[task 2020-07-15T21:21:34.199Z] 21:21:34     INFO - TEST-INFO | leakcheck | default leaked 1 nsZipArchive
[task 2020-07-15T21:21:34.199Z] 21:21:34     INFO - TEST-INFO | leakcheck | default leaked 1 nsZipReaderCache
[task 2020-07-15T21:21:34.199Z] 21:21:34     INFO - TEST-INFO | leakcheck | default leaked 1 xpc::CompartmentPrivate
[task 2020-07-15T21:21:34.208Z] 21:21:34     INFO - TEST-UNEXPECTED-FAIL | leakcheck | default 4940957 bytes leaked (AbstractThread, AbstractWatcher, AnimationTimeline, AsyncFreeSnowWhite, AtomSet, ...)
[task 2020-07-15T21:21:34.209Z] 21:21:34     INFO - 
[task 2020-07-15T21:21:34.209Z] 21:21:34     INFO - runtests.py | Running tests: end.

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

The following also seems to start with the backed out changes:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=309903739&repo=autoland&lineNumber=26210

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c1c5eddf7af
If the user manages to close the WebRTC indicator, close all of the active streams. r=pbz
https://hg.mozilla.org/integration/autoland/rev/400275fe8a1d
Add a test that ensures that we stop all streams if the WebRTC indicator is closed. r=pbz

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/3c1c5eddf7af
https://hg.mozilla.org/mozilla-central/rev/400275fe8a1d

Comment 8

[Daniel Bodea [:danibodea]]

When the user closes the Global Sharing Overlay by clicking the "x" button after hovering over its taskbar entry, all the permissions are being removed (microphone, camera and screen sharing).

This is definitely NOT like the behavior seen in the Chrome release or Chromium browser versions. Chrome browsers only remove the screen sharing permission when their overlay is being closed and leaves the microphone and camera sharing permissions as allowed.

This makes for a different issue to emerge: when the user is in a meet.jit.si meeting/conference while sharing screen and he closes the Global Sharing Overlay altogether, he will notice that the all the permissions are removed, the camera sharing permission is re-requested, but the audio/microphone sharing permission is not. A page refresh is required to re-enter the conference correctly.

I do not see the benefit of this change. Can you explain why this is necessary?

Comment 9

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

Hello dbodea,

(In reply to Bodea Daniel [:danibodea] from comment #8)

When the user closes the Global Sharing Overlay by clicking the "x" button after hovering over its taskbar entry, all the permissions are being removed (microphone, camera and screen sharing).

Yes, this was the intended behaviour.

This is definitely NOT like the behavior seen in the Chrome release or Chromium browser versions. Chrome browsers only remove the screen sharing permission when their overlay is being closed and leaves the microphone and camera sharing permissions as allowed.

This is because Chromium-based browsers only show that indicator when sharing the screen - whereas, our indicator also includes display of the microphone and camera streaming state.

I do not see the benefit of this change. Can you explain why this is necessary?

Sure, no problem. We very much want the user to associate the indicator with the sharing of all streams - display, microphone and camera. When that indicator is not open, we don't want any streams to be running. We've given the user the ability to minimize and move the indicator to get it out of the way, but we don't want to make it easy for the user to be able to sever the association between the indicator and the sharing of the streams, because we want to enforce the association. We want users to know that when the indicator is around, streams are being shared, and if the indicator is not around, no streams are being shared.