Show the Cast toolbar icon ephemerally when Cast is in use

chrome/browser/ui/toolbar/media_router_action.cc

Index: chrome/browser/ui/toolbar/media_router_action.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action.cc b/chrome/browser/ui/toolbar/media_router_action.cc
index 97c60b2a4d4204916f94fb4734162b036ff85d49..53f8195143aec6dabc852ab654552d07dd306c83 100644
--- a/chrome/browser/ui/toolbar/media_router_action.cc
+++ b/chrome/browser/ui/toolbar/media_router_action.cc
@@ -17,8 +17,13 @@
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/media_router_action_platform_delegate.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
+#include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 #include "chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/pref_service_factory.h"
+#include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia.h"
@@ -47,11 +52,13 @@ MediaRouterAction::MediaRouterAction(Browser* browser,
       platform_delegate_(MediaRouterActionPlatformDelegate::Create(browser)),
       contextual_menu_(browser),
       tab_strip_model_observer_(this),
+      toolbar_actions_bar_observer_(this),
       weak_ptr_factory_(this) {
   DCHECK(browser_);
   DCHECK(toolbar_actions_bar_);
   tab_strip_model_observer_.Add(browser_->tab_strip_model());
-
+  toolbar_actions_bar_observer_.Add(toolbar_actions_bar_);
+  contextual_menu_.SetMediaRouterAction(weak_ptr_factory_.GetWeakPtr());
   RegisterObserver();
 }
 
@@ -113,7 +120,6 @@ bool MediaRouterAction::HasPopup(
 
 void MediaRouterAction::HidePopup() {
   GetMediaRouterDialogController()->HideMediaRouterDialog();
-  OnPopupHidden();

[msw, 2016/07/26 19:59:31]

Why is this no longer called?

[takumif, 2016/07/28 20:04:10]

MediaRouterDialogControllerImpl::OnDialogClosed also calls OnPopupHidden, so
this is redundant. Also this is only called when the popup is closed through the
toolbar, whereas OnDialogClosed is always called.

 }
 
 gfx::NativeView MediaRouterAction::GetPopupNativeView() {
@@ -160,12 +166,15 @@ void MediaRouterAction::OnIssueUpdated(const media_router::Issue* issue) {
 void MediaRouterAction::OnRoutesUpdated(
     const std::vector<media_router::MediaRoute>& routes,
     const std::vector<media_router::MediaRoute::Id>& joinable_route_ids) {
+  bool had_local_display_route = has_local_display_route_;
   has_local_display_route_ =
       std::find_if(routes.begin(), routes.end(),
                    [](const media_router::MediaRoute& route) {
                      return route.is_local() && route.for_display();
                    }) != routes.end();
   MaybeUpdateIcon();
+  if (has_local_display_route_ != had_local_display_route)
+    UpdateVisibility();
 }
 
 void MediaRouterAction::ActiveTabChanged(content::WebContents* old_contents,
@@ -173,11 +182,59 @@ void MediaRouterAction::ActiveTabChanged(content::WebContents* old_contents,
                                          int index,
                                          int reason) {
   UpdatePopupState();
+  UpdateVisibility();
+}
+
+void MediaRouterAction::ToggleVisibilityPreference() {
+  SetVisibilityPreference(!browser_->profile()->GetPrefs()->

[msw, 2016/07/26 19:59:31]

optional nit: use the IsEphemeral (perhaps renamed) helper here.

[takumif, 2016/07/28 20:04:10]

Done.

+      GetBoolean(prefs::kMediaRouterAlwaysShowActionIcon));
+}
+
+void MediaRouterAction::SetVisibilityPreference(bool always_show) {
+  PrefService* pref_service = browser_->profile()->GetPrefs();
+  pref_service->SetBoolean(prefs::kMediaRouterAlwaysShowActionIcon,
+                           always_show);
+  UpdateVisibility();
+}
+
+void MediaRouterAction::UpdateVisibility() {
+  if (!delegate_)
+    return;
+
+  MediaRouterDialogControllerImpl* controller =
+      GetMediaRouterDialogController();
+  bool show_icon = !IsEphemeral() ||
+                   controller->IsShowingMediaRouterDialog() ||
+                   has_local_display_route_;
+  ToolbarActionsModel* model = ToolbarActionsModel::Get(browser_->profile());
+  if (show_icon) {
+    if (!model->HasComponentAction(GetId()))
+      model->AddComponentAction(GetId());
+  } else {
+    if (model->HasComponentAction(GetId()))

[Devlin, 2016/07/26 21:59:02]

isn't this the same as
else if (model->HasComponentAction(GetID())) {
 ...
}
?

[takumif, 2016/07/28 20:04:10]

Yes, it is. Corrected.

+      model->RemoveComponentAction(GetId());
+  }
 }
 
 void MediaRouterAction::OnPopupHidden() {
   if (delegate_)
     delegate_->OnPopupClosed();
+  ToolbarActionsModel* model = ToolbarActionsModel::Get(browser_->profile());
+  if (model->HasComponentAction(GetId()) &&

[msw, 2016/07/26 19:59:31]

Shouldn't this also check IsShowingMediaRouterDialog?
(I'm not sure if the dialog is the same as the popup...)

[takumif, 2016/07/28 20:04:10]

I could replace this with a call to MRAction::UpdateVisibility from
MRDialogControllerImpl::Reset, so I did that. Yes, the dialog is same as the
popup. MR uses the term dialog and the toolbar uses popup.

+      !has_local_display_route_ &&
+      IsEphemeral()) {
+    model->RemoveComponentAction(GetId());
+  }
+}
+
+void MediaRouterAction::OnToolbarActionsBarAnimationEnded() {
+  if (delegate_) {
+    if (GetMediaRouterDialogController()->IsShowingMediaRouterDialog()) {

[msw, 2016/07/26 19:59:31]

nit: curlies not needed.

[takumif, 2016/07/28 20:04:10]

Done.

+      delegate_->OnPopupShown(true);

[Devlin, 2016/07/26 21:59:02]

This behavior, or rather the motivation, isn't clear to me.  Can you add a
comment?

[takumif, 2016/07/28 20:04:10]

Done.

+    } else {
+      delegate_->OnPopupClosed();
+    }
+  }
 }
 
 void MediaRouterAction::OnPopupShown() {
@@ -203,11 +260,15 @@ void MediaRouterAction::UpdatePopupState() {
     controller->SetMediaRouterAction(weak_ptr_factory_.GetWeakPtr());
 
   // Update the button in case the pressed state is out of sync with dialog
-  // visibility.
-  if (controller->IsShowingMediaRouterDialog())
-    OnPopupShown();
-  else
-    OnPopupHidden();
+  // visibility. If we just added the ephemeral icon to the toolbar and need to
+  // press it, we do that in |OnToolbarActionsBarAnimationEnded|.
+  if (delegate_) {
+    if (!IsEphemeral() && controller->IsShowingMediaRouterDialog()) {

[msw, 2016/07/26 19:59:31]

nit: curlies not needed.

[takumif, 2016/07/28 20:04:10]

Done.

+      delegate_->OnPopupShown(true);
+    } else if (!controller->IsShowingMediaRouterDialog()) {
+      delegate_->OnPopupClosed();
+    }
+  }
 }
 
 MediaRouterDialogControllerImpl*
@@ -216,7 +277,7 @@ MediaRouterAction::GetMediaRouterDialogController() {
   content::WebContents* web_contents = delegate_->GetCurrentWebContents();
   DCHECK(web_contents);
   return MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
-      web_contents);
+    web_contents);

[Devlin, 2016/07/26 21:59:02]

indentation was right before.

[takumif, 2016/07/28 20:04:10]

Done.

 }
 
 MediaRouterActionPlatformDelegate* MediaRouterAction::GetPlatformDelegate() {
@@ -249,3 +310,12 @@ gfx::VectorIconId MediaRouterAction::GetCurrentIcon() const {
   return has_local_display_route_ ? gfx::VectorIconId::MEDIA_ROUTER_ACTIVE
                                   : gfx::VectorIconId::MEDIA_ROUTER_IDLE;
 }
+
+base::WeakPtr<MediaRouterAction> MediaRouterAction::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
+bool MediaRouterAction::IsEphemeral() {
+  PrefService* pref_service = browser_->profile()->GetPrefs();
+  return !pref_service->GetBoolean(prefs::kMediaRouterAlwaysShowActionIcon);
+}