[Harmony Cast Dialog] Make UMA metrics support the dialog

Make the Harmony dialog record the following metrics. These are existing
metrics already used by the WebUI dialog.

MediaRouter.Ui.Device.Count
MediaRouter.Ui.Action.StartLocalPosition
MediaRouter.Ui.Dialog.Paint

Bug: 853369
Change-Id: Ie49a969faa0d91ec8026a55b2f2be15dd25f26ea
Reviewed-on: https://chromium-review.googlesource.com/1119055
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571979}

chrome/browser/media/router/media_router_metrics.cc

@@ -59,12 +59,18 @@ const char MediaRouterMetrics::kHistogramPresentationUrlType[] =
     "MediaRouter.PresentationRequest.AvailabilityUrlType";
 const char MediaRouterMetrics::kHistogramRouteCreationOutcome[] =
     "MediaRouter.Route.CreationOutcome";
+const char MediaRouterMetrics::kHistogramUiDeviceCount[] =
+    "MediaRouter.Ui.Device.Count";
 const char MediaRouterMetrics::kHistogramUiDialogPaint[] =
     "MediaRouter.Ui.Dialog.Paint";
 const char MediaRouterMetrics::kHistogramUiDialogLoadedWithData[] =
     "MediaRouter.Ui.Dialog.LoadedWithData";
 const char MediaRouterMetrics::kHistogramUiFirstAction[] =
     "MediaRouter.Ui.FirstAction";
+const char MediaRouterMetrics::kHistogramStartLocalPosition[] =
+    "MediaRouter.Ui.Action.StartLocalPosition";
+const char MediaRouterMetrics::kHistogramStartLocalSessionSuccessful[] =
+    "MediaRouter.Ui.Action.StartLocalSessionSuccessful";
 
 // static
 void MediaRouterMetrics::RecordMediaRouterDialogOrigin(
@@ -148,4 +154,19 @@ void MediaRouterMetrics::RecordMediaSinkType(SinkIconType sink_icon_type) {
                             SinkIconType::TOTAL_COUNT);
 }
 
+// static
+void MediaRouterMetrics::RecordDeviceCount(int device_count) {
+  UMA_HISTOGRAM_COUNTS_100(kHistogramUiDeviceCount, device_count);
+}
+
+// static
+void MediaRouterMetrics::RecordStartRouteDeviceIndex(int index) {
+  base::UmaHistogramSparse(kHistogramStartLocalPosition, std::min(index, 100));
+}
+
+// static
+void MediaRouterMetrics::RecordStartLocalSessionSuccessful(bool success) {
+  UMA_HISTOGRAM_BOOLEAN(kHistogramStartLocalSessionSuccessful, success);
+}
+
 }  // namespace media_router

chrome/browser/media/router/media_router_metrics.h

@@ -89,9 +89,12 @@ class MediaRouterMetrics {
   static const char kHistogramMediaSinkType[];
   static const char kHistogramPresentationUrlType[];
   static const char kHistogramRouteCreationOutcome[];
+  static const char kHistogramUiDeviceCount[];
   static const char kHistogramUiDialogPaint[];
   static const char kHistogramUiDialogLoadedWithData[];
   static const char kHistogramUiFirstAction[];
+  static const char kHistogramStartLocalPosition[];
+  static const char kHistogramStartLocalSessionSuccessful[];
 
   // Records where the user clicked to open the Media Router dialog.
   static void RecordMediaRouterDialogOrigin(
@@ -135,6 +138,17 @@ class MediaRouterMetrics {
 
   // Records the type of the sink that media is being Cast to.
   static void RecordMediaSinkType(SinkIconType sink_icon_type);
+
+  // Records the number of devices shown in the Cast dialog. The device count
+  // may be 0.
+  static void RecordDeviceCount(int device_count);
+
+  // Records the index of the device the user has started casting to on the
+  // devices list. The index starts at 0.
+  static void RecordStartRouteDeviceIndex(int index);
+
+  // Records whether or not an attempt to start casting was successful.
+  static void RecordStartLocalSessionSuccessful(bool success);
 };
 
 }  // namespace media_router

chrome/browser/media/router/media_router_metrics_unittest.cc

@@ -189,4 +189,45 @@ TEST(MediaRouterMetricsTest, RecordMediaSinkType) {
                   Bucket(static_cast<int>(SinkIconType::GENERIC), 1)));
 }
 
+TEST(MediaRouterMetricsTest, RecordDeviceCount) {
+  base::HistogramTester tester;
+  tester.ExpectTotalCount(MediaRouterMetrics::kHistogramUiDeviceCount, 0);
+
+  MediaRouterMetrics::RecordDeviceCount(30);
+  MediaRouterMetrics::RecordDeviceCount(0);
+
+  tester.ExpectTotalCount(MediaRouterMetrics::kHistogramUiDeviceCount, 2);
+  EXPECT_THAT(tester.GetAllSamples(MediaRouterMetrics::kHistogramUiDeviceCount),
+              ElementsAre(Bucket(0, 1), Bucket(30, 1)));
+}
+
+TEST(MediaRouterMetricsTest, RecordStartRouteDeviceIndex) {
+  base::HistogramTester tester;
+  tester.ExpectTotalCount(MediaRouterMetrics::kHistogramStartLocalPosition, 0);
+
+  MediaRouterMetrics::RecordStartRouteDeviceIndex(30);
+  MediaRouterMetrics::RecordStartRouteDeviceIndex(0);
+
+  tester.ExpectTotalCount(MediaRouterMetrics::kHistogramStartLocalPosition, 2);
+  EXPECT_THAT(
+      tester.GetAllSamples(MediaRouterMetrics::kHistogramStartLocalPosition),
+      ElementsAre(Bucket(0, 1), Bucket(30, 1)));
+}
+
+TEST(MediaRouterMetricsTest, RecordStartLocalSessionSuccessful) {
+  base::HistogramTester tester;
+  tester.ExpectTotalCount(
+      MediaRouterMetrics::kHistogramStartLocalSessionSuccessful, 0);
+
+  MediaRouterMetrics::RecordStartLocalSessionSuccessful(true);
+  MediaRouterMetrics::RecordStartLocalSessionSuccessful(false);
+  MediaRouterMetrics::RecordStartLocalSessionSuccessful(true);
+
+  tester.ExpectTotalCount(
+      MediaRouterMetrics::kHistogramStartLocalSessionSuccessful, 3);
+  EXPECT_THAT(tester.GetAllSamples(
+                  MediaRouterMetrics::kHistogramStartLocalSessionSuccessful),
+              ElementsAre(Bucket(false, 1), Bucket(true, 2)));
+}
+
 }  // namespace media_router

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

@@ -4,7 +4,10 @@
 
 #include "chrome/browser/ui/views/media_router/cast_dialog_view.h"
 
+#include "base/location.h"
 #include "base/optional.h"
+#include "base/time/time.h"
+#include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/media_router/cast_dialog_controller.h"
 #include "chrome/browser/ui/media_router/cast_dialog_model.h"
@@ -15,6 +18,7 @@
 #include "chrome/browser/ui/views/media_router/cast_dialog_sink_button.h"
 #include "chrome/common/media_router/media_sink.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/browser_thread.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/native_widget_types.h"
@@ -52,9 +56,11 @@ bool SupportsDesktopSource(const UIMediaSink& sink) {
 // static
 void CastDialogView::ShowDialog(views::View* anchor_view,
                                 CastDialogController* controller,
-                                Browser* browser) {
+                                Browser* browser,
+                                const base::Time& start_time) {
   DCHECK(!instance_);
-  instance_ = new CastDialogView(anchor_view, controller, browser);
+  DCHECK(!start_time.is_null());
+  instance_ = new CastDialogView(anchor_view, controller, browser, start_time);
   views::Widget* widget =
       views::BubbleDialogDelegateView::CreateBubble(instance_);
   widget->Show();
@@ -125,6 +131,7 @@ bool CastDialogView::Accept() {
     for (MediaCastMode cast_mode : {PRESENTATION, TAB_MIRROR, DESKTOP_MIRROR}) {
       if ((cast_mode & selected_source_) &&
           base::ContainsKey(sink.cast_modes, cast_mode)) {
+        MediaRouterMetrics::RecordStartRouteDeviceIndex(selected_sink_index_);
         controller_->StartCasting(sink.id, cast_mode);
         break;
       }
@@ -161,11 +168,14 @@ void CastDialogView::OnControllerInvalidated() {
 
 CastDialogView::CastDialogView(views::View* anchor_view,
                                CastDialogController* controller,
-                               Browser* browser)
+                               Browser* browser,
+                               const base::Time& start_time)
     : BubbleDialogDelegateView(anchor_view, views::BubbleBorder::TOP_RIGHT),
       selected_source_(kTabSource),
       controller_(controller),
-      browser_(browser) {
+      browser_(browser),
+      start_time_(start_time),
+      weak_factory_(this) {
   ShowNoSinksView();
 }
 
@@ -200,6 +210,15 @@ gfx::Size CastDialogView::CalculatePreferredSize() const {
   return gfx::Size(width, GetHeightForWidth(width));
 }
 
+void CastDialogView::OnPaint(gfx::Canvas* canvas) {
+  views::BubbleDialogDelegateView::OnPaint(canvas);
+  if (!start_time_.is_null()) {
+    MediaRouterMetrics::RecordMediaRouterDialogPaint(base::Time::Now() -
+                                                     start_time_);
+    start_time_ = base::Time();
+  }
+}
+
 void CastDialogView::Init() {
   auto* provider = ChromeLayoutProvider::Get();
   set_margins(
@@ -211,6 +230,7 @@ void CastDialogView::Init() {
                   0));
   SetLayoutManager(std::make_unique<views::FillLayout>());
   controller_->AddObserver(this);
+  RecordSinkCountWithDelay();
 }
 
 void CastDialogView::WindowClosing() {
@@ -335,6 +355,20 @@ void CastDialogView::MaybeSizeToContents() {
     SizeToContents();
 }
 
+void CastDialogView::RecordSinkCountWithDelay() {
+  // Record the number of sinks after three seconds. This is consistent with the
+  // WebUI dialog.
+  content::BrowserThread::PostDelayedTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&CastDialogView::RecordSinkCount,
+                     weak_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(3));
+}
+
+void CastDialogView::RecordSinkCount() {
+  media_router::MediaRouterMetrics::RecordDeviceCount(sink_buttons_.size());
+}
+
 // static
 CastDialogView* CastDialogView::instance_ = nullptr;
 

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

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ui/media_router/cast_dialog_controller.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
@@ -16,6 +17,10 @@
 
 class Browser;
 
+namespace gfx {
+class Canvas;
+}  // namespace gfx
+
 namespace media_router {
 
 class CastDialogSinkButton;
@@ -33,7 +38,8 @@ class CastDialogView : public views::BubbleDialogDelegateView,
   // currently shown.
   static void ShowDialog(views::View* anchor_view,
                          CastDialogController* controller,
-                         Browser* browser);
+                         Browser* browser,
+                         const base::Time& start_time);
 
   // No-op if the dialog is currently not shown.
   static void HideDialog();
@@ -68,6 +74,7 @@ class CastDialogView : public views::BubbleDialogDelegateView,
 
   // views::View:
   gfx::Size CalculatePreferredSize() const override;
+  void OnPaint(gfx::Canvas* canvas) override;
 
   // ui::SimpleMenuModel::Delegate:
   bool IsCommandIdChecked(int command_id) const override;
@@ -92,7 +99,8 @@ class CastDialogView : public views::BubbleDialogDelegateView,
  private:
   CastDialogView(views::View* anchor_view,
                  CastDialogController* controller,
-                 Browser* browser);
+                 Browser* browser,
+                 const base::Time& start_time);
   ~CastDialogView() override;
 
   // views::BubbleDialogDelegateView:
@@ -120,6 +128,12 @@ class CastDialogView : public views::BubbleDialogDelegateView,
 
   void MaybeSizeToContents();
 
+  // Posts a delayed task to record an UMA metric for the number of sinks shown.
+  void RecordSinkCountWithDelay();
+
+  // Records an UMA metric for the number of sinks shown.
+  void RecordSinkCount();
+
   // The singleton dialog instance. This is a nullptr when a dialog is not
   // shown.
   static CastDialogView* instance_;
@@ -159,6 +173,11 @@ class CastDialogView : public views::BubbleDialogDelegateView,
   std::unique_ptr<ui::SimpleMenuModel> sources_menu_model_;
   std::unique_ptr<views::MenuRunner> sources_menu_runner_;
 
+  // The time when the dialog UI started initializing.
+  base::Time start_time_;
+
+  base::WeakPtrFactory<CastDialogView> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(CastDialogView);
 };
 

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

@@ -11,6 +11,7 @@
 
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/media_router/cast_dialog_controller.h"
 #include "chrome/browser/ui/media_router/cast_dialog_model.h"
@@ -101,7 +102,7 @@ class CastDialogViewTest : public ChromeViewsTestBase {
               dialog_ = static_cast<CastDialogView*>(observer);
             })));
     CastDialogView::ShowDialog(anchor_widget_->GetContentsView(), &controller_,
-                               nullptr);
+                               nullptr, base::Time::Now());
 
     dialog_->OnModelUpdated(model);
   }
@@ -148,7 +149,7 @@ TEST_F(CastDialogViewTest, MAYBE_ShowAndHideDialog) {
 
   EXPECT_CALL(controller_, AddObserver(_));
   CastDialogView::ShowDialog(anchor_widget_->GetContentsView(), &controller_,
-                             nullptr);
+                             nullptr, base::Time::Now());
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(CastDialogView::IsShowing());
   EXPECT_NE(nullptr, CastDialogView::GetCurrentDialogWidget());

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

@@ -75,6 +75,7 @@ MediaRouterDialogControllerViews::GetOrCreateForWebContents(
 }
 
 void MediaRouterDialogControllerViews::CreateMediaRouterDialog() {
+  base::Time dialog_creation_time = base::Time::Now();
   MediaRouterDialogControllerImplBase::CreateMediaRouterDialog();
 
   ui_ = std::make_unique<MediaRouterViewsUI>();
@@ -93,7 +94,8 @@ void MediaRouterDialogControllerViews::CreateMediaRouterDialog() {
   views::View* action_view = browser_actions->GetViewForId(
       ComponentToolbarActionsFactory::kMediaRouterActionId);
   CastDialogView::ShowDialog(action_view, ui_.get(),
-                             chrome::FindBrowserWithWebContents(initiator()));
+                             chrome::FindBrowserWithWebContents(initiator()),
+                             dialog_creation_time);
   CastDialogView::GetCurrentDialogWidget()->AddObserver(this);
 }
 

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

@@ -7,8 +7,10 @@
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/media/router/media_router_metrics.h"
 #include "chrome/browser/ui/media_router/media_sink_with_cast_modes.h"
 #include "chrome/browser/ui/media_router/ui_media_sink.h"
+#include "chrome/common/media_router/route_request_result.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 

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

@@ -737,8 +737,7 @@ void MediaRouterWebUIMessageHandler::OnReportClickedSinkIndex(
     DVLOG(1) << "Unable to extract args.";
     return;
   }
-  base::UmaHistogramSparse("MediaRouter.Ui.Action.StartLocalPosition",
-                           std::min(index, 100));
+  MediaRouterMetrics::RecordStartRouteDeviceIndex(index);
 }
 
 void MediaRouterWebUIMessageHandler::OnReportFilter(const base::ListValue*) {
@@ -800,9 +799,8 @@ void MediaRouterWebUIMessageHandler::OnReportRouteCreation(
     DVLOG(1) << "Unable to extract args.";
     return;
   }
-
-  UMA_HISTOGRAM_BOOLEAN("MediaRouter.Ui.Action.StartLocalSessionSuccessful",
-                        route_created_successfully);
+  MediaRouterMetrics::RecordStartLocalSessionSuccessful(
+      route_created_successfully);
 }
 
 void MediaRouterWebUIMessageHandler::OnReportRouteCreationOutcome(
@@ -841,7 +839,7 @@ void MediaRouterWebUIMessageHandler::OnReportSinkCount(
     DVLOG(1) << "Unable to extract args.";
     return;
   }
-  UMA_HISTOGRAM_COUNTS_100("MediaRouter.Ui.Device.Count", sink_count);
+  MediaRouterMetrics::RecordDeviceCount(sink_count);
 }
 
 void MediaRouterWebUIMessageHandler::OnReportTimeToClickSink(