Show per-sink issues in Harmony Cast Dialog

- Add sink_id attribute to IssueInfo and mojom::Issue
- Move the code for observing issues from MRUI to MRUIBase
- Associate sinks and issues in MediaRouterViewsUI
- Show info icon in CastDialogSinkButton if there is an issue for the sink

The corresponding extension-side change is at cl/197620276.

Bug: 554646

Change-Id: Idf63467aa7bb18174430e8e74d2e80e3a9333eb2
Reviewed-on: https://chromium-review.googlesource.com/1064568
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566313}

chrome/browser/ui/media_router/media_router_ui_base.cc

@@ -14,6 +14,8 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/media/router/issue_manager.h"
+#include "chrome/browser/media/router/issues_observer.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"
@@ -26,9 +28,11 @@
 #include "chrome/common/media_router/media_source.h"
 #include "chrome/common/media_router/media_source_helper.h"
 #include "chrome/common/media_router/route_request_result.h"
+#include "chrome/grit/generated_resources.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 #include "third_party/icu/source/i18n/unicode/coll.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "url/origin.h"
 
 #if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
@@ -137,9 +141,12 @@ bool MediaRouterUIBase::CreateRoute(const MediaSink::Id& sink_id,
                                     MediaCastMode cast_mode) {
   base::Optional<RouteParameters> params =
       GetRouteParameters(sink_id, cast_mode);
-  if (!params)
+  if (!params) {
+    SendIssueForUnableToCast(cast_mode);
     return false;
+  }
 
+  GetIssueManager()->ClearNonBlockingIssues();
   GetMediaRouter()->CreateRoute(params->source_id, sink_id, params->origin,
                                 initiator_,
                                 std::move(params->route_response_callbacks),
@@ -192,6 +199,14 @@ std::string MediaRouterUIBase::GetTruncatedPresentationRequestSourceName()
              : TruncateHost(GetHostFromURL(gurl));
 }
 
+void MediaRouterUIBase::AddIssue(const IssueInfo& issue) {
+  GetIssueManager()->AddIssue(issue);
+}
+
+void MediaRouterUIBase::RemoveIssue(const Issue::Id& issue_id) {
+  GetIssueManager()->ClearIssue(issue_id);
+}
+
 MediaRouterUIBase::RouteRequest::RouteRequest(const MediaSink::Id& sink_id)
     : sink_id(sink_id) {
   static base::AtomicSequenceNumber g_next_request_id;
@@ -431,6 +446,76 @@ GURL MediaRouterUIBase::GetFrameURL() const {
                                : GURL();
 }
 
+void MediaRouterUIBase::SendIssueForRouteTimeout(
+    MediaCastMode cast_mode,
+    const base::string16& presentation_request_source_name) {
+  std::string issue_title;
+  switch (cast_mode) {
+    case PRESENTATION:
+      DLOG_IF(ERROR, presentation_request_source_name.empty())
+          << "Empty presentation request source name.";
+      issue_title =
+          l10n_util::GetStringFUTF8(IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT,
+                                    presentation_request_source_name);
+      break;
+    case TAB_MIRROR:
+      issue_title = l10n_util::GetStringUTF8(
+          IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_TAB);
+      break;
+    case DESKTOP_MIRROR:
+      issue_title = l10n_util::GetStringUTF8(
+          IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_DESKTOP);
+      break;
+    case LOCAL_FILE:
+      issue_title = l10n_util::GetStringUTF8(
+          IDS_MEDIA_ROUTER_ISSUE_FILE_CAST_GENERIC_ERROR);
+      break;
+  }
+
+  AddIssue(IssueInfo(issue_title, IssueInfo::Action::DISMISS,
+                     IssueInfo::Severity::NOTIFICATION));
+}
+
+void MediaRouterUIBase::SendIssueForUnableToCast(MediaCastMode cast_mode) {
+  // For a generic error, claim a tab error unless it was specifically desktop
+  // mirroring.
+  std::string issue_title =
+      (cast_mode == MediaCastMode::DESKTOP_MIRROR)
+          ? l10n_util::GetStringUTF8(
+                IDS_MEDIA_ROUTER_ISSUE_UNABLE_TO_CAST_DESKTOP)
+          : l10n_util::GetStringUTF8(
+                IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_TAB);
+  AddIssue(IssueInfo(issue_title, IssueInfo::Action::DISMISS,
+                     IssueInfo::Severity::WARNING));
+}
+
+IssueManager* MediaRouterUIBase::GetIssueManager() {
+  return GetMediaRouter()->GetIssueManager();
+}
+
+void MediaRouterUIBase::StartObservingIssues() {
+  issues_observer_ =
+      std::make_unique<UiIssuesObserver>(GetIssueManager(), this);
+  issues_observer_->Init();
+}
+
+MediaRouterUIBase::UiIssuesObserver::UiIssuesObserver(
+    IssueManager* issue_manager,
+    MediaRouterUIBase* ui)
+    : IssuesObserver(issue_manager), ui_(ui) {
+  DCHECK(ui);
+}
+
+MediaRouterUIBase::UiIssuesObserver::~UiIssuesObserver() = default;
+
+void MediaRouterUIBase::UiIssuesObserver::OnIssue(const Issue& issue) {
+  ui_->OnIssue(issue);
+}
+
+void MediaRouterUIBase::UiIssuesObserver::OnIssuesCleared() {
+  ui_->OnIssueCleared();
+}
+
 MediaRouterUIBase::UIMediaRoutesObserver::UIMediaRoutesObserver(
     MediaRouter* router,
     const MediaSource::Id& source_id,

chrome/browser/ui/media_router/media_router_ui_base.h

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
+#include "chrome/browser/media/router/issues_observer.h"
 #include "chrome/browser/media/router/media_router_dialog_controller.h"
 #include "chrome/browser/media/router/presentation/presentation_service_delegate_impl.h"
 #include "chrome/browser/ui/media_router/media_cast_mode.h"
@@ -113,6 +114,12 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
   // Returns a source name that can be shown in the dialog.
   std::string GetTruncatedPresentationRequestSourceName() const;
 
+  // Calls MediaRouter to add the given issue.
+  void AddIssue(const IssueInfo& issue);
+
+  // Calls MediaRouter to remove the given issue.
+  void RemoveIssue(const Issue::Id& issue_id);
+
   const std::vector<MediaRoute>& routes() const { return routes_; }
   content::WebContents* initiator() const { return initiator_; }
 
@@ -180,6 +187,20 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
   // Otherwise returns an empty GURL.
   GURL GetFrameURL() const;
 
+  // Creates and sends an issue if route creation timed out.
+  void SendIssueForRouteTimeout(
+      MediaCastMode cast_mode,
+      const base::string16& presentation_request_source_name);
+
+  // Creates and sends an issue if casting fails for any other reason.
+  void SendIssueForUnableToCast(MediaCastMode cast_mode);
+
+  // Returns the IssueManager associated with |router_|.
+  IssueManager* GetIssueManager();
+
+  // Instantiates and initializes the issues observer.
+  void StartObservingIssues();
+
   const base::Optional<RouteRequest> current_route_request() const {
     return current_route_request_;
   }
@@ -203,6 +224,24 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
   FRIEND_TEST_ALL_PREFIXES(MediaRouterUITest,
                            UIMediaRoutesObserverSkipsUnavailableCastModes);
 
+  // This class calls to refresh the UI when the highest priority issue is
+  // updated.
+  class UiIssuesObserver : public IssuesObserver {
+   public:
+    UiIssuesObserver(IssueManager* issue_manager, MediaRouterUIBase* ui);
+    ~UiIssuesObserver() override;
+
+    // IssuesObserver:
+    void OnIssue(const Issue& issue) override;
+    void OnIssuesCleared() override;
+
+   private:
+    // Reference back to the owning MediaRouterUIBase instance.
+    MediaRouterUIBase* const ui_;
+
+    DISALLOW_COPY_AND_ASSIGN(UiIssuesObserver);
+  };
+
   class UIMediaRoutesObserver : public MediaRoutesObserver {
    public:
     using RoutesUpdatedCallback =
@@ -213,7 +252,7 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
                           const RoutesUpdatedCallback& callback);
     ~UIMediaRoutesObserver() override;
 
-    // MediaRoutesObserver
+    // MediaRoutesObserver:
     void OnRoutesUpdated(
         const std::vector<MediaRoute>& routes,
         const std::vector<MediaRoute::Id>& joinable_route_ids) override;
@@ -225,6 +264,10 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
     DISALLOW_COPY_AND_ASSIGN(UIMediaRoutesObserver);
   };
 
+  // Called by |issues_observer_| when the top issue has changed.
+  virtual void OnIssue(const Issue& issue) = 0;
+  virtual void OnIssueCleared() = 0;
+
   // Returns the MediaRouter for this instance's BrowserContext.
   virtual MediaRouter* GetMediaRouter() const;
 
@@ -265,6 +308,8 @@ class MediaRouterUIBase : public QueryResultManager::Observer,
   // WebContents for the tab for which the Cast dialog is shown.
   content::WebContents* initiator_;
 
+  std::unique_ptr<IssuesObserver> issues_observer_;
+
 #if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
   // Keeps track of which display the initiator WebContents is on. This is used
   // to make sure we don't show a wired display presentation over the

chrome/browser/ui/media_router/ui_media_sink.h

@@ -7,6 +7,7 @@
 
 #include "base/strings/string16.h"
 #include "chrome/browser/ui/media_router/media_cast_mode.h"
+#include "chrome/common/media_router/issue.h"
 #include "chrome/common/media_router/media_sink.h"
 #include "url/gurl.h"
 
@@ -21,9 +22,7 @@ enum class UIMediaSinkState {
   // Sink has a media route.
   CONNECTED,
   // Sink is disconnected/cached (not available right now).
-  UNAVAILABLE,
-  // Sink is in an error state.
-  ERROR_STATE
+  UNAVAILABLE
 };
 
 struct UIMediaSink {
@@ -62,10 +61,9 @@ struct UIMediaSink {
   // The current state of the media sink.
   UIMediaSinkState state = UIMediaSinkState::AVAILABLE;
 
-  // Help center article ID for troubleshooting.
-  // This will show an info bubble with a tooltip for the article.
-  // This is a nullopt if there are no issues with the sink.
-  base::Optional<int> tooltip_article_id;
+  // An issue the sink is having. This is a nullopt when there are no issues
+  // with the sink.
+  base::Optional<Issue> issue;
 
   // Set of Cast Modes (e.g. presentation, desktop mirroring) supported by the
   // sink.

chrome/browser/ui/views/hover_button.cc

@@ -70,6 +70,7 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
     : views::MenuButton(text, this, false),
       title_(nullptr),
       subtitle_(nullptr),
+      secondary_icon_view_(nullptr),
       listener_(button_listener) {
   SetFocusBehavior(FocusBehavior::ALWAYS);
   SetFocusPainter(nullptr);
@@ -174,6 +175,7 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
   title_wrapper->set_can_process_events_within_subtree(false);
   grid_layout->AddView(title_wrapper);
 
+  secondary_icon_view_ = secondary_icon_view.get();
   if (secondary_icon_view) {
     columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
                        kFixed, views::GridLayout::USE_PREF, 0, 0);
@@ -335,6 +337,17 @@ views::View* HoverButton::GetTooltipHandlerForPoint(const gfx::Point& point) {
   if (!HitTestPoint(point))
     return nullptr;
 
+  // Let the secondary icon handle it if it has a tooltip.
+  if (secondary_icon_view_) {
+    gfx::Point point_in_icon_coords(point);
+    ConvertPointToTarget(this, secondary_icon_view_, &point_in_icon_coords);
+    base::string16 tooltip;
+    if (secondary_icon_view_->HitTestPoint(point_in_icon_coords) &&
+        secondary_icon_view_->GetTooltipText(point_in_icon_coords, &tooltip)) {
+      return secondary_icon_view_;
+    }
+  }
+
   // If possible, take advantage of the |views::Label| tooltip behavior, which
   // only sets the tooltip when the text is too long.
   if (title_)

chrome/browser/ui/views/hover_button.h

@@ -110,6 +110,7 @@ class HoverButton : public views::MenuButton, public views::MenuButtonListener {
  private:
   views::StyledLabel* title_;
   views::Label* subtitle_;
+  views::View* secondary_icon_view_;
 
   // The horizontal space the padding and icon take up. Used for calculating the
   // available space for |title_|, if it exists.

chrome/browser/ui/views/media_router/cast_dialog_sink_button.cc

@@ -4,8 +4,11 @@
 
 #include "chrome/browser/ui/views/media_router/cast_dialog_sink_button.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/app/vector_icons/vector_icons.h"
+#include "chrome/common/media_router/issue.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/vector_icons/vector_icons.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/gfx/color_palette.h"
@@ -66,10 +69,17 @@ std::unique_ptr<views::View> CreatePrimaryIconForSink(const UIMediaSink& sink) {
 
 std::unique_ptr<views::View> CreateSecondaryIconForSink(
     const UIMediaSink& sink) {
-  if (sink.state == UIMediaSinkState::CONNECTED) {
-    constexpr int kSecondaryIconSize = 16;
+  constexpr int kSecondaryIconSize = 16;
+  if (sink.issue) {
     auto icon_view = std::make_unique<views::ImageView>();
-    icon_view->SetImage(gfx::CreateVectorIcon(
+    icon_view->SetImage(CreateVectorIcon(::vector_icons::kInfoOutlineIcon,
+                                         kSecondaryIconSize,
+                                         gfx::kChromeIconGrey));
+    icon_view->SetTooltipText(base::UTF8ToUTF16(sink.issue->info().title));
+    return icon_view;
+  } else if (sink.state == UIMediaSinkState::CONNECTED) {
+    auto icon_view = std::make_unique<views::ImageView>();
+    icon_view->SetImage(CreateVectorIcon(
         views::kMenuCheckIcon, kSecondaryIconSize, gfx::kChromeIconGrey));
     return icon_view;
   } else if (sink.state == UIMediaSinkState::CONNECTING) {

chrome/browser/ui/views/media_router/media_router_views_ui.cc

@@ -14,6 +14,17 @@
 
 namespace media_router {
 
+namespace {
+
+// Returns true if |issue| is associated with |ui_sink|.
+bool IssueMatches(const Issue& issue, const UIMediaSink ui_sink) {
+  return issue.info().sink_id == ui_sink.id ||
+         (!issue.info().route_id.empty() &&
+          issue.info().route_id == ui_sink.route_id);
+}
+
+}  // namespace
+
 MediaRouterViewsUI::MediaRouterViewsUI() = default;
 
 MediaRouterViewsUI::~MediaRouterViewsUI() {
@@ -54,6 +65,14 @@ std::vector<MediaSinkWithCastModes> MediaRouterViewsUI::GetEnabledSinks()
   return sinks;
 }
 
+void MediaRouterViewsUI::InitCommon(content::WebContents* initiator) {
+  MediaRouterUIBase::InitCommon(initiator);
+  // We don't start observing issues in MediaRouterUIBase::InitCommon() because
+  // in the WebUI dialog, we need to wait for the WebUI to load before
+  // starting to observe.
+  StartObservingIssues();
+}
+
 void MediaRouterViewsUI::OnRoutesUpdated(
     const std::vector<MediaRoute>& routes,
     const std::vector<MediaRoute::Id>& joinable_route_ids) {
@@ -71,7 +90,7 @@ void MediaRouterViewsUI::UpdateSinks() {
           return route.media_sink_id() == sink.sink.id();
         });
     const MediaRoute* route = route_it == routes().end() ? nullptr : &*route_it;
-    model_.media_sinks.push_back(ConvertToUISink(sink, route));
+    model_.media_sinks.push_back(ConvertToUISink(sink, route, issue_));
   }
   for (CastDialogController::Observer& observer : observers_)
     observer.OnModelUpdated(model_);
@@ -79,7 +98,8 @@ void MediaRouterViewsUI::UpdateSinks() {
 
 UIMediaSink MediaRouterViewsUI::ConvertToUISink(
     const MediaSinkWithCastModes& sink,
-    const MediaRoute* route) {
+    const MediaRoute* route,
+    const base::Optional<Issue>& issue) {
   UIMediaSink ui_sink;
   ui_sink.id = sink.sink.id();
   ui_sink.friendly_name = base::UTF8ToUTF16(sink.sink.name());
@@ -96,7 +116,19 @@ UIMediaSink MediaRouterViewsUI::ConvertToUISink(
                         : UIMediaSinkState::AVAILABLE;
     ui_sink.cast_modes = sink.cast_modes;
   }
+  if (issue && IssueMatches(*issue, ui_sink))
+    ui_sink.issue = issue;
   return ui_sink;
 }
 
+void MediaRouterViewsUI::OnIssue(const Issue& issue) {
+  issue_ = issue;
+  UpdateSinks();
+}
+
+void MediaRouterViewsUI::OnIssueCleared() {
+  issue_ = base::nullopt;
+  UpdateSinks();
+}
+
 }  // namespace media_router

chrome/browser/ui/views/media_router/media_router_views_ui.h

@@ -34,15 +34,23 @@ class MediaRouterViewsUI : public MediaRouterUIBase,
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, NotifyObserver);
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, RemovePseudoSink);
   FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, ConnectingState);
+  FRIEND_TEST_ALL_PREFIXES(MediaRouterViewsUITest, AddAndRemoveIssue);
 
   // MediaRouterUIBase:
+  void InitCommon(content::WebContents* initiator) override;
   void OnRoutesUpdated(
       const std::vector<MediaRoute>& routes,
       const std::vector<MediaRoute::Id>& joinable_route_ids) override;
   void UpdateSinks() override;
+  void OnIssue(const Issue& issue) override;
+  void OnIssueCleared() override;
+
+  // This value is set whenever there is an outstanding issue.
+  base::Optional<Issue> issue_;
 
   UIMediaSink ConvertToUISink(const MediaSinkWithCastModes& sink,
-                              const MediaRoute* route);
+                              const MediaRoute* route,
+                              const base::Optional<Issue>& issue);
 
   // Contains up-to-date data to show in the dialog.
   CastDialogModel model_;

chrome/browser/ui/views/media_router/media_router_views_ui_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/media/router/media_sinks_observer.h"
 #include "chrome/browser/media/router/test/mock_media_router.h"
+#include "chrome/browser/media/router/test/test_helper.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/ui/media_router/cast_dialog_controller.h"
 #include "chrome/browser/ui/media_router/media_cast_mode.h"
@@ -183,7 +184,49 @@ TEST_F(MediaRouterViewsUITest, ConnectingState) {
       })));
   MediaRoute route(kRouteId, MediaSource(kSourceId), kSinkId, "", true, true);
   ui_->OnRoutesUpdated({route}, {});
+  ui_->RemoveObserver(&observer);
+}
+
+TEST_F(MediaRouterViewsUITest, AddAndRemoveIssue) {
+  MediaSink sink1("sink_id1", "Sink 1", SinkIconType::CAST_AUDIO);
+  MediaSinkWithCastModes sink1_with_cast_modes(sink1);
+  sink1_with_cast_modes.cast_modes = {MediaCastMode::TAB_MIRROR};
+  MediaSink sink2("sink_id2", "Sink 2", SinkIconType::CAST_AUDIO);
+  MediaSinkWithCastModes sink2_with_cast_modes(sink2);
+  sink2_with_cast_modes.cast_modes = {MediaCastMode::TAB_MIRROR};
+  ui_->OnResultsUpdated({sink1_with_cast_modes, sink2_with_cast_modes});
 
+  MockControllerObserver observer;
+  ui_->AddObserver(&observer);
+  MockIssuesObserver issues_observer(mock_router_.GetIssueManager());
+  issues_observer.Init();
+  const std::string issue_title("Issue 1");
+  IssueInfo issue(issue_title, IssueInfo::Action::DISMISS,
+                  IssueInfo::Severity::WARNING);
+  issue.sink_id = sink2.id();
+  Issue::Id issue_id = -1;
+
+  EXPECT_CALL(issues_observer, OnIssue)
+      .WillOnce(
+          Invoke([&issue_id](const Issue& issue) { issue_id = issue.id(); }));
+  EXPECT_CALL(observer, OnModelUpdated(_))
+      .WillOnce(WithArg<0>(
+          Invoke([&sink1, &sink2, &issue_title](const CastDialogModel& model) {
+            EXPECT_EQ(2u, model.media_sinks.size());
+            EXPECT_EQ(model.media_sinks[0].id, sink1.id());
+            EXPECT_FALSE(model.media_sinks[0].issue.has_value());
+            EXPECT_EQ(model.media_sinks[1].id, sink2.id());
+            EXPECT_EQ(model.media_sinks[1].issue->info().title, issue_title);
+          })));
+  mock_router_.GetIssueManager()->AddIssue(issue);
+
+  EXPECT_CALL(observer, OnModelUpdated(_))
+      .WillOnce(WithArg<0>(Invoke([&sink2](const CastDialogModel& model) {
+        EXPECT_EQ(2u, model.media_sinks.size());
+        EXPECT_EQ(model.media_sinks[1].id, sink2.id());
+        EXPECT_FALSE(model.media_sinks[1].issue.has_value());
+      })));
+  mock_router_.GetIssueManager()->ClearIssue(issue_id);
   ui_->RemoveObserver(&observer);
 }
 

chrome/browser/ui/views/profiles/profile_chooser_view.cc

@@ -1186,6 +1186,7 @@ views::View* ProfileChooserView::CreateDiceSigninView() {
     // Make sure that the arrow is flipped in RTL mode.
     submenu_arrow_icon_view->EnableCanvasFlippingForRTLUI(true);
   }
+  LOG(ERROR) << "----------- use secondary icon";
   sync_to_another_account_button_ = new HoverButton(
       this, std::move(switch_account_icon_view),
       l10n_util::GetStringUTF16(

chrome/browser/ui/webui/media_router/media_router_ui.cc

@@ -16,7 +16,6 @@
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "chrome/browser/media/router/issue_manager.h"
-#include "chrome/browser/media/router/issues_observer.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"
@@ -63,28 +62,6 @@
 
 namespace media_router {
 
-// This class calls to refresh the UI when the highest priority issue is
-// updated.
-class MediaRouterUI::UIIssuesObserver : public IssuesObserver {
- public:
-  UIIssuesObserver(IssueManager* issue_manager, MediaRouterUI* ui)
-      : IssuesObserver(issue_manager), ui_(ui) {
-    DCHECK(ui);
-  }
-
-  ~UIIssuesObserver() override {}
-
-  // IssuesObserver implementation.
-  void OnIssue(const Issue& issue) override { ui_->SetIssue(issue); }
-  void OnIssuesCleared() override { ui_->ClearIssue(); }
-
- private:
-  // Reference back to the owning MediaRouterUI instance.
-  MediaRouterUI* ui_;
-
-  DISALLOW_COPY_AND_ASSIGN(UIIssuesObserver);
-};
-
 // Observes a WebContents and requests fullscreening of its first
 // video element.  The request is sent after the WebContents is loaded and tab
 // capture has begun. Marked final to prevent inheritance so delete calls are
@@ -231,9 +208,7 @@ void MediaRouterUI::OnUIInitialized() {
   // TODO(imcheng): We should be able to instantiate |issue_observer_| during
   // InitCommon by storing an initial Issue in this class.
   // Register for Issue updates.
-  issues_observer_ =
-      std::make_unique<UIIssuesObserver>(GetIssueManager(), this);
-  issues_observer_->Init();
+  StartObservingIssues();
 }
 
 bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id,
@@ -242,6 +217,8 @@ bool MediaRouterUI::CreateRoute(const MediaSink::Id& sink_id,
   // necessary.
   content::WebContents* tab_contents = initiator();
 
+  // TODO(http://crbug.com/842787): Code is duplicated between MRUI and MRUIBase
+  // until MRUIBase supports local file casting.
   base::Optional<RouteParameters> params;
   if (cast_mode == MediaCastMode::LOCAL_FILE) {
     GURL url = media_router_file_dialog_->GetLastSelectedFileUrl();
@@ -279,14 +256,6 @@ bool MediaRouterUI::ConnectRoute(const MediaSink::Id& sink_id,
   return true;
 }
 
-void MediaRouterUI::AddIssue(const IssueInfo& issue) {
-  GetIssueManager()->AddIssue(issue);
-}
-
-void MediaRouterUI::ClearIssue(const Issue::Id& issue_id) {
-  GetIssueManager()->ClearIssue(issue_id);
-}
-
 void MediaRouterUI::OpenFileDialog() {
   if (!media_router_file_dialog_) {
     media_router_file_dialog_ = std::make_unique<MediaRouterFileDialog>(this);
@@ -478,12 +447,12 @@ void MediaRouterUI::FileDialogSelectionFailed(const IssueInfo& issue) {
   AddIssue(issue);
 }
 
-void MediaRouterUI::SetIssue(const Issue& issue) {
+void MediaRouterUI::OnIssue(const Issue& issue) {
   if (ui_initialized_)
     handler_->UpdateIssue(issue);
 }
 
-void MediaRouterUI::ClearIssue() {
+void MediaRouterUI::OnIssueCleared() {
   if (ui_initialized_)
     handler_->ClearIssue();
 }
@@ -542,47 +511,6 @@ void MediaRouterUI::OnSearchSinkResponseReceived(
   CreateRoute(found_sink_id, cast_mode);
 }
 
-void MediaRouterUI::SendIssueForRouteTimeout(
-    MediaCastMode cast_mode,
-    const base::string16& presentation_request_source_name) {
-  std::string issue_title;
-  switch (cast_mode) {
-    case PRESENTATION:
-      DLOG_IF(ERROR, presentation_request_source_name.empty())
-          << "Empty presentation request source name.";
-      issue_title =
-          l10n_util::GetStringFUTF8(IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT,
-                                    presentation_request_source_name);
-      break;
-    case TAB_MIRROR:
-      issue_title = l10n_util::GetStringUTF8(
-          IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_TAB);
-      break;
-    case DESKTOP_MIRROR:
-      issue_title = l10n_util::GetStringUTF8(
-          IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_DESKTOP);
-      break;
-    default:
-      NOTREACHED();
-  }
-
-  AddIssue(IssueInfo(issue_title, IssueInfo::Action::DISMISS,
-                     IssueInfo::Severity::NOTIFICATION));
-}
-
-void MediaRouterUI::SendIssueForUnableToCast(MediaCastMode cast_mode) {
-  // For a generic error, claim a tab error unless it was specifically desktop
-  // mirroring.
-  std::string issue_title =
-      (cast_mode == MediaCastMode::DESKTOP_MIRROR)
-          ? l10n_util::GetStringUTF8(
-                IDS_MEDIA_ROUTER_ISSUE_UNABLE_TO_CAST_DESKTOP)
-          : l10n_util::GetStringUTF8(
-                IDS_MEDIA_ROUTER_ISSUE_CREATE_ROUTE_TIMEOUT_FOR_TAB);
-  AddIssue(IssueInfo(issue_title, IssueInfo::Action::DISMISS,
-                     IssueInfo::Severity::WARNING));
-}
-
 void MediaRouterUI::InitCommon(content::WebContents* initiator) {
   MediaRouterUIBase::InitCommon(initiator);
   UpdateCastModes();
@@ -698,10 +626,6 @@ void MediaRouterUI::UpdateMediaRouteStatus(const MediaStatus& status) {
   handler_->UpdateMediaRouteStatus(status);
 }
 
-IssueManager* MediaRouterUI::GetIssueManager() {
-  return GetMediaRouter()->GetIssueManager();
-}
-
 void MediaRouterUI::UpdateSinks() {
   if (ui_initialized_)
     handler_->UpdateSinks(GetEnabledSinks());

chrome/browser/ui/webui/media_router/media_router_ui.h

@@ -22,9 +22,6 @@ struct SelectedFileInfo;
 namespace media_router {
 
 struct MediaStatus;
-class Issue;
-class IssueManager;
-class IssuesObserver;
 class MediaRouterWebUIMessageHandler;
 class RouteRequestResult;
 
@@ -52,12 +49,6 @@ class MediaRouterUI
   bool ConnectRoute(const MediaSink::Id& sink_id,
                     const MediaRoute::Id& route_id);
 
-  // Calls MediaRouter to add the given issue.
-  void AddIssue(const IssueInfo& issue);
-
-  // Calls MediaRouter to clear the given issue.
-  void ClearIssue(const Issue::Id& issue_id);
-
   // Called to open a file dialog with the media_router_ui file dialog handler.
   void OpenFileDialog();
 
@@ -178,11 +169,8 @@ class MediaRouterUI
   void FileDialogFileSelected(const ui::SelectedFileInfo& file_info) override;
   void FileDialogSelectionFailed(const IssueInfo& issue) override;
 
-  // Called by |issues_observer_| when the top issue has changed.
-  // If the UI is already initialized, notifies |handler_| to update the UI.
-  // Ignored if the UI is not yet initialized.
-  void SetIssue(const Issue& issue);
-  void ClearIssue();
+  void OnIssue(const Issue& issue) override;
+  void OnIssueCleared() override;
 
   void OnRoutesUpdated(
       const std::vector<MediaRoute>& routes,
@@ -207,14 +195,6 @@ class MediaRouterUI
   void OnSearchSinkResponseReceived(MediaCastMode cast_mode,
                                     const MediaSink::Id& found_sink_id);
 
-  // Creates and sends an issue if route creation timed out.
-  void SendIssueForRouteTimeout(
-      MediaCastMode cast_mode,
-      const base::string16& presentation_request_source_name);
-
-  // Creates and sends an issue if casting fails for any other reason.
-  void SendIssueForUnableToCast(MediaCastMode cast_mode);
-
   void InitCommon(content::WebContents* initiator) override;
 
   // PresentationServiceDelegateImpl::DefaultPresentationObserver:
@@ -253,9 +233,6 @@ class MediaRouterUI
   // handler of a media status update for the route currently shown in the UI.
   void UpdateMediaRouteStatus(const MediaStatus& status);
 
-  // Returns the IssueManager associated with |router_|.
-  IssueManager* GetIssueManager();
-
   void UpdateSinks() override;
 
   MediaRouter* GetMediaRouter() const override;
@@ -264,8 +241,6 @@ class MediaRouterUI
   // only after it has deleted |this|.
   MediaRouterWebUIMessageHandler* handler_ = nullptr;
 
-  std::unique_ptr<IssuesObserver> issues_observer_;
-
   // Set to true by |handler_| when the UI has been initialized.
   bool ui_initialized_;
 

chrome/browser/ui/webui/media_router/media_router_webui_message_handler.cc

@@ -645,7 +645,7 @@ void MediaRouterWebUIMessageHandler::OnActOnIssue(const base::ListValue* args) {
       static_cast<IssueInfo::Action>(action_type_num);
   if (ActOnIssueType(action_type, args_dict))
     DVLOG(1) << "ActOnIssueType failed for Issue ID " << issue_id;
-  media_router_ui_->ClearIssue(issue_id);
+  media_router_ui_->RemoveIssue(issue_id);
 }
 
 void MediaRouterWebUIMessageHandler::OnJoinRoute(const base::ListValue* args) {

chrome/common/media_router/issue.h

@@ -10,6 +10,7 @@
 
 #include "base/logging.h"
 #include "chrome/common/media_router/media_route.h"
+#include "chrome/common/media_router/media_sink.h"
 
 namespace media_router {
 
@@ -62,7 +63,11 @@ struct IssueInfo {
 
   // ID of route associated with the Issue, or empty if no route is associated
   // with it.
-  std::string route_id;
+  MediaRoute::Id route_id;
+
+  // ID of the sink associated with this issue, or empty if no sink is
+  // associated with it.
+  MediaSink::Id sink_id;
 
   // |true| if the issue needs to be resolved before continuing. Note that a
   // Issue of severity FATAL is considered blocking by default.

chrome/common/media_router/mojo/media_router.mojom

@@ -150,8 +150,13 @@ struct Issue {
   };
 
   // If set, the ID of the route to which this issue pertains.
-  // If not set (default), then this is a global issue.
-  string? route_id;
+  // If this is an empty string (default), then this is a global issue.
+  string route_id;
+
+  // The ID of the sink associated with this issue.
+  // If this is an empty string (default), then this is a global issue.
+  // TODO(https://crbug.com/554646): Associate all issues with sinks.
+  string sink_id;
 
   Severity severity;
 

chrome/common/media_router/mojo/media_router_struct_traits.cc

@@ -33,11 +33,11 @@ bool StructTraits<media_router::mojom::IssueDataView, media_router::IssueInfo>::
   if (!data.ReadSecondaryActions(&out->secondary_actions))
     return false;
 
-  base::Optional<std::string> route_id;
-  if (!data.ReadRouteId(&route_id))
+  if (!data.ReadRouteId(&out->route_id))
     return false;
 
-  out->route_id = route_id.value_or(std::string());
+  if (!data.ReadSinkId(&out->sink_id))
+    return false;
 
   out->is_blocking = data.is_blocking();
   out->help_page_id = data.help_page_id();

chrome/common/media_router/mojo/media_router_struct_traits.h

@@ -209,6 +209,10 @@ struct StructTraits<media_router::mojom::IssueDataView,
     return issue.route_id;
   }
 
+  static const std::string& sink_id(const media_router::IssueInfo& issue) {
+    return issue.sink_id;
+  }
+
   static media_router::IssueInfo::Severity severity(
       const media_router::IssueInfo& issue) {
     return issue.severity;

chrome/renderer/resources/extensions/media_router_bindings.js

@@ -896,7 +896,8 @@ MediaRouter.prototype.onIssue = function(issue) {
     'defaultAction': issueActionToMojo_(issue.defaultAction),
     'secondaryActions': secondaryActions,
     'helpPageId': issue.helpPageId,
-    'isBlocking': issue.isBlocking
+    'isBlocking': issue.isBlocking,
+    'sinkId': issue.sinkId || ''
   }));
 };