[DIAL MRP] Add a few key metrics.

MediaRouter.Dial.CreateRoute - records result of a CreateRoute call.
MediaRouter.Dial.TerminateRoute - similarly for a TerminateRoute call.
MediaRouter.Dial.ParseMessage - records result of parsing a message
coming from the Cast SDK.
MediaRouter.Dial.FetchAppInfo - records reuslt of a app info request.
Note we already have metrics for device description.

Bug: 808720,798861
Change-Id: Ic48eba338298e7086fcab290f24ab490e5944ab3
Reviewed-on: https://chromium-review.googlesource.com/1153648Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580575}

chrome/browser/media/router/BUILD.gn

@@ -114,6 +114,8 @@ static_library("router") {
       "providers/dial/dial_internal_message_util.h",
       "providers/dial/dial_media_route_provider.cc",
       "providers/dial/dial_media_route_provider.h",
+      "providers/dial/dial_media_route_provider_metrics.cc",
+      "providers/dial/dial_media_route_provider_metrics.h",
       "providers/extension/extension_media_route_provider_proxy.cc",
       "providers/extension/extension_media_route_provider_proxy.h",
       "providers/wired_display/wired_display_media_route_provider.cc",

chrome/browser/media/router/discovery/dial/dial_app_discovery_service.cc

@@ -8,6 +8,7 @@
 #include "base/strings/string_util.h"
 #include "base/time/default_clock.h"
 #include "chrome/browser/media/router/data_decoder_util.h"
+#include "chrome/browser/media/router/media_router_metrics.h"
 #include "net/http/http_status_code.h"
 #include "url/gurl.h"
 
@@ -117,10 +118,14 @@ void DialAppDiscoveryService::PendingRequest::OnDialAppInfoFetchError(
   if (response_code == net::HTTP_NOT_FOUND ||
       response_code >= net::HTTP_INTERNAL_SERVER_ERROR ||
       response_code == net::HTTP_OK) {
+    MediaRouterMetrics::RecordDialFetchAppInfo(
+        DialAppInfoResultCode::kNotFound);
     std::move(app_info_cb_)
         .Run(sink_id_, app_name_,
              DialAppInfoResult(nullptr, DialAppInfoResultCode::kNotFound));
   } else {
+    MediaRouterMetrics::RecordDialFetchAppInfo(
+        DialAppInfoResultCode::kNetworkError);
     std::move(app_info_cb_)
         .Run(sink_id_, app_name_,
              DialAppInfoResult(nullptr, DialAppInfoResultCode::kNetworkError));
@@ -135,10 +140,13 @@ void DialAppDiscoveryService::PendingRequest::OnDialAppInfoParsed(
   if (!parsed_app_info) {
     DVLOG(2) << "Failed to parse app info XML in utility process, error: "
              << parsing_result;
+    MediaRouterMetrics::RecordDialFetchAppInfo(
+        DialAppInfoResultCode::kParsingError);
     std::move(app_info_cb_)
         .Run(sink_id_, app_name_,
              DialAppInfoResult(nullptr, DialAppInfoResultCode::kParsingError));
   } else {
+    MediaRouterMetrics::RecordDialFetchAppInfo(DialAppInfoResultCode::kOk);
     std::move(app_info_cb_)
         .Run(sink_id_, app_name_,
              DialAppInfoResult(std::move(parsed_app_info),

chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h

@@ -25,11 +25,14 @@ namespace media_router {
 class DataDecoder;
 
 // Represents DIAL app status on receiver device.
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
 enum class DialAppInfoResultCode {
   kOk = 0,
-  kNotFound,
-  kNetworkError,
-  kParsingError
+  kNotFound = 1,
+  kNetworkError = 2,
+  kParsingError = 3,
+  kCount
 };
 
 struct DialAppInfoResult {

chrome/browser/media/router/media_router_metrics.cc

@@ -49,6 +49,8 @@ const char MediaRouterMetrics::kHistogramCloseLatency[] =
     "MediaRouter.Ui.Action.CloseLatency";
 const char MediaRouterMetrics::kHistogramDialParsingError[] =
     "MediaRouter.Dial.ParsingError";
+const char MediaRouterMetrics::kHistogramDialFetchAppInfo[] =
+    "MediaRouter.Dial.FetchAppInfo";
 const char MediaRouterMetrics::kHistogramIconClickLocation[] =
     "MediaRouter.Icon.Click.Location";
 const char MediaRouterMetrics::kHistogramMediaRouterCastingSource[] =
@@ -155,6 +157,13 @@ void MediaRouterMetrics::RecordDialParsingError(
       SafeDialDeviceDescriptionParser::ParsingError::kTotalCount);
 }
 
+// static
+void MediaRouterMetrics::RecordDialFetchAppInfo(
+    DialAppInfoResultCode result_code) {
+  UMA_HISTOGRAM_ENUMERATION(kHistogramDialFetchAppInfo, result_code,
+                            DialAppInfoResultCode::kCount);
+}
+
 // static
 void MediaRouterMetrics::RecordPresentationUrlType(const GURL& url) {
   PresentationUrlType type = GetPresentationUrlType(url);

chrome/browser/media/router/media_router_metrics.h

@@ -9,6 +9,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/time/time.h"
+#include "chrome/browser/media/router/discovery/dial/dial_app_discovery_service.h"
 #include "chrome/browser/media/router/discovery/dial/safe_dial_device_description_parser.h"
 #include "chrome/browser/ui/media_router/media_cast_mode.h"
 #include "media/base/container_names.h"
@@ -83,6 +84,7 @@ class MediaRouterMetrics {
   // UMA histogram names.
   static const char kHistogramCloseLatency[];
   static const char kHistogramDialParsingError[];
+  static const char kHistogramDialFetchAppInfo[];
   static const char kHistogramIconClickLocation[];
   static const char kHistogramMediaRouterCastingSource[];
   static const char kHistogramMediaRouterFileFormat[];
@@ -139,6 +141,9 @@ class MediaRouterMetrics {
   static void RecordDialParsingError(
       SafeDialDeviceDescriptionParser::ParsingError parsing_error);
 
+  // Records the result of a DIAL app info request.
+  static void RecordDialFetchAppInfo(DialAppInfoResultCode result_code);
+
   // Records the type of Presentation URL used by a web page.
   static void RecordPresentationUrlType(const GURL& url);
 

chrome/browser/media/router/providers/dial/dial_media_route_provider.cc

@@ -10,6 +10,7 @@
 #include "base/no_destructor.h"
 #include "base/stl_util.h"
 #include "chrome/browser/media/router/data_decoder_util.h"
+#include "chrome/browser/media/router/providers/dial/dial_media_route_provider_metrics.h"
 #include "chrome/common/media_router/media_source_helper.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "url/origin.h"
@@ -22,9 +23,11 @@ url::Origin CreateOrigin(const std::string& url) {
   return url::Origin::Create(GURL(url));
 }
 
-void ReportParseError(const std::string& error) {
-  // TODO(crbug.com/808720): Record UMA metric for parse result.
-  DVLOG(2) << "Failed to parse DIAL internal message: " << error;
+void ReportParseError(DialParseMessageResult result,
+                      const std::string& error_message) {
+  DCHECK_NE(result, DialParseMessageResult::kSuccess);
+  DVLOG(2) << "Failed to parse DIAL internal message: " << error_message;
+  DialMediaRouteProviderMetrics::RecordParseMessageResult(result);
 }
 
 static constexpr int kMaxPendingDialLaunches = 10;
@@ -93,6 +96,8 @@ void DialMediaRouteProvider::CreateRoute(const std::string& media_source,
   if (!sink) {
     std::move(callback).Run(base::nullopt, "Unknown sink " + sink_id,
                             RouteRequestResult::SINK_NOT_FOUND);
+    DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+        DialCreateRouteResult::kSinkNotFound);
     return;
   }
 
@@ -101,6 +106,8 @@ void DialMediaRouteProvider::CreateRoute(const std::string& media_source,
   if (!activity) {
     std::move(callback).Run(base::nullopt, "Unsupported source " + media_source,
                             RouteRequestResult::NO_SUPPORTED_PROVIDER);
+    DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+        DialCreateRouteResult::kUnsupportedSource);
     return;
   }
 
@@ -109,6 +116,8 @@ void DialMediaRouteProvider::CreateRoute(const std::string& media_source,
       activity_manager_->GetActivityBySinkId(sink_id)) {
     std::move(callback).Run(base::nullopt, "Activity already exists",
                             RouteRequestResult::ROUTE_ALREADY_EXISTS);
+    DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+        DialCreateRouteResult::kRouteAlreadyExists);
     return;
   }
 
@@ -167,6 +176,8 @@ void DialMediaRouteProvider::TerminateRoute(const std::string& route_id,
     DVLOG(2) << "No activity record found with route_id " << route_id;
     std::move(callback).Run("Activity not found",
                             RouteRequestResult::ROUTE_NOT_FOUND);
+    DialMediaRouteProviderMetrics::RecordTerminateRouteResult(
+        DialTerminateRouteResult::kRouteNotFound);
     return;
   }
 
@@ -177,6 +188,8 @@ void DialMediaRouteProvider::TerminateRoute(const std::string& route_id,
     DVLOG(2) << __func__ << ": Sink not found: " << route.media_sink_id();
     std::move(callback).Run("Sink not found",
                             RouteRequestResult::SINK_NOT_FOUND);
+    DialMediaRouteProviderMetrics::RecordTerminateRouteResult(
+        DialTerminateRouteResult::kSinkNotFound);
     return;
   }
 
@@ -192,7 +205,8 @@ void DialMediaRouteProvider::SendRouteMessage(
       message,
       base::BindRepeating(&DialMediaRouteProvider::HandleParsedRouteMessage,
                           weak_ptr_factory_.GetWeakPtr(), media_route_id),
-      base::BindRepeating(&ReportParseError));
+      base::BindRepeating(&ReportParseError,
+                          DialParseMessageResult::kParseError));
   // TODO(https://crbug.com/866551): SendRouteMessageCallback is no longer used.
   // Always invoke it with true until it is removed.
   std::move(callback).Run(true);
@@ -205,10 +219,13 @@ void DialMediaRouteProvider::HandleParsedRouteMessage(
   std::unique_ptr<DialInternalMessage> internal_message =
       DialInternalMessage::From(std::move(*message), &error);
   if (!internal_message) {
-    ReportParseError(error);
+    ReportParseError(DialParseMessageResult::kInvalidMessage, error);
     return;
   }
 
+  DialMediaRouteProviderMetrics::RecordParseMessageResult(
+      DialParseMessageResult::kSuccess);
+
   const DialActivity* activity = activity_manager_->GetActivity(route_id);
   if (!activity) {
     DVLOG(2) << "No activity record found with route_id " << route_id;
@@ -261,6 +278,8 @@ void DialMediaRouteProvider::SendCustomDialLaunchMessage(
     // Note: this leaves the route in a stuck state; the client must terminate
     // the route. Maybe we should clean up the route here.
     DVLOG(2) << __func__ << ": unable to get app info for " << route_id;
+    DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+        DialCreateRouteResult::kAppInfoNotFound);
     return;
   }
 
@@ -315,6 +334,9 @@ void DialMediaRouteProvider::HandleAppLaunchResult(
     const MediaRoute::Id& route_id,
     bool success) {
   DVLOG(2) << "Launch result for: " << route_id << ": " << success;
+  DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+      success ? DialCreateRouteResult::kSuccess
+              : DialCreateRouteResult::kAppLaunchFailed);
   NotifyAllOnRoutesUpdated();
 }
 
@@ -344,6 +366,11 @@ void DialMediaRouteProvider::HandleStopAppResult(
     media_router_->OnPresentationConnectionStateChanged(
         route_id, mojom::MediaRouter::PresentationConnectionState::TERMINATED);
     NotifyAllOnRoutesUpdated();
+    DialMediaRouteProviderMetrics::RecordTerminateRouteResult(
+        DialTerminateRouteResult::kSuccess);
+  } else {
+    DialMediaRouteProviderMetrics::RecordTerminateRouteResult(
+        DialTerminateRouteResult::kStopAppFailed);
   }
   std::move(callback).Run(message, result_code);
 }

chrome/browser/media/router/providers/dial/dial_media_route_provider_metrics.cc

+// Copyright 2018 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/media/router/providers/dial/dial_media_route_provider_metrics.h"
+
+#include "base/metrics/histogram_macros.h"
+
+namespace media_router {
+
+// static
+void DialMediaRouteProviderMetrics::RecordCreateRouteResult(
+    DialCreateRouteResult result) {
+  UMA_HISTOGRAM_ENUMERATION(kHistogramDialCreateRouteResult, result,
+                            DialCreateRouteResult::kCount);
+}
+
+// static
+void DialMediaRouteProviderMetrics::RecordTerminateRouteResult(
+    DialTerminateRouteResult result) {
+  UMA_HISTOGRAM_ENUMERATION(kHistogramDialTerminateRouteResult, result,
+                            DialTerminateRouteResult::kCount);
+}
+
+// static
+void DialMediaRouteProviderMetrics::RecordParseMessageResult(
+    DialParseMessageResult result) {
+  UMA_HISTOGRAM_ENUMERATION(kHistogramDialParseMessageResult, result,
+                            DialParseMessageResult::kCount);
+}
+
+}  // namespace media_router

chrome/browser/media/router/providers/dial/dial_media_route_provider_metrics.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_MEDIA_ROUTER_PROVIDERS_DIAL_DIAL_MEDIA_ROUTE_PROVIDER_METRICS_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_PROVIDERS_DIAL_DIAL_MEDIA_ROUTE_PROVIDER_METRICS_H_
+
+namespace media_router {
+
+static constexpr char kHistogramDialCreateRouteResult[] =
+    "MediaRouter.Dial.CreateRoute";
+static constexpr char kHistogramDialTerminateRouteResult[] =
+    "MediaRouter.Dial.TerminateRoute";
+static constexpr char kHistogramDialParseMessageResult[] =
+    "MediaRouter.Dial.ParseMessage";
+
+// Note on enums defined in this file:
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+enum class DialCreateRouteResult {
+  kSuccess = 0,
+  kSinkNotFound = 1,
+  kAppInfoNotFound = 2,
+  kAppLaunchFailed = 3,
+  kUnsupportedSource = 4,
+  kRouteAlreadyExists = 5,
+  kCount
+};
+
+enum class DialTerminateRouteResult {
+  kSuccess = 0,
+  kRouteNotFound = 1,
+  kSinkNotFound = 2,
+  kStopAppFailed = 3,
+  kCount
+};
+
+enum class DialParseMessageResult {
+  kSuccess = 0,
+  kParseError = 1,
+  kInvalidMessage = 2,
+  kCount
+};
+
+class DialMediaRouteProviderMetrics {
+ public:
+  static void RecordCreateRouteResult(DialCreateRouteResult result);
+  static void RecordTerminateRouteResult(DialTerminateRouteResult result);
+  static void RecordParseMessageResult(DialParseMessageResult result);
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_PROVIDERS_DIAL_DIAL_MEDIA_ROUTE_PROVIDER_METRICS_H_

tools/metrics/histograms/enums.xml

@@ -31138,6 +31138,22 @@ Called by update_use_counter_css.py.-->
   <int value="2" label="Failure Invalid Sink"/>
 </enum>
 
+<enum name="MediaRouterDialCreateRouteResult">
+  <int value="0" label="Success"/>
+  <int value="1" label="Sink not found"/>
+  <int value="2" label="App info not found"/>
+  <int value="3" label="App launch failed"/>
+  <int value="4" label="Unsupported source"/>
+  <int value="5" label="Route already exists"/>
+</enum>
+
+<enum name="MediaRouterDialFetchAppInfoResult">
+  <int value="0" label="Success"/>
+  <int value="1" label="Not found"/>
+  <int value="2" label="Network error"/>
+  <int value="3" label="Parse error"/>
+</enum>
+
 <enum name="MediaRouterDialogOpenOrigin">
   <int value="0" label="Toolbar"/>
   <int value="1" label="Overflow Menu"/>
@@ -31145,6 +31161,12 @@ Called by update_use_counter_css.py.-->
   <int value="3" label="Page"/>
 </enum>
 
+<enum name="MediaRouterDialParseMessageResult">
+  <int value="0" label="Success"/>
+  <int value="1" label="Parse error"/>
+  <int value="2" label="Invalid message"/>
+</enum>
+
 <enum name="MediaRouterDialParsingError">
   <int value="0" label="None"/>
   <int value="1" label="Invalid XML"/>
@@ -31159,6 +31181,13 @@ Called by update_use_counter_css.py.-->
   <int value="10" label="Utility process error"/>
 </enum>
 
+<enum name="MediaRouterDialTerminateRouteResult">
+  <int value="0" label="Success"/>
+  <int value="1" label="Route not found"/>
+  <int value="2" label="Sink not found"/>
+  <int value="3" label="Stop app failed"/>
+</enum>
+
 <enum name="MediaRouterInitialViews">
   <int value="0" label="Route Details"/>
   <int value="1" label="Sink List"/>

tools/metrics/histograms/histograms.xml

@@ -43497,6 +43497,24 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="MediaRouter.Dial.CreateRoute"
+    enum="MediaRouterDialCreateRouteResult">
+  <owner>imcheng@chromium.org</owner>
+  <summary>
+    The result of a DIAL CreateRoute request. Recorded when the user requests to
+    create a media route to a DIAL device.
+  </summary>
+</histogram>
+
+<histogram name="MediaRouter.Dial.FetchAppInfo"
+    enum="MediaRouterDialFetchAppInfoResult">
+  <owner>imcheng@chromium.org</owner>
+  <summary>
+    The result of a DIAL app info request. Recorded when an app info request is
+    issued to a DIAL device.
+  </summary>
+</histogram>
+
 <histogram name="MediaRouter.Dial.KnownDevicesCount" units="devices">
   <owner>mfoltz@chromium.org</owner>
   <summary>
@@ -43505,6 +43523,16 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="MediaRouter.Dial.ParseMessage"
+    enum="MediaRouterDialParseMessageResult">
+  <owner>imcheng@chromium.org</owner>
+  <summary>
+    The result of parsing a Cast SDK message in the DIAL media route provider.
+    Recorded when the DIAL media route prover finishes parsing a Cast SDK
+    message sent from a Cast-enabled webpage.
+  </summary>
+</histogram>
+
 <histogram name="MediaRouter.Dial.ParsingError"
     enum="MediaRouterDialParsingError">
   <owner>mfoltz@chromium.org</owner>
@@ -43515,6 +43543,15 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="MediaRouter.Dial.TerminateRoute"
+    enum="MediaRouterDialTerminateRouteResult">
+  <owner>imcheng@chromium.org</owner>
+  <summary>
+    The result of a DIAL TerminateRoute request. Recorded the user requests to
+    terminate a DIAL media route.
+  </summary>
+</histogram>
+
 <histogram name="MediaRouter.Icon.Click.Location"
     enum="MediaRouterDialogOpenOrigin">
   <owner>takumif@chromium.org</owner>