Reflect the current state in Media Router Action icon.

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 cc9425e2e29181180028cb4d9f5b94a518816b76..f61f31f84a8f99edc5c5cfecabcb8b378a0a1ccf 100644
--- a/chrome/browser/ui/toolbar/media_router_action.cc
+++ b/chrome/browser/ui/toolbar/media_router_action.cc
@@ -5,7 +5,13 @@
 #include "chrome/browser/ui/toolbar/media_router_action.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_dialog_controller.h"
+#include "chrome/browser/media/router/media_router_factory.h"
+#include "chrome/browser/media/router/media_router_mojo_impl.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/toolbar/media_router_action_platform_delegate.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
 #include "chrome/grit/generated_resources.h"
@@ -17,10 +23,21 @@
 using media_router::MediaRouterDialogController;
 
 MediaRouterAction::MediaRouterAction(Browser* browser)
-    : id_("media_router_action"),
+    : media_router::IssuesObserver(GetMediaRouter(browser)),
+      media_router::MediaRoutesObserver(GetMediaRouter(browser)),
+      id_("media_router_action"),
       name_(l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_TITLE)),
+      media_router_active_icon_(ui::ResourceBundle::GetSharedInstance().
+          GetImageNamed(IDR_MEDIA_ROUTER_ACTIVE_ICON)),
+      media_router_error_icon_(ui::ResourceBundle::GetSharedInstance().
+          GetImageNamed(IDR_MEDIA_ROUTER_ERROR_ICON)),
       media_router_idle_icon_(ui::ResourceBundle::GetSharedInstance().
           GetImageNamed(IDR_MEDIA_ROUTER_IDLE_ICON)),
+      media_router_warning_icon_(ui::ResourceBundle::GetSharedInstance().
+          GetImageNamed(IDR_MEDIA_ROUTER_WARNING_ICON)),
+      state_(MEDIA_ROUTER_ACTION_IDLE),
+      issue_(nullptr),
+      has_local_routes_(false),
       delegate_(nullptr),
       platform_delegate_(MediaRouterActionPlatformDelegate::Create(browser)),
       contextual_menu_(browser) {
@@ -39,8 +56,19 @@ void MediaRouterAction::SetDelegate(ToolbarActionViewDelegate* delegate) {
 
 gfx::Image MediaRouterAction::GetIcon(content::WebContents* web_contents,
                                       const gfx::Size& size) {
-  // TODO(apacible): Return icon based on casting state.
-  return media_router_idle_icon_;
+  switch (state_) {
+    case MEDIA_ROUTER_ACTION_IDLE:
+      return media_router_idle_icon_;
+    case MEDIA_ROUTER_ACTION_ACTIVE:
+      return media_router_active_icon_;
+    case MEDIA_ROUTER_ACTION_ERROR:
+      return media_router_error_icon_;
+    case MEDIA_ROUTER_ACTION_WARNING:
+      return media_router_warning_icon_;
+    default:

[Peter Kasting, 2015/08/03 21:28:51]

Don't add a default case when your switch handles all values of an enum.

Instead put the code here outside the switch.

[apacible, 2015/08/05 06:47:55]

Acknowledged; ended up removing enum.

+      NOTREACHED();
+      return media_router_idle_icon_;
+  }
 }
 
 base::string16 MediaRouterAction::GetActionName() const {
@@ -98,6 +126,32 @@ bool MediaRouterAction::ExecuteAction(bool by_user) {
 void MediaRouterAction::UpdateState() {
 }
 
+bool MediaRouterAction::DisabledClickOpensMenu() const {
+  return false;
+}
+
+void MediaRouterAction::OnIssueUpdated(const media_router::Issue* issue) {
+  if (issue)
+    issue_.reset(new media_router::Issue(*issue));
+  else
+    issue_.reset();

[Peter Kasting, 2015/08/03 21:28:51]

Nit: Simpler:

  issue.reset(issue ? new media_router::Issue(*issue) : nullptr);

[apacible, 2015/08/05 06:47:55]

Done.

+
+  MaybeUpdateIcon();
+}
+
+void MediaRouterAction::OnRoutesUpdated(
+    const std::vector<media_router::MediaRoute>& routes) {
+  // Check if any of the routes are local.
+  auto it = std::find_if(routes.begin(), routes.end(),
+                         [](const media_router::MediaRoute& route) {
+                             return route.is_local();
+                         });
+
+  has_local_routes_ = it != routes.end();

[Peter Kasting, 2015/08/03 21:28:51]

Nit: Simpler:

  has_local_routes_ =
      std::find_if(routes.begin(), routes.end(),
                   [](const auto& route) { return route.is_local(); }) !=
      routes.end();

[apacible, 2015/08/05 06:47:55]

Done.

+
+  MaybeUpdateIcon();
+}
+
 MediaRouterDialogController*
 MediaRouterAction::GetMediaRouterDialogController() {
   DCHECK(delegate_);
@@ -106,6 +160,35 @@ MediaRouterAction::GetMediaRouterDialogController() {
   return MediaRouterDialogController::GetOrCreateForWebContents(web_contents);
 }
 
-bool MediaRouterAction::DisabledClickOpensMenu() const {
-  return false;
+media_router::MediaRouter* MediaRouterAction::GetMediaRouter(Browser* browser) {
+  return media_router::MediaRouterFactory::GetApiForBrowserContext(
+      (content::BrowserContext*) browser->profile());

[Peter Kasting, 2015/08/03 21:28:51]

C++-style casts, please

[apacible, 2015/08/05 06:47:55]

Done.

+}
+
+void MediaRouterAction::MaybeUpdateIcon() {
+  MediaRouterActionState new_state = GetMediaRouterActionState();
+
+  // Update the current state if it has changed.
+  if (new_state != state_) {
+    state_ = new_state;
+
+    // Update the icon to reflect the new state.
+    if (delegate_)
+      delegate_->UpdateState();
+  }
+}
+
+MediaRouterActionState MediaRouterAction::GetMediaRouterActionState() {
+  // Highest priority is to indicate whether there's an issue.
+  if (issue_) {
+    if (issue_.get()->severity() == media_router::Issue::FATAL)

[Peter Kasting, 2015/08/03 21:28:51]

Don't use .get() just to call ->.  Use -> directly without the .get().  (2x)

[apacible, 2015/08/05 06:47:55]

Done.

+      return MEDIA_ROUTER_ACTION_ERROR;
+    else if (issue_.get()->severity() == media_router::Issue::WARNING)

[Peter Kasting, 2015/08/03 21:28:51]

No else after return

[apacible, 2015/08/05 06:47:55]

Done.

+      return MEDIA_ROUTER_ACTION_WARNING;
+  }
+
+  if (has_local_routes_)
+    return MEDIA_ROUTER_ACTION_ACTIVE;
+
+  return MEDIA_ROUTER_ACTION_IDLE;

[Peter Kasting, 2015/08/03 21:28:51]

Nit: Simpler:

  return has_local_routes_ ?
      MEDIA_ROUTER_ACTION_ACTIVE : MEDIA_ROUTER_ACTION_IDLE;

[apacible, 2015/08/05 06:47:55]

Done.

 }