[Media Router] Add logs in MediaRouterViewsUI

- Log the current available media sinks users can see in the Cast Dialog
when users close the window.
- Log when useres start/stop a cast session.
- Replace some DLOG()s with the native component logger.

Bug: 687380, b/154273698
Change-Id: I383ea07adc0a5b1a9a59898be17127f0cd87dc20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303568
Commit-Queue: Muyao Xu <muyaoxu@google.com>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794730}

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

@@ -98,6 +98,8 @@ void MediaRouterDialogControllerViews::Reset() {
 
 void MediaRouterDialogControllerViews::OnWidgetClosing(views::Widget* widget) {
   DCHECK(scoped_widget_observer_.IsObserving(widget));
+  if (ui_)
+    ui_->LogMediaSinkStatus();
   Reset();
   scoped_widget_observer_.Remove(widget);
 }

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

@@ -14,7 +14,9 @@
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
@@ -63,6 +65,8 @@ namespace media_router {
 
 namespace {
 
+constexpr char kLoggerComponent[] = "MediaRouterViewsUI";
+
 // 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 ||
@@ -260,7 +264,8 @@ class MediaRouterViewsUI::WebContentsFullscreenOnLoadedObserver final
 
 MediaRouterViewsUI::MediaRouterViewsUI(content::WebContents* initiator)
     : presentation_manager_(WebContentsPresentationManager::Get(initiator)),
-      initiator_(initiator) {
+      initiator_(initiator),
+      logger_(GetMediaRouter()->GetLogger()) {
   CHECK(initiator_);
   if (presentation_manager_)
     presentation_manager_->AddObserver(this);
@@ -362,6 +367,9 @@ void MediaRouterViewsUI::InitWithStartPresentationContext(
 
 bool MediaRouterViewsUI::CreateRoute(const MediaSink::Id& sink_id,
                                      MediaCastMode cast_mode) {
+  logger_->LogInfo(mojom::LogCategory::kUi, kLoggerComponent,
+                   "CreateRoute requested by MediaRouterViewsUI.", sink_id, "",
+                   "");
 #if defined(OS_MAC)
   if (RequiresScreenCapturePermission(cast_mode)) {
     const bool screen_capture_allowed =
@@ -422,6 +430,9 @@ bool MediaRouterViewsUI::CreateRoute(const MediaSink::Id& sink_id,
 }
 
 void MediaRouterViewsUI::TerminateRoute(const MediaRoute::Id& route_id) {
+  logger_->LogInfo(mojom::LogCategory::kUi, kLoggerComponent,
+                   "TerminateRoute requested by MediaRouterViewsUI.", "", "",
+                   MediaRoute::GetPresentationIdFromMediaRouteId(route_id));
   GetMediaRouter()->TerminateRoute(route_id);
 }
 
@@ -480,6 +491,22 @@ base::string16 MediaRouterViewsUI::GetPresentationRequestSourceName() const {
 
 void MediaRouterViewsUI::AddIssue(const IssueInfo& issue) {
   GetIssueManager()->AddIssue(issue);
+  switch (issue.severity) {
+    case IssueInfo::Severity::NOTIFICATION:
+      logger_->LogInfo(
+          mojom::LogCategory::kUi, kLoggerComponent, issue.message,
+          issue.sink_id,
+          MediaRoute::GetPresentationIdFromMediaRouteId(issue.route_id),
+          MediaRoute::GetMediaSourceIdFromMediaRouteId(issue.route_id));
+      break;
+    default:
+      logger_->LogError(
+          mojom::LogCategory::kUi, kLoggerComponent, issue.message,
+          issue.sink_id,
+          MediaRoute::GetMediaSourceIdFromMediaRouteId(issue.route_id),
+          MediaRoute::GetPresentationIdFromMediaRouteId(issue.route_id));
+      break;
+  }
 }
 
 void MediaRouterViewsUI::RemoveIssue(const Issue::Id& issue_id) {
@@ -495,6 +522,24 @@ void MediaRouterViewsUI::OpenFileDialog() {
   media_router_file_dialog_->OpenFileDialog(GetBrowser());
 }
 
+void MediaRouterViewsUI::LogMediaSinkStatus() {
+  std::vector<std::string> sink_ids;
+  for (const auto& sink : GetEnabledSinks()) {
+    if (sink.sink.id().length() <= 4) {
+      sink_ids.push_back(sink.sink.id());
+    } else {
+      sink_ids.push_back(sink.sink.id().substr(sink.sink.id().length() - 4));
+    }
+  }
+  logger_->LogInfo(
+      mojom::LogCategory::kUi, kLoggerComponent,
+      base::StrCat(
+          {base::StringPrintf("%zu sinks available on CastDialogView closed: ",
+                              sink_ids.size()),
+           base::JoinString(sink_ids, ",")}),
+      "", "", "");
+}
+
 MediaRouterViewsUI::RouteRequest::RouteRequest(const MediaSink::Id& sink_id)
     : sink_id(sink_id) {
   static base::AtomicSequenceNumber g_next_request_id;
@@ -668,23 +713,27 @@ base::Optional<RouteParameters> MediaRouterViewsUI::GetRouteParameters(
       query_result_manager_->GetSourceForCastModeAndSink(cast_mode, sink_id);
 
   if (!source) {
-    LOG(ERROR) << "No corresponding MediaSource for cast mode "
-               << static_cast<int>(cast_mode) << " and sink " << sink_id;
+    logger_->LogError(
+        mojom::LogCategory::kUi, kLoggerComponent,
+        base::StringPrintf("No corresponding MediaSource for cast mode %d.",
+                           static_cast<int>(cast_mode)),
+        sink_id, "", "");
     return base::nullopt;
   }
   params.source_id = source->id();
 
   bool for_presentation_source = cast_mode == MediaCastMode::PRESENTATION;
   if (for_presentation_source && !presentation_request_) {
-    DLOG(ERROR) << "Requested to create a route for presentation, but "
-                << "presentation request is missing.";
+    logger_->LogError(mojom::LogCategory::kUi, kLoggerComponent,
+                      "Requested to create a route for presentation, but "
+                      "presentation request is missing.",
+                      sink_id, source->id(), "");
     return base::nullopt;
   }
 
   current_route_request_ = base::make_optional<RouteRequest>(sink_id);
   params.origin = for_presentation_source ? presentation_request_->frame_origin
                                           : url::Origin::Create(GURL());
-  DVLOG(1) << "DoCreateRoute: origin: " << params.origin;
 
   // This callback must be invoked before
   // HandleCreateSessionRequestRouteResponse(), which closes the dialog and
@@ -878,7 +927,6 @@ void MediaRouterViewsUI::OnRouteResponseReceived(
     MediaCastMode cast_mode,
     const base::string16& presentation_request_source_name,
     const RouteRequestResult& result) {
-  DVLOG(1) << "OnRouteResponseReceived";
   // If we receive a new route that we aren't expecting, do nothing.
   if (!current_route_request_ || route_request_id != current_route_request_->id)
     return;
@@ -886,7 +934,9 @@ void MediaRouterViewsUI::OnRouteResponseReceived(
   const MediaRoute* route = result.route();
   if (!route) {
     // The provider will handle sending an issue for a failed route request.
-    DVLOG(1) << "MediaRouteResponse returned error: " << result.error();
+    logger_->LogError(mojom::LogCategory::kUi, kLoggerComponent,
+                      "MediaRouteResponse returned error: " + result.error(),
+                      sink_id, "", "");
   }
 
   current_route_request_.reset();

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

@@ -116,6 +116,9 @@ class MediaRouterViewsUI
   // Opens a file picker for when the user selected local file casting.
   void OpenFileDialog();
 
+  // Uses LoggerImpl to log current available sinks.
+  void LogMediaSinkStatus();
+
   const std::vector<MediaRoute>& routes() const { return routes_; }
   content::WebContents* initiator() const { return initiator_; }
 
@@ -411,6 +414,7 @@ class MediaRouterViewsUI
 #if defined(OS_MAC)
   base::Optional<bool> screen_capture_allowed_for_testing_;
 #endif
+  LoggerImpl* logger_;
 
   // NOTE: Weak pointers must be invalidated before all other member variables.
   // Therefore |weak_factory_| must be placed at the end.

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

@@ -141,6 +141,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
     SetMediaRouterFactory();
     mock_router_ = static_cast<MockMediaRouter*>(
         MediaRouterFactory::GetApiForBrowserContext(GetBrowserContext()));
+    logger_ = std::make_unique<LoggerImpl>();
 
     // Store sink observers so that they can be notified in tests.
     ON_CALL(*mock_router_, RegisterMediaSinksObserver(_))
@@ -148,6 +149,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
           media_sinks_observers_.push_back(observer);
           return true;
         });
+    ON_CALL(*mock_router_, GetLogger()).WillByDefault(Return(logger_.get()));
 
     CreateSessionServiceTabHelper(web_contents());
     ui_ = std::make_unique<MediaRouterViewsUI>(web_contents());
@@ -251,6 +253,7 @@ class MediaRouterViewsUITest : public ChromeRenderViewHostTestHarness {
   MockMediaRouter* mock_router_ = nullptr;
   std::unique_ptr<MediaRouterViewsUI> ui_;
   std::unique_ptr<StartPresentationContext> start_presentation_context_;
+  std::unique_ptr<LoggerImpl> logger_;
   content::PresentationRequest presentation_request_{
       {0, 0},
       {GURL("https://google.com/presentation")},