Reland "Moving external protocol dialog metrics to intent

handling metrics."

The external protocol dialog class recorded metrics that fall under
the purview of the intent handling metrics class, so it has been
moved to be under that class structure. The original change caused
compile failures and was reverted, this one should be fixed.

BUG=1046611

TBR=djacobo@chromium.org
TBR reviewers:
djacobo: please review the changes to
         chrome/browser/chromeos/arc/intent_helper/
         arc_external_protocol_dialog.*

Change-Id: If47a6306f7baddb834585113736d4b8b2485f3d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2027007
Commit-Queue: Alex Linker <ajlinker@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736609}

chrome/browser/apps/intent_helper/apps_navigation_throttle.cc

@@ -12,7 +12,6 @@
 #include "base/optional.h"
 #include "chrome/browser/apps/intent_helper/intent_picker_auto_display_service.h"
 #include "chrome/browser/apps/intent_helper/page_transition_util.h"
-#include "chrome/browser/chromeos/apps/metrics/intent_handling_metrics.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/menu_manager.h"
 #include "chrome/browser/prerender/prerender_contents.h"

chrome/browser/chromeos/apps/metrics/intent_handling_metrics.cc

@@ -4,8 +4,6 @@
 
 #include "chrome/browser/chromeos/apps/metrics/intent_handling_metrics.h"
 
-#include <string>
-
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
 #include "components/arc/metrics/arc_metrics_constants.h"
@@ -52,4 +50,20 @@ void IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
   RecordIntentPickerMetrics(source, should_persist, action, platform);
 }
 
+void IntentHandlingMetrics::RecordExternalProtocolMetrics(
+    arc::Scheme scheme,
+    PickerEntryType entry_type,
+    bool accepted,
+    bool persisted) {
+  arc::ProtocolAction action =
+      arc::GetProtocolAction(scheme, entry_type, accepted, persisted);
+  if (accepted) {
+    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog.Accepted",
+                              action);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog.Rejected",
+                              action);
+  }
+}
+
 }  // namespace apps

chrome/browser/chromeos/apps/metrics/intent_handling_metrics.h

@@ -10,6 +10,7 @@
 
 #include "chrome/browser/apps/intent_helper/apps_navigation_throttle.h"
 #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
+#include "chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h"
 #include "components/arc/metrics/arc_metrics_constants.h"
 
 namespace apps {
@@ -29,6 +30,11 @@ class IntentHandlingMetrics {
       IntentPickerCloseReason close_reason,
       Source source,
       bool should_persist);
+
+  static void RecordExternalProtocolMetrics(arc::Scheme scheme,
+                                            apps::PickerEntryType entry_type,
+                                            bool accepted,
+                                            bool persisted);
 };
 
 }  // namespace apps

chrome/browser/chromeos/apps/metrics/intent_handling_metrics_unittest.cc

@@ -8,6 +8,7 @@
 #include "base/test/gtest_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/chromeos/apps/intent_helper/chromeos_apps_navigation_throttle.h"
+#include "chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace apps {
@@ -58,4 +59,16 @@ TEST_F(IntentHandlingMetricsTest,
       AppsNavigationThrottle::PickerAction::ARC_APP_PREFERRED_PRESSED, 1);
 }
 
+TEST_F(IntentHandlingMetricsTest, TestRecordExternalProtocolMetrics) {
+  base::HistogramTester histogram_tester;
+
+  IntentHandlingMetrics test = IntentHandlingMetrics();
+  test.RecordExternalProtocolMetrics(arc::Scheme::IRC, PickerEntryType::kArc,
+                                     /*accepted=*/true, /*persisted=*/true);
+
+  histogram_tester.ExpectBucketCount(
+      "ChromeOS.Apps.ExternalProtocolDialog.Accepted",
+      arc::ProtocolAction::IRC_ACCEPTED_PERSISTED, 1);
+}
+
 }  // namespace apps

chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc

@@ -493,8 +493,8 @@ void OnIntentPickerClosed(
     DCHECK_EQ(apps::IntentPickerCloseReason::OPEN_APP, reason);
     DCHECK(!should_persist);
     HandleDeviceSelection(web_contents, devices, selected_app_package, url);
-    RecordUmaDialogAction(Scheme::TEL, entry_type, /*accepted=*/true,
-                          should_persist);
+    apps::IntentHandlingMetrics::RecordExternalProtocolMetrics(
+        Scheme::TEL, entry_type, /*accepted=*/true, should_persist);
     apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
         selected_app_package, entry_type, reason,
         apps::Source::kExternalProtocol, should_persist);
@@ -588,8 +588,8 @@ void OnIntentPickerClosed(
   auto scheme_it = string_to_scheme.find(scheme);
   if (scheme_it != string_to_scheme.end())
     url_scheme = scheme_it->second;
-  RecordUmaDialogAction(url_scheme, entry_type, protocol_accepted,
-                        should_persist);
+  apps::IntentHandlingMetrics::RecordExternalProtocolMetrics(
+      url_scheme, entry_type, protocol_accepted, should_persist);
 
   apps::IntentHandlingMetrics::RecordIntentPickerUserInteractionMetrics(
       selected_app_package, entry_type, reason, apps::Source::kExternalProtocol,
@@ -824,22 +824,6 @@ void OnIntentPickerClosedForTesting(
                        reason, should_persist);
 }
 
-// TODO(crbug.com/1044710): convert this to new scheme.
-void RecordUmaDialogAction(Scheme scheme,
-                           apps::PickerEntryType entry_type,
-                           bool accepted,
-                           bool persisted) {
-  ProtocolAction action =
-      GetProtocolAction(scheme, entry_type, accepted, persisted);
-  if (accepted) {
-    base::UmaHistogramEnumeration(
-        "ChromeOS.Apps.ExternalProtocolDialog.Accepted", action);
-  } else {
-    base::UmaHistogramEnumeration(
-        "ChromeOS.Apps.ExternalProtocolDialog.Rejected", action);
-  }
-}
-
 ProtocolAction GetProtocolAction(Scheme scheme,
                                  apps::PickerEntryType entry_type,
                                  bool accepted,

chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.h

@@ -149,11 +149,6 @@ bool GetAndResetSafeToRedirectToArcWithoutUserConfirmationFlagForTesting(
 bool IsChromeAnAppCandidateForTesting(
     const std::vector<mojom::IntentHandlerInfoPtr>& handlers);
 
-void RecordUmaDialogAction(Scheme scheme,
-                           apps::PickerEntryType entry_type,
-                           bool accepted,
-                           bool persisted);
-
 ProtocolAction GetProtocolAction(Scheme scheme,
                                  apps::PickerEntryType entry_type,
                                  bool accepted,