[Media Router] Clean up issues related code.

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 "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"
@@ skipping 22 matching lines @@
       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_issue_(false),
+      current_issue_severity_(media_router::Issue::Severity::NOTIFICATION),
       has_local_display_route_(false),
       delegate_(nullptr),
       browser_(browser),
       toolbar_actions_bar_(toolbar_actions_bar),
       platform_delegate_(MediaRouterActionPlatformDelegate::Create(browser)),
       contextual_menu_(browser),
       tab_strip_model_observer_(this),
       weak_ptr_factory_(this) {
   DCHECK(browser_);
   DCHECK(toolbar_actions_bar_);
@@ skipping 91 matching lines @@
 
 void MediaRouterAction::UpdateState() {
   if (delegate_)
     delegate_->UpdateState();
 }
 
 bool MediaRouterAction::DisabledClickOpensMenu() const {
   return false;
 }
 
 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(
     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();
@@ skipping 64 matching lines @@
 
     // Tell the associated view to update its icon to reflect the change made
     // above.
     if (delegate_)
       delegate_->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)
+  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
                                   : gfx::VectorIconId::MEDIA_ROUTER_IDLE;
 }