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/993761Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550453}

ash/system/screen_security/screen_tray_item.cc

@@ -100,8 +100,11 @@ ScreenTrayItem::ScreenTrayItem(SystemTray* system_tray, UmaType uma_type)
 ScreenTrayItem::~ScreenTrayItem() = default;
 
 void ScreenTrayItem::SetIsStarted(bool is_started) {
+  // If |is_started| is false then that means the screen share has already
+  // stopped; however, we still have to run the stop callbacks because we do not
+  // know which screen share session ended.
   if (!is_started)
-    stop_callback_.Reset();
+    Stop();
   is_started_ = is_started;
 }
 
@@ -113,7 +116,7 @@ void ScreenTrayItem::Update() {
 }
 
 void ScreenTrayItem::Start(base::OnceClosure stop_callback) {
-  stop_callback_ = std::move(stop_callback);
+  stop_callbacks_.emplace_back(std::move(stop_callback));
   is_started_ = true;
 
   if (tray_view_)
@@ -127,10 +130,11 @@ void ScreenTrayItem::Stop() {
   is_started_ = false;
   Update();
 
-  if (stop_callback_.is_null())
-    return;
-
-  std::move(stop_callback_).Run();
+  for (base::OnceClosure& callback : stop_callbacks_) {
+    if (callback)
+      std::move(callback).Run();
+  }
+  stop_callbacks_.clear();
 }
 
 views::View* ScreenTrayItem::CreateTrayView(LoginStatus status) {

ash/system/screen_security/screen_tray_item.h

@@ -101,7 +101,10 @@ class ASH_EXPORT ScreenTrayItem : public SystemTrayItem {
   void OnDefaultViewDestroyed() override;
 
  private:
-  base::OnceClosure stop_callback_;
+  // There can be multiple cast sessions at the same time. If the user hits the
+  // stop button, stop all sessions since there is not a good UI to distinguish
+  // between the different sessions.
+  std::vector<base::OnceClosure> stop_callbacks_;
   tray::ScreenTrayView* tray_view_;
   tray::ScreenStatusView* default_view_;
   bool is_started_;

ash/system/screen_security/screen_tray_item_unittest.cc

@@ -38,12 +38,13 @@ void ClickViewCenter(views::View* view) {
 
 class ScreenTrayItemTest : public AshTestBase {
  public:
-  ScreenTrayItemTest() : tray_item_(NULL), stop_callback_hit_count_(0) {}
+  ScreenTrayItemTest() {}
   ~ScreenTrayItemTest() override = default;
 
   ScreenTrayItem* tray_item() { return tray_item_; }
   void set_tray_item(ScreenTrayItem* tray_item) { tray_item_ = tray_item; }
 
+  void reset_stop_callback_hit_count() { stop_callback_hit_count_ = 0; }
   int stop_callback_hit_count() const { return stop_callback_hit_count_; }
 
   void StartSession() {
@@ -56,8 +57,8 @@ class ScreenTrayItemTest : public AshTestBase {
   void StopCallback() { stop_callback_hit_count_++; }
 
  private:
-  ScreenTrayItem* tray_item_;
-  int stop_callback_hit_count_;
+  ScreenTrayItem* tray_item_ = nullptr;
+  int stop_callback_hit_count_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenTrayItemTest);
 };
@@ -119,48 +120,58 @@ TEST_F(ScreenShareTest, StartAndStop) {
   TestStartAndStop(this);
 }
 
-void TestNotificationStartAndStop(ScreenTrayItemTest* test,
-                                  const base::Closure& start_function,
-                                  const base::Closure& stop_function) {
+void TestNotificationStartAndStop(
+    ScreenTrayItemTest* test,
+    const std::vector<base::RepeatingClosure>& start_functions,
+    const base::RepeatingClosure& stop_function) {
+  test->reset_stop_callback_hit_count();
   ScreenTrayItem* tray_item = test->tray_item();
   EXPECT_FALSE(tray_item->is_started());
 
-  start_function.Run();
+  for (const auto& start : start_functions) {
+    start.Run();
     EXPECT_TRUE(tray_item->is_started());
+  }
 
-  // The stop callback shouldn't be called because we stopped
-  // through the notification system.
+  // The stop callback is called even if the notification fires. There is a
+  // non-test ScreenTrayItem which is also called which is why the stop callback
+  // hit count is 2x the expected value.
   stop_function.Run();
   EXPECT_FALSE(tray_item->is_started());
-  EXPECT_EQ(0, test->stop_callback_hit_count());
+  EXPECT_EQ(static_cast<int>(start_functions.size()) * 2,
+            test->stop_callback_hit_count());
 }
 
 TEST_F(ScreenCaptureTest, NotificationStartAndStop) {
-  base::Closure start_function = base::Bind(
+  base::RepeatingClosure start_function = base::BindRepeating(
       &SystemTrayNotifier::NotifyScreenCaptureStart,
       base::Unretained(Shell::Get()->system_tray_notifier()),
       base::Bind(&ScreenTrayItemTest::StopCallback, base::Unretained(this)),
       base::UTF8ToUTF16(kTestScreenCaptureAppName));
 
-  base::Closure stop_function =
-      base::Bind(&SystemTrayNotifier::NotifyScreenCaptureStop,
+  base::RepeatingClosure stop_function = base::BindRepeating(
+      &SystemTrayNotifier::NotifyScreenCaptureStop,
       base::Unretained(Shell::Get()->system_tray_notifier()));
 
-  TestNotificationStartAndStop(this, start_function, stop_function);
+  TestNotificationStartAndStop(this, {start_function}, stop_function);
+  TestNotificationStartAndStop(this, {start_function, start_function},
+                               stop_function);
 }
 
 TEST_F(ScreenShareTest, NotificationStartAndStop) {
-  base::Closure start_func = base::Bind(
+  base::RepeatingClosure start_function = base::BindRepeating(
       &SystemTrayNotifier::NotifyScreenShareStart,
       base::Unretained(Shell::Get()->system_tray_notifier()),
       base::Bind(&ScreenTrayItemTest::StopCallback, base::Unretained(this)),
       base::UTF8ToUTF16(kTestScreenShareHelperName));
 
-  base::Closure stop_func =
-      base::Bind(&SystemTrayNotifier::NotifyScreenShareStop,
+  base::RepeatingClosure stop_function = base::BindRepeating(
+      &SystemTrayNotifier::NotifyScreenShareStop,
       base::Unretained(Shell::Get()->system_tray_notifier()));
 
-  TestNotificationStartAndStop(this, start_func, stop_func);
+  TestNotificationStartAndStop(this, {start_function}, stop_function);
+  TestNotificationStartAndStop(this, {start_function, start_function},
+                               stop_function);
 }
 
 void TestSystemTrayInteraction(ScreenTrayItemTest* test) {