Show media router toolbar icon ephemerally for active local routes and issues

chrome/browser/ui/toolbar/media_router_action.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/toolbar/media_router_action.h"
 
+#include <string>
+
 #include "base/metrics/user_metrics.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/media/router/issue.h"
 #include "chrome/browser/media/router/media_route.h"
 #include "chrome/browser/media/router/media_router.h"
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #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/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/paint_vector_icon.h"
 
 using media_router::MediaRouterDialogControllerImpl;
 
 namespace {
 
 media_router::MediaRouter* GetMediaRouter(Browser* browser) {
   return media_router::MediaRouterFactory::GetApiForBrowserContext(
       browser->profile());
 }
 
 }  // namespace
 
 MediaRouterAction::MediaRouterAction(Browser* browser,
                                      ToolbarActionsBar* toolbar_actions_bar)
     : media_router::IssuesObserver(GetMediaRouter(browser)),
       media_router::MediaRoutesObserver(GetMediaRouter(browser)),
       current_icon_(gfx::VectorIconId::MEDIA_ROUTER_IDLE),
       has_local_display_route_(false),
+      has_dialog_(false),
       delegate_(nullptr),
       browser_(browser),
       toolbar_actions_bar_(toolbar_actions_bar),
       platform_delegate_(MediaRouterActionPlatformDelegate::Create(browser)),
-      contextual_menu_(browser),
+      contextual_menu_(browser, this),
       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_);
   RegisterObserver();
 }
 
 MediaRouterAction::~MediaRouterAction() {
   UnregisterObserver();
 }
 
 std::string MediaRouterAction::GetId() const {
   return ComponentToolbarActionsFactory::kMediaRouterActionId;
 }
 
 void MediaRouterAction::SetDelegate(ToolbarActionViewDelegate* delegate) {
   delegate_ = delegate;
 
-  // Updates the current popup state if |delegate_| is non-null and has
-  // WebContents ready.
+  // Updates the current dialog state if |delegate_| has WebContents ready.
   // In cases such as opening a new browser window, SetDelegate() will be
-  // called before the WebContents is set. In those cases, we update the popup
+  // called before the WebContents is set. In those cases, we update the dialog
   // state when ActiveTabChanged() is called.
-  if (delegate_ && delegate_->GetCurrentWebContents())
-    UpdatePopupState();
+  if (delegate_ && delegate_->GetCurrentWebContents()) {
+    RegisterWithDialogController();
+    // Update the button in case the pressed state is out of sync with dialog

[imcheng, 2016/09/16 21:47:49]

nit: blank line before comments

+    // visibility. If we just added the ephemeral icon to the toolbar and need
+    // to depress it, we do that in |OnToolbarActionsBarAnimationEnded|.
+    if (GetAlwaysShowActionPref())

[imcheng, 2016/09/16 21:47:48]

question: So if I added the icon by preference (i.e., not ephemerally) then
OnToolbarActionsBarAnimationEnded won't get called, hence the need to call
UpdateDialogState here? Why would that be the case?

[takumif, 2016/09/20 14:50:46]

Actually, OnToolbarActionsBarAnimationEnded, OnActiveTabCahged, and the calls
from MRDialogControllerImpl cover all the cases, so this is not necessary.
Removing.

+      UpdateDialogState();
+  }
 }
 
 gfx::Image MediaRouterAction::GetIcon(content::WebContents* web_contents,
                                       const gfx::Size& size) {
   // Color is defined in the icon.
   return gfx::Image(
       gfx::CreateVectorIcon(current_icon_, gfx::kPlaceholderColor));
 }
 
 base::string16 MediaRouterAction::GetActionName() const {
@@ skipping 20 matching lines @@
   return false;
 }
 
 bool MediaRouterAction::HasPopup(
     content::WebContents* web_contents) const {
   return true;
 }
 
 void MediaRouterAction::HidePopup() {
   GetMediaRouterDialogController()->HideMediaRouterDialog();
-  OnPopupHidden();
 }
 
 gfx::NativeView MediaRouterAction::GetPopupNativeView() {
   return nullptr;
 }
 
 ui::MenuModel* MediaRouterAction::GetContextMenu() {
   return contextual_menu_.menu_model();
 }
 
 bool MediaRouterAction::ExecuteAction(bool by_user) {
   base::RecordAction(base::UserMetricsAction("MediaRouter_Icon_Click"));
 
   if (GetMediaRouterDialogController()->IsShowingMediaRouterDialog()) {
     GetMediaRouterDialogController()->HideMediaRouterDialog();
     return false;
   }
 
   GetMediaRouterDialogController()->ShowMediaRouterDialog();
   if (GetPlatformDelegate()) {
     media_router::MediaRouterMetrics::RecordMediaRouterDialogOrigin(
         GetPlatformDelegate()->CloseOverflowMenuIfOpen() ?
         media_router::MediaRouterDialogOpenOrigin::OVERFLOW_MENU :
         media_router::MediaRouterDialogOpenOrigin::TOOLBAR);
   }
   return true;
 }
 
 void MediaRouterAction::UpdateState() {
-  if (delegate_)
-    delegate_->UpdateState();
+  DCHECK(delegate_);
+  delegate_->UpdateState();
 }
 
 bool MediaRouterAction::DisabledClickOpensMenu() const {
   return false;
 }
 
 void MediaRouterAction::OnIssueUpdated(const media_router::Issue* issue) {
   issue_.reset(issue ? new media_router::Issue(*issue) : nullptr);
-
   MaybeUpdateIcon();
 }
 
 void MediaRouterAction::OnRoutesUpdated(
     const std::vector<media_router::MediaRoute>& routes,
     const std::vector<media_router::MediaRoute::Id>& joinable_route_ids) {
   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();
 }
 
 void MediaRouterAction::ActiveTabChanged(content::WebContents* old_contents,
                                          content::WebContents* new_contents,
                                          int index,
                                          int reason) {
-  UpdatePopupState();
+  RegisterWithDialogController();
+  UpdateDialogState();
 }
 
-void MediaRouterAction::OnPopupHidden() {
-  if (delegate_)
-    delegate_->OnPopupClosed();
+void MediaRouterAction::OnToolbarActionsBarAnimationEnded() {
+  // Depress the action if the dialog is shown.
+  DCHECK(delegate_);
+  if (GetMediaRouterDialogController()->IsShowingMediaRouterDialog())

[imcheng, 2016/09/16 21:47:48]

Is it possible for this to return false, and do we need to call OnDialogHidden()
in that case? (Or just call UpdateDialogState() here)?

[takumif, 2016/09/20 14:50:46]

This can be false in cases such as receiving an issue while the dialog is not
open. This callback's purpose is to depress the newly created action, and we
don't have to worry about calling OnDialogHidden() because it comes
un-depressed.

+    OnDialogShown();
 }
 
-void MediaRouterAction::OnPopupShown() {
-  // We depress the action regardless of whether ExecuteAction() was user
-  // initiated.
-  if (delegate_)
-    delegate_->OnPopupShown(true);
+void MediaRouterAction::OnDialogHidden() {
+  if (has_dialog_) {
+    has_dialog_ = false;
+    DCHECK(delegate_);
+    delegate_->OnPopupClosed();

[imcheng, 2016/09/16 21:47:49]

Will |has_dialog_| always be in sync with |delegate_|'s state?

[takumif, 2016/09/20 14:50:46]

It should be. We only call OnPopupShown/Closed() when we change |has_dialog_|.

+  }
 }
 
-void MediaRouterAction::UpdatePopupState() {
+void MediaRouterAction::OnDialogShown() {
+  if (!has_dialog_) {
+    has_dialog_ = true;
+    DCHECK(delegate_);
+    // We depress the action regardless of whether ExecuteAction() was user
+    // initiated.
+    delegate_->OnPopupShown(true);
+  }
+}
+
+bool MediaRouterAction::GetAlwaysShowActionPref() const {
+  return ToolbarActionsModel::Get(browser_->profile())
+      ->component_migration_helper()->GetComponentActionPref(
+          ComponentToolbarActionsFactory::kMediaRouterActionId);
+}
+
+void MediaRouterAction::SetAlwaysShowActionPref(bool always_show) {
+  ToolbarActionsModel::Get(browser_->profile())->component_migration_helper()
+      ->SetComponentActionPref(
+          ComponentToolbarActionsFactory::kMediaRouterActionId, always_show);
+}
+
+base::WeakPtr<MediaRouterAction> MediaRouterAction::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
+void MediaRouterAction::RegisterWithDialogController() {
   MediaRouterDialogControllerImpl* controller =
       GetMediaRouterDialogController();
 
   if (!controller)
     return;
 
-  // When each browser window is created, its toolbar creates a
-  // MediaRouterAction that is only destroyed when the browser window is torn
-  // down. |controller| will keep track of that instance. If |this| was created
-  // in overflow mode, it will be destroyed when the overflow menu is closed.
-  // If SetMediaRouterAction() was previously called, this is a no-op.
+  // |controller| keeps track of |this| if |this| was created with the browser
+  // window or ephemerally by activating the Cast functionality. If |this| was
+  // created in overflow mode, it will be destroyed when the overflow menu is
+  // closed. If SetMediaRouterAction() was previously called, this is a no-op.
   if (!toolbar_actions_bar_->in_overflow_mode())
     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();
+void MediaRouterAction::UpdateDialogState() {
+  DCHECK(delegate_);

[imcheng, 2016/09/16 21:47:49]

nit: this DCHECK not needed as the methods that you call already have them.

[takumif, 2016/09/20 14:50:46]

Removed.

+  if (GetMediaRouterDialogController()->IsShowingMediaRouterDialog())
+    OnDialogShown();
   else
-    OnPopupHidden();
+    OnDialogHidden();
 }
 
 MediaRouterDialogControllerImpl*
 MediaRouterAction::GetMediaRouterDialogController() {
   DCHECK(delegate_);
   content::WebContents* web_contents = delegate_->GetCurrentWebContents();
   DCHECK(web_contents);
   return MediaRouterDialogControllerImpl::GetOrCreateForWebContents(
       web_contents);
 }
 
 MediaRouterActionPlatformDelegate* MediaRouterAction::GetPlatformDelegate() {
   return platform_delegate_.get();
 }
 
 void MediaRouterAction::MaybeUpdateIcon() {
   gfx::VectorIconId new_icon = GetCurrentIcon();
 
   // Update the current state if it has changed.
   if (new_icon != current_icon_) {
     current_icon_ = new_icon;
 
     // Tell the associated view to update its icon to reflect the change made
-    // above.
+    // above. If MaybeUpdateIcon() was called as a result of instantiating
+    // |this|, then |delegate_| may not be set yet.
     if (delegate_)
-      delegate_->UpdateState();
+      UpdateState();
   }
 }
 
 gfx::VectorIconId MediaRouterAction::GetCurrentIcon() const {
   // Highest priority is to indicate whether there's an issue.
   if (issue_) {
     if (issue_->severity() == media_router::Issue::FATAL)
       return gfx::VectorIconId::MEDIA_ROUTER_ERROR;
     if (issue_->severity() == media_router::Issue::WARNING)
       return gfx::VectorIconId::MEDIA_ROUTER_WARNING;
   }
 
   return has_local_display_route_ ? gfx::VectorIconId::MEDIA_ROUTER_ACTIVE
                                   : gfx::VectorIconId::MEDIA_ROUTER_IDLE;
 }