[Media Router] Clean up issues related code.

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..7f28bf91a9f0feb57dd177b9ebf3924a975dc947 100644
--- a/chrome/browser/ui/toolbar/media_router_action.cc
+++ b/chrome/browser/ui/toolbar/media_router_action.cc
@@ -40,6 +40,8 @@ MediaRouterAction::MediaRouterAction(Browser* browser,
     : media_router::IssuesObserver(GetMediaRouter(browser)),
       media_router::MediaRoutesObserver(GetMediaRouter(browser)),
       current_icon_(gfx::VectorIconId::MEDIA_ROUTER_IDLE),
+      has_issue_(false),
+      current_issue_severity_(media_router::Issue::Severity::NOTIFICATION),
       has_local_display_route_(false),
       delegate_(nullptr),
       browser_(browser),
@@ -152,9 +154,15 @@ bool MediaRouterAction::DisabledClickOpensMenu() const {
 }
 
 void MediaRouterAction::OnIssueUpdated(const media_router::Issue* issue) {

[mark a. foltz, 2016/07/27 18:52:04]

I don't think |issue| should be modified here.  Can this be const Issue&
instead?

[imcheng, 2016/09/13 20:27:54]

There was a similar comment in Takumi's patch. We currently use nullptr to
denote "no more issues" in the IssuesObserver API. Using "const Issue*" ensures
the input issue doesn't get modified. 

-  issue_.reset(issue ? new media_router::Issue(*issue) : nullptr);
-
-  MaybeUpdateIcon();
+  bool new_has_issue = (issue != nullptr);
+  media_router::Issue::Severity new_severity =
+      issue ? issue->severity() :
+      media_router::Issue::Severity::NOTIFICATION;
+  if (has_issue_ != new_has_issue || current_issue_severity_ != new_severity) {
+    has_issue_ = new_has_issue;
+    current_issue_severity_ = new_severity;
+    MaybeUpdateIcon();
+  }
 }
 
 void MediaRouterAction::OnRoutesUpdated(
@@ -239,11 +247,12 @@ void MediaRouterAction::MaybeUpdateIcon() {
 
 gfx::VectorIconId MediaRouterAction::GetCurrentIcon() const {
   // Highest priority is to indicate whether there's an issue.
-  if (issue_) {
-    if (issue_->severity() == media_router::Issue::FATAL)
+  if (has_issue_) {
+    if (current_issue_severity_ == media_router::Issue::Severity::FATAL)
       return gfx::VectorIconId::MEDIA_ROUTER_ERROR;
-    if (issue_->severity() == media_router::Issue::WARNING)
+    else if (current_issue_severity_ == media_router::Issue::Severity::WARNING)
       return gfx::VectorIconId::MEDIA_ROUTER_WARNING;
+    // Fall through for Severity::NOTIFICATION.
   }
 
   return has_local_display_route_ ? gfx::VectorIconId::MEDIA_ROUTER_ACTIVE