[Harmony Cast Dialog] Disable the stop button when clicked

Disable the "Stop" button after it's clicked, so that the user doesn't
click on it repeatedly. The button becomes enabled again when there is
a model update, or if the user clicks on a sink.

When the stop button is clicked for a sink, the sink enters DISCONNECTING
state for that dialog, until its route is removed.

Bug: 848975
Change-Id: I0acde957053a445fce6bb24cdb54c5e30dd45587
Reviewed-on: https://chromium-review.googlesource.com/1123326
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577665}

chrome/browser/ui/media_router/ui_media_sink.h

@@ -17,10 +17,14 @@ enum class UIMediaSinkState {
   // Sink is available to be Cast to.
   AVAILABLE,
   // Sink is starting a new Casting activity. A sink temporarily enters this
-  // state when transitioning from AVAILABLE to CONNECTED (or to ERROR_STATE).
+  // state when transitioning from AVAILABLE to CONNECTED.
   CONNECTING,
   // Sink has a media route.
   CONNECTED,
+  // Sink is still connected but is in the process of disconnecting. A sink
+  // temporarily enters this state when transitioning from CONNECTED to
+  // AVAILABLE.
+  DISCONNECTING,
   // Sink is disconnected/cached (not available right now).
   UNAVAILABLE
 };

chrome/browser/ui/views/media_router/cast_dialog_sink_button.cc

@@ -76,7 +76,8 @@ std::unique_ptr<views::View> CreateSecondaryIconForSink(
         CastDialogSinkButton::kSecondaryIconSize, gfx::kChromeIconGrey));
     icon_view->SetTooltipText(base::UTF8ToUTF16(sink.issue->info().title));
     return icon_view;
-  } else if (sink.state == UIMediaSinkState::CONNECTED) {
+  } else if (sink.state == UIMediaSinkState::CONNECTED ||
+             sink.state == UIMediaSinkState::DISCONNECTING) {
     auto icon_view = std::make_unique<views::ImageView>();
     icon_view->SetImage(CreateVectorIcon(
         views::kMenuCheckIcon, CastDialogSinkButton::kSecondaryIconSize,
@@ -167,7 +168,9 @@ std::unique_ptr<views::InkDrop> CastDialogSinkButton::CreateInkDrop() {
 }
 
 base::string16 CastDialogSinkButton::GetActionText() const {
-  return l10n_util::GetStringUTF16(sink_.state == UIMediaSinkState::CONNECTED
+  return l10n_util::GetStringUTF16(sink_.state == UIMediaSinkState::CONNECTED ||
+                                           sink_.state ==
+                                               UIMediaSinkState::DISCONNECTING
                                        ? IDS_MEDIA_ROUTER_STOP_CASTING_BUTTON
                                        : IDS_MEDIA_ROUTER_START_CASTING_BUTTON);
 }

chrome/browser/ui/views/media_router/cast_dialog_sink_button_unittest.cc

@@ -37,13 +37,18 @@ void CheckActionTextForState(UIMediaSinkState state,
 }  // namespace
 
 TEST_F(CastDialogSinkButtonTest, GetActionText) {
-  // TODO(crbug.com/826089): Determine what the text should be for other states.
   CheckActionTextForState(
       UIMediaSinkState::AVAILABLE,
       l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_START_CASTING_BUTTON));
+  CheckActionTextForState(
+      UIMediaSinkState::CONNECTING,
+      l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_START_CASTING_BUTTON));
   CheckActionTextForState(
       UIMediaSinkState::CONNECTED,
       l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_STOP_CASTING_BUTTON));
+  CheckActionTextForState(
+      UIMediaSinkState::DISCONNECTING,
+      l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_STOP_CASTING_BUTTON));
 }
 
 }  // namespace media_router

chrome/browser/ui/views/media_router/cast_dialog_view.cc

@@ -123,7 +123,8 @@ int CastDialogView::GetDialogButtons() const {
 
 bool CastDialogView::IsDialogButtonEnabled(ui::DialogButton button) const {
   return !sink_buttons_.empty() &&
-         GetSelectedSink().state != UIMediaSinkState::CONNECTING;
+         GetSelectedSink().state != UIMediaSinkState::CONNECTING &&
+         GetSelectedSink().state != UIMediaSinkState::DISCONNECTING;
 }
 
 views::View* CastDialogView::CreateExtraView() {

chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc

@@ -134,6 +134,10 @@ class CastDialogViewTest : public ChromeViewsTestBase {
     return dialog_->sources_menu_runner_for_test();
   }
 
+  views::LabelButton* main_button() {
+    return dialog_->GetDialogClientView()->ok_button();
+  }
+
   content::TestBrowserThreadBundle test_thread_bundle_;
   std::unique_ptr<views::Widget> anchor_widget_;
   MockCastDialogController controller_;
@@ -308,4 +312,33 @@ TEST_F(CastDialogViewTest, SwitchToNoDeviceView) {
   EXPECT_FALSE(dialog_->GetDialogClientView()->ok_button()->enabled());
 }
 
+TEST_F(CastDialogViewTest, ReenableStopButtonWithUpdate) {
+  std::vector<UIMediaSink> media_sinks = {CreateConnectedSink()};
+  media_sinks[0].state = UIMediaSinkState::DISCONNECTING;
+  CastDialogModel model = CreateModelWithSinks(media_sinks);
+  InitializeDialogWithModel(model);
+  // The main button should be disabled while the sink is disconnecting.
+  EXPECT_FALSE(main_button()->enabled());
+
+  // Updating the model should re-enable the main button.
+  media_sinks[0].state = UIMediaSinkState::AVAILABLE;
+  media_sinks[0].route_id = "";
+  model.set_media_sinks(std::move(media_sinks));
+  dialog_->OnModelUpdated(model);
+  EXPECT_TRUE(main_button()->enabled());
+}
+
+TEST_F(CastDialogViewTest, ReenableStopButtonWithSinkSelection) {
+  std::vector<UIMediaSink> media_sinks = {CreateConnectedSink(),
+                                          CreateAvailableSink()};
+  media_sinks[0].state = UIMediaSinkState::DISCONNECTING;
+  InitializeDialogWithModel(CreateModelWithSinks(media_sinks));
+  // The main button should be disabled while the sink is disconnecting.
+  EXPECT_FALSE(main_button()->enabled());
+
+  // Selecting another sink should re-enable the main button.
+  SelectSinkAtIndex(1);
+  EXPECT_TRUE(main_button()->enabled());
+}
+
 }  // namespace media_router

chrome/browser/ui/views/media_router/media_router_views_ui.cc

@@ -56,7 +56,11 @@ void MediaRouterViewsUI::StartCasting(const std::string& sink_id,
 }
 
 void MediaRouterViewsUI::StopCasting(const std::string& route_id) {
-  TerminateRoute(route_id);
+  terminating_route_id_ = route_id;
+  // |route_id| may become invalid after UpdateSinks(), so we cannot refer to
+  // |route_id| below this line.
+  UpdateSinks();
+  TerminateRoute(terminating_route_id_.value());
 }
 
 std::vector<MediaSinkWithCastModes> MediaRouterViewsUI::GetEnabledSinks()
@@ -84,6 +88,13 @@ void MediaRouterViewsUI::OnRoutesUpdated(
     const std::vector<MediaRoute>& routes,
     const std::vector<MediaRoute::Id>& joinable_route_ids) {
   MediaRouterUIBase::OnRoutesUpdated(routes, joinable_route_ids);
+  if (terminating_route_id_ &&
+      std::find_if(
+          routes.begin(), routes.end(), [this](const MediaRoute& route) {
+            return route.media_route_id() == terminating_route_id_.value();
+          }) == routes.end()) {
+    terminating_route_id_.reset();
+  }
   UpdateSinks();
 }
 
@@ -116,7 +127,10 @@ UIMediaSink MediaRouterViewsUI::ConvertToUISink(
   if (route) {
     ui_sink.status_text = base::UTF8ToUTF16(route->description());
     ui_sink.route_id = route->media_route_id();
-    ui_sink.state = UIMediaSinkState::CONNECTED;
+    ui_sink.state = terminating_route_id_ && route->media_route_id() ==
+                                                 terminating_route_id_.value()
+                        ? UIMediaSinkState::DISCONNECTING
+                        : UIMediaSinkState::CONNECTED;
   } else {
     ui_sink.state = current_route_request() &&
                             sink.sink.id() == current_route_request()->sink_id

chrome/browser/ui/views/media_router/media_router_views_ui.h

@@ -34,6 +34,7 @@ class MediaRouterViewsUI : public MediaRouterUIBase,
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, NotifyObserver);
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, RemovePseudoSink);
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, ConnectingState);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, DisconnectingState);
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, AddAndRemoveIssue);
 
   // MediaRouterUIBase:
@@ -63,6 +64,10 @@ class MediaRouterViewsUI : public MediaRouterUIBase,
   // is selected and casting starts.
   base::Optional<MediaSink::Id> local_file_sink_id_;
 
+  // This value is set when the UI requests a route to be terminated, and gets
+  // reset when the route is removed.
+  base::Optional<MediaRoute::Id> terminating_route_id_;
+
   // Observers for dialog model updates.
   base::ObserverList<CastDialogController::Observer> observers_;
 

chrome/browser/ui/views/media_router/media_router_views_ui_unittest.cc

@@ -170,7 +170,7 @@ TEST_F(MediaRouterViewsUITest, ConnectingState) {
   // When a request to Cast to a sink is made, its state should become
   // CONNECTING.
   EXPECT_CALL(observer, OnModelUpdated(_))
-      .WillOnce(WithArg<0>(Invoke([&sink](const CastDialogModel& model) {
+      .WillOnce(WithArg<0>(Invoke([](const CastDialogModel& model) {
         ASSERT_EQ(1u, model.media_sinks().size());
         EXPECT_EQ(UIMediaSinkState::CONNECTING, model.media_sinks()[0].state);
       })));
@@ -178,7 +178,7 @@ TEST_F(MediaRouterViewsUITest, ConnectingState) {
 
   // Once a route is created for the sink, its state should become CONNECTED.
   EXPECT_CALL(observer, OnModelUpdated(_))
-      .WillOnce(WithArg<0>(Invoke([&sink](const CastDialogModel& model) {
+      .WillOnce(WithArg<0>(Invoke([](const CastDialogModel& model) {
         ASSERT_EQ(1u, model.media_sinks().size());
         EXPECT_EQ(UIMediaSinkState::CONNECTED, model.media_sinks()[0].state);
       })));
@@ -187,6 +187,36 @@ TEST_F(MediaRouterViewsUITest, ConnectingState) {
   ui_->RemoveObserver(&observer);
 }
 
+TEST_F(MediaRouterViewsUITest, DisconnectingState) {
+  MockControllerObserver observer;
+  ui_->AddObserver(&observer);
+
+  MediaSink sink(kSinkId, kSinkName, SinkIconType::GENERIC);
+  MediaRoute route(kRouteId, MediaSource(kSourceId), kSinkId, "", true, true);
+  for (MediaSinksObserver* sinks_observer : media_sinks_observers_)
+    sinks_observer->OnSinksUpdated({sink}, std::vector<url::Origin>());
+  ui_->OnRoutesUpdated({route}, {});
+
+  // When a request to stop casting to a sink is made, its state should become
+  // DISCONNECTING.
+  EXPECT_CALL(observer, OnModelUpdated(_))
+      .WillOnce(WithArg<0>(Invoke([](const CastDialogModel& model) {
+        ASSERT_EQ(1u, model.media_sinks().size());
+        EXPECT_EQ(UIMediaSinkState::DISCONNECTING,
+                  model.media_sinks()[0].state);
+      })));
+  ui_->StopCasting(kRouteId);
+
+  // Once the route is removed, the sink's state should become AVAILABLE.
+  EXPECT_CALL(observer, OnModelUpdated(_))
+      .WillOnce(WithArg<0>(Invoke([](const CastDialogModel& model) {
+        ASSERT_EQ(1u, model.media_sinks().size());
+        EXPECT_EQ(UIMediaSinkState::AVAILABLE, model.media_sinks()[0].state);
+      })));
+  ui_->OnRoutesUpdated({}, {});
+  ui_->RemoveObserver(&observer);
+}
+
 TEST_F(MediaRouterViewsUITest, AddAndRemoveIssue) {
   MediaSink sink1("sink_id1", "Sink 1", SinkIconType::CAST_AUDIO);
   MediaSinkWithCastModes sink1_with_cast_modes(sink1);