When switching profiles screen sharing is supposed to stop, but it only works when there is an active notification in the tray, so this feature is broken as well.

This sounds like a real bug. We should not lose track of screensharing sessions.

Albert, do you know who owns the screensharing notification?

jdufault@ do you still own the screen sharing integration or has that been handed off to someone else?

I question the "medium" classification here.  Definitely sounds like something we should fix, but this seems more like a privacy concern of "oops I didn't realize I was still sharing" rather than anything an attacker could actively exploit.

I'm guessing the simplest solution is just to kill all sessions on clicking stop.

Honestly, I'd argue for preventing multiple sessions in the first place as the effort required to support multiples properly (disclosing which apps are sharing in the notification, etc.) seem prohibitively expensive relative to the value.

Looking at the code both approaches abodenha@ outline seem straight-forward, though if we prevent multiple sessions the code to do so will be more of a hack, since it will be doing validation at the UI layer.

I'll upload a CL to kill all sessions when clicking stop.

https://cs.chromium.org/chromium/src/ash/system/screen_security/screen_tray_item.cc?l=116

Thanks, jdufault@!

jorgelo@ since this is classified "medium" should we aim to merge this to 66 or is 67 acceptable?

CL at https://chromium-review.googlesource.com/c/chromium/src/+/993761

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/200d0cedc373ac353383b1bf8274d4c0441aec5b

commit 200d0cedc373ac353383b1bf8274d4c0441aec5b
Author: Jacob Dufault <jdufault@google.com>
Date: Fri Apr 13 01:06:55 2018

cros: Stop all screen share sessions if "Stop" button is pressed.

Previously, not all screen share sessions would stop and the shelf UI would not
indicate that there was a screen share session.

Bug:  826041 
Change-Id: Ia32bf38e6a0dc158037d9d482e5b2bba6edf17f5
Reviewed-on: https://chromium-review.googlesource.com/993761
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550453}
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item.cc
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item.h
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item_unittest.cc

Let me know if this requires a merge.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/200d0cedc373ac353383b1bf8274d4c0441aec5b

commit 200d0cedc373ac353383b1bf8274d4c0441aec5b
Author: Jacob Dufault <jdufault@google.com>
Date: Fri Apr 13 01:06:55 2018

cros: Stop all screen share sessions if "Stop" button is pressed.

Previously, not all screen share sessions would stop and the shelf UI would not
indicate that there was a screen share session.

Bug:  826041 
Change-Id: Ia32bf38e6a0dc158037d9d482e5b2bba6edf17f5
Reviewed-on: https://chromium-review.googlesource.com/993761
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550453}
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item.cc
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item.h
[modify] https://crrev.com/200d0cedc373ac353383b1bf8274d4c0441aec5b/ash/system/screen_security/screen_tray_item_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728

commit 38ffe94eb5ee5b175c9d579e1457ba9fc46b4728
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue May 08 06:36:33 2018

Stop all screen share in notification controller.

The CL https://crrev.com/c/993761 made a change to stop all screen
capture/share sessions in ScreenTrayItem when one of the sessions is
stopped.

Unfortunately the fix does not work in UnifiedSystemTray as it's done
in SystemTrayItem. Also, the behaviors from the notification stop button
and the tray stop button were different.

This CL moves the logic from ScreenCaptureItem to
ScreenSecurityNotificationController.

Ideally we should fix Screen{Capture,Share}Observer interfaces, or
prevent multiple share sessions in the first place but it is not trivial
as noted in  https://crbug.com/826041#c6 .

TEST=ScreenCaptureTest,ScreenShareTest
BUG= 826041 

Change-Id: I3f559b95d99bfd4c04a1241e2e86de2718f1dc29
Reviewed-on: https://chromium-review.googlesource.com/1046287
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556710}
[modify] https://crrev.com/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728/ash/system/screen_security/screen_security_notification_controller.cc
[modify] https://crrev.com/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728/ash/system/screen_security/screen_security_notification_controller.h
[modify] https://crrev.com/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728/ash/system/screen_security/screen_tray_item.cc
[modify] https://crrev.com/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728/ash/system/screen_security/screen_tray_item.h
[modify] https://crrev.com/38ffe94eb5ee5b175c9d579e1457ba9fc46b4728/ash/system/screen_security/screen_tray_item_unittest.cc

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot