Ensure PreferredActivityFound metric is logging correct platform.

This CL moves the GetPickerAction and GetDestinationPlatform function
calls out of RecordUma such that ChromeOSAppsNavigationThrottle can
call its own GetDestinationPlatform which sets the platform for when
a preferred activity is found.

BUG=968411

Change-Id: Ia838828ec4d03f1543243827f87601bfa2534c4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836293
Commit-Queue: Melissa Zhang <melzhang@chromium.org>
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702730}

chrome/browser/apps/intent_helper/apps_navigation_throttle.cc

@@ -155,30 +155,11 @@ void AppsNavigationThrottle::OnIntentPickerClosed(
     case PickerEntryType::kDevice:
       NOTREACHED();
   }
-  RecordUma(launch_name, entry_type, close_reason, Source::kHttpOrHttps,
-            should_persist);
-}
-
-// static
-void AppsNavigationThrottle::RecordUma(const std::string& selected_app_package,
-                                       PickerEntryType entry_type,
-                                       IntentPickerCloseReason close_reason,
-                                       Source source,
-                                       bool should_persist) {
   PickerAction action =
       GetPickerAction(entry_type, close_reason, should_persist);
-  Platform platform = GetDestinationPlatform(selected_app_package, action);
-
-  // TODO(crbug.com/985233) For now External Protocol Dialog is only querying
-  // ARC apps.
-  if (source == Source::kExternalProtocol) {
-    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog", action);
-  } else {
-    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.IntentPickerAction", action);
-
-    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.IntentPickerDestinationPlatform",
-                              platform);
-  }
+  Platform platform = GetDestinationPlatform(launch_name, action);
+  RecordUma(launch_name, entry_type, close_reason, Source::kHttpOrHttps,
+            should_persist, action, platform);
 }
 
 // static
@@ -277,6 +258,26 @@ bool AppsNavigationThrottle::CanCreate(content::WebContents* web_contents) {
   return true;
 }
 
+// static
+void AppsNavigationThrottle::RecordUma(const std::string& selected_app_package,
+                                       PickerEntryType entry_type,
+                                       IntentPickerCloseReason close_reason,
+                                       Source source,
+                                       bool should_persist,
+                                       PickerAction action,
+                                       Platform platform) {
+  // TODO(crbug.com/985233) For now External Protocol Dialog is only querying
+  // ARC apps.
+  if (source == Source::kExternalProtocol) {
+    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.ExternalProtocolDialog", action);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.IntentPickerAction", action);
+
+    UMA_HISTOGRAM_ENUMERATION("ChromeOS.Apps.IntentPickerDestinationPlatform",
+                              platform);
+  }
+}
+
 // static
 AppsNavigationThrottle::Platform AppsNavigationThrottle::GetDestinationPlatform(
     const std::string& selected_launch_name,
@@ -305,6 +306,42 @@ AppsNavigationThrottle::Platform AppsNavigationThrottle::GetDestinationPlatform(
   return Platform::ARC;
 }
 
+// static
+AppsNavigationThrottle::PickerAction AppsNavigationThrottle::GetPickerAction(
+    PickerEntryType entry_type,
+    IntentPickerCloseReason close_reason,
+    bool should_persist) {
+  switch (close_reason) {
+    case IntentPickerCloseReason::ERROR_BEFORE_PICKER:
+      return PickerAction::ERROR_BEFORE_PICKER;
+    case IntentPickerCloseReason::ERROR_AFTER_PICKER:
+      return PickerAction::ERROR_AFTER_PICKER;
+    case IntentPickerCloseReason::DIALOG_DEACTIVATED:
+      return PickerAction::DIALOG_DEACTIVATED;
+    case IntentPickerCloseReason::PREFERRED_APP_FOUND:
+      return PickerAction::PREFERRED_ACTIVITY_FOUND;
+    case IntentPickerCloseReason::STAY_IN_CHROME:
+      return should_persist ? PickerAction::CHROME_PREFERRED_PRESSED
+                            : PickerAction::CHROME_PRESSED;
+    case IntentPickerCloseReason::OPEN_APP:
+      switch (entry_type) {
+        case PickerEntryType::kUnknown:
+          NOTREACHED();
+          return PickerAction::INVALID;
+        case PickerEntryType::kArc:
+          return should_persist ? PickerAction::ARC_APP_PREFERRED_PRESSED
+                                : PickerAction::ARC_APP_PRESSED;
+        case PickerEntryType::kWeb:
+          return PickerAction::PWA_APP_PRESSED;
+        case PickerEntryType::kDevice:
+          return PickerAction::DEVICE_PRESSED;
+      }
+  }
+
+  NOTREACHED();
+  return PickerAction::INVALID;
+}
+
 // static
 std::vector<IntentPickerAppInfo> AppsNavigationThrottle::FindPwaForUrl(
     content::WebContents* web_contents,
@@ -421,42 +458,6 @@ bool AppsNavigationThrottle::navigate_from_link() {
   return navigate_from_link_;
 }
 
-// static
-AppsNavigationThrottle::PickerAction AppsNavigationThrottle::GetPickerAction(
-    PickerEntryType entry_type,
-    IntentPickerCloseReason close_reason,
-    bool should_persist) {
-  switch (close_reason) {
-    case IntentPickerCloseReason::ERROR_BEFORE_PICKER:
-      return PickerAction::ERROR_BEFORE_PICKER;
-    case IntentPickerCloseReason::ERROR_AFTER_PICKER:
-      return PickerAction::ERROR_AFTER_PICKER;
-    case IntentPickerCloseReason::DIALOG_DEACTIVATED:
-      return PickerAction::DIALOG_DEACTIVATED;
-    case IntentPickerCloseReason::PREFERRED_APP_FOUND:
-      return PickerAction::PREFERRED_ACTIVITY_FOUND;
-    case IntentPickerCloseReason::STAY_IN_CHROME:
-      return should_persist ? PickerAction::CHROME_PREFERRED_PRESSED
-                            : PickerAction::CHROME_PRESSED;
-    case IntentPickerCloseReason::OPEN_APP:
-      switch (entry_type) {
-        case PickerEntryType::kUnknown:
-          NOTREACHED();
-          return PickerAction::INVALID;
-        case PickerEntryType::kArc:
-          return should_persist ? PickerAction::ARC_APP_PREFERRED_PRESSED
-                                : PickerAction::ARC_APP_PRESSED;
-        case PickerEntryType::kWeb:
-          return PickerAction::PWA_APP_PRESSED;
-        case PickerEntryType::kDevice:
-          return PickerAction::DEVICE_PRESSED;
-      }
-  }
-
-  NOTREACHED();
-  return PickerAction::INVALID;
-}
-
 content::NavigationThrottle::ThrottleCheckResult
 AppsNavigationThrottle::HandleRequest() {
   content::NavigationHandle* handle = navigation_handle();

chrome/browser/apps/intent_helper/apps_navigation_throttle.h

@@ -69,12 +69,6 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
       IntentPickerCloseReason close_reason,
       bool should_persist);
 
-  static void RecordUma(const std::string& selected_app_package,
-                        PickerEntryType entry_type,
-                        IntentPickerCloseReason close_reason,
-                        Source source,
-                        bool should_persist);
-
   static bool IsGoogleRedirectorUrlForTesting(const GURL& url);
 
   static bool ShouldOverrideUrlLoadingForTesting(const GURL& previous_url,
@@ -156,6 +150,14 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
   // Checks whether we can create the apps_navigation_throttle.
   static bool CanCreate(content::WebContents* web_contents);
 
+  static void RecordUma(const std::string& selected_app_package,
+                        PickerEntryType entry_type,
+                        IntentPickerCloseReason close_reason,
+                        Source source,
+                        bool should_persist,
+                        PickerAction action,
+                        Platform platform);
+
   // Determines the destination of the current navigation. We know that if the
   // |picker_action| is either ERROR or DIALOG_DEACTIVATED the navigation MUST
   // stay in Chrome, and when |picker_action| is PWA_APP_PRESSED the navigation
@@ -165,6 +167,12 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
       const std::string& selected_launch_name,
       PickerAction picker_action);
 
+  // Converts the provided |entry_type|, |close_reason| and |should_persist|
+  // boolean to a PickerAction value for recording in UMA.
+  static PickerAction GetPickerAction(PickerEntryType entry_type,
+                                      IntentPickerCloseReason close_reason,
+                                      bool should_persist);
+
   // If an installed PWA exists that can handle |url|, prepends it to |apps| and
   // returns the new list.
   static std::vector<IntentPickerAppInfo> FindPwaForUrl(
@@ -224,12 +232,6 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
   FRIEND_TEST_ALL_PREFIXES(chromeos::ChromeOsAppsNavigationThrottleTest,
                            TestGetDestinationPlatform);
 
-  // Converts the provided |entry_type|, |close_reason| and |should_persist|
-  // boolean to a PickerAction value for recording in UMA.
-  static PickerAction GetPickerAction(PickerEntryType entry_type,
-                                      IntentPickerCloseReason close_reason,
-                                      bool should_persist);
-
   content::NavigationThrottle::ThrottleCheckResult HandleRequest();
 
   // A reference to the starting GURL.

chrome/browser/chromeos/apps/intent_helper/chromeos_apps_navigation_throttle.cc

@@ -113,8 +113,12 @@ void ChromeOsAppsNavigationThrottle::RecordUma(
     UMA_HISTOGRAM_ENUMERATION("Arc.UserInteraction",
                               arc::UserInteractionType::APP_STARTED_FROM_LINK);
   }
+  PickerAction action = apps::AppsNavigationThrottle::GetPickerAction(
+      entry_type, close_reason, should_persist);
+  Platform platform = GetDestinationPlatform(selected_app_package, action);
   apps::AppsNavigationThrottle::RecordUma(selected_app_package, entry_type,
-                                          close_reason, source, should_persist);
+                                          close_reason, source, should_persist,
+                                          action, platform);
 }
 
 ChromeOsAppsNavigationThrottle::ChromeOsAppsNavigationThrottle(