Show Media Router toolbar icon ephemerally for MR dialogs

chrome/browser/ui/toolbar/media_router_action_controller.cc

Index: chrome/browser/ui/toolbar/media_router_action_controller.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action_controller.cc b/chrome/browser/ui/toolbar/media_router_action_controller.cc
index 722745255b949eb1d22bd56fda177439e377fe63..817be4f5dddd1a39bca54e83602251dc5be0d27d 100644
--- a/chrome/browser/ui/toolbar/media_router_action_controller.cc
+++ b/chrome/browser/ui/toolbar/media_router_action_controller.cc
@@ -15,9 +15,13 @@ MediaRouterActionController::MediaRouterActionController(Profile* profile)
           profile,
           media_router::MediaRouterFactory::GetApiForBrowserContext(profile),
           ToolbarActionsModel::Get(profile),
-          ToolbarActionsModel::Get(profile)->component_migration_helper()) {}
+          ToolbarActionsModel::Get(profile)->component_migration_helper()) {
+  DCHECK(component_action_delegate_);
+  DCHECK(component_migration_helper_);
+}
 
 MediaRouterActionController::~MediaRouterActionController() {
+  DCHECK_EQ(dialog_count_, 0u);
   UnregisterObserver();  // media_router::IssuesObserver.
 }
 
@@ -39,6 +43,18 @@ void MediaRouterActionController::OnRoutesUpdated(
   MaybeAddOrRemoveAction();
 }
 
+void MediaRouterActionController::OnDialogShown() {
+  dialog_count_++;
+  MaybeAddOrRemoveAction();
+}
+
+void MediaRouterActionController::OnDialogHidden() {
+  DCHECK_NE(dialog_count_, 0u);

[msw, 2016/11/04 00:07:16]

nit: DCHECK_GE?

[takumif, 2016/11/09 04:37:27]

Changing to DCHECK_GT.

+  if (dialog_count_)

[msw, 2016/11/04 00:07:16]

nit: if dialog_count_ > 0 or use dialog_count_ = std::max(0, dialog_count_ - 1);

[takumif, 2016/11/09 04:37:27]

mfoltz@'s nit was to change from dialog_count_ > 0 to this. Do you feel strongly
about this?

[msw, 2016/11/09 20:32:33]

No, I think it's worse, but I don't feel too strongly.

+    dialog_count_--;
+  MaybeAddOrRemoveAction();
+}
+
 MediaRouterActionController::MediaRouterActionController(
     Profile* profile,
     media_router::MediaRouter* router,
@@ -51,7 +67,6 @@ MediaRouterActionController::MediaRouterActionController(
       component_action_delegate_(component_action_delegate),
       component_migration_helper_(component_migration_helper) {
   DCHECK(profile_);
-  DCHECK(component_action_delegate_);
   RegisterObserver();  // media_router::IssuesObserver.
   pref_change_registrar_.Init(profile->GetPrefs());
   pref_change_registrar_.Add(
@@ -63,9 +78,10 @@ MediaRouterActionController::MediaRouterActionController(
 void MediaRouterActionController::MaybeAddOrRemoveAction() {
   if (ShouldEnableAction()) {
     if (!component_action_delegate_->HasComponentAction(
-            ComponentToolbarActionsFactory::kMediaRouterActionId))
+            ComponentToolbarActionsFactory::kMediaRouterActionId)) {
       component_action_delegate_->AddComponentAction(
           ComponentToolbarActionsFactory::kMediaRouterActionId);
+    }
   } else if (component_action_delegate_->HasComponentAction(
                  ComponentToolbarActionsFactory::kMediaRouterActionId)) {
     component_action_delegate_->RemoveComponentAction(
@@ -74,7 +90,7 @@ void MediaRouterActionController::MaybeAddOrRemoveAction() {
 }
 
 bool MediaRouterActionController::ShouldEnableAction() const {
-  return has_local_display_route_ || has_issue_ ||
+  return has_local_display_route_ || has_issue_ || dialog_count_ ||

[msw, 2016/11/04 00:07:16]

nit: dialog_count_ > 0

[takumif, 2016/11/09 04:37:27]

Same as above.

          component_migration_helper_->GetComponentActionPref(
              ComponentToolbarActionsFactory::kMediaRouterActionId);
 }