Adding a destination platform histogram for UMA.

This enum helps to keep track of whether a navigation observed by
ArcNavigationThrottle is continued in Chrome or redirected to a ARC.
Likewise it helps determinating if a navigation is continued in Chrome
or redirected to ARC via the external protocol dialog.

BUG=661672
TEST=try

Review-Url: https://codereview.chromium.org/2476783002
Cr-Commit-Position: refs/heads/master@{#430176}

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

@@ -10,7 +10,6 @@
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
-#include "base/metrics/histogram_macros.h"
 #include "chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.h"
 #include "chrome/browser/chromeos/external_protocol_dialog.h"
 #include "chrome/browser/tab_contents/tab_util.h"
@@ -40,12 +39,6 @@ constexpr uint32_t kMinVersionForHandleUrl = 2;
 constexpr uint32_t kMinVersionForRequestUrlHandlerList = 2;
 constexpr uint32_t kMinVersionForAddPreferredPackage = 7;
 
-void RecordUma(ArcNavigationThrottle::CloseReason close_reason) {
-  UMA_HISTOGRAM_ENUMERATION(
-      "Arc.IntentHandlerAction", static_cast<int>(close_reason),
-      static_cast<int>(ArcNavigationThrottle::CloseReason::SIZE));
-}
-
 // Shows the Chrome OS' original external protocol dialog as a fallback.
 void ShowFallbackExternalProtocolDialog(int render_process_host_id,
                                         int routing_id,
@@ -271,7 +264,11 @@ void OnIntentPickerClosed(int render_process_host_id,
       break;
     }
   }
-  RecordUma(close_reason);
+
+  ArcNavigationThrottle::Platform platform =
+      ArcNavigationThrottle::GetDestinationPlatform(selected_app_package,
+                                                    close_reason);
+  ArcNavigationThrottle::RecordUma(close_reason, platform);
 }
 
 // Called when ARC returned activity icons for the |handlers|.
@@ -324,8 +321,11 @@ void OnUrlHandlerList(int render_process_host_id,
   GetActionResult result;
   if (HandleUrl(render_process_host_id, routing_id, url, handlers,
                 handlers.size(), &result)) {
-    if (result == GetActionResult::HANDLE_URL_IN_ARC)
-      RecordUma(ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND);
+    if (result == GetActionResult::HANDLE_URL_IN_ARC) {
+      ArcNavigationThrottle::RecordUma(
+          ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND,
+          ArcNavigationThrottle::Platform::ARC);
+    }
     return;  // the |url| has been handled.
   }
 

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

@@ -370,9 +370,9 @@ void ArcNavigationThrottle::OnIntentPickerClosed(
     }
   }
 
-  UMA_HISTOGRAM_ENUMERATION("Arc.IntentHandlerAction",
-                            static_cast<int>(close_reason),
-                            static_cast<int>(CloseReason::SIZE));
+  Platform platform =
+      GetDestinationPlatform(selected_app_package, close_reason);
+  RecordUma(close_reason, platform);
 }
 
 // static
@@ -386,6 +386,29 @@ size_t ArcNavigationThrottle::GetAppIndex(
   return handlers.size();
 }
 
+// static
+ArcNavigationThrottle::Platform ArcNavigationThrottle::GetDestinationPlatform(
+    const std::string& selected_app_package,
+    CloseReason close_reason) {
+  return (close_reason != CloseReason::ERROR &&
+          close_reason != CloseReason::DIALOG_DEACTIVATED &&
+          !ArcIntentHelperBridge::IsIntentHelperPackage(selected_app_package))
+             ? Platform::ARC
+             : Platform::CHROME;
+}
+
+// static
+void ArcNavigationThrottle::RecordUma(CloseReason close_reason,
+                                      Platform platform) {
+  UMA_HISTOGRAM_ENUMERATION("Arc.IntentHandlerAction",
+                            static_cast<int>(close_reason),
+                            static_cast<int>(CloseReason::SIZE));
+
+  UMA_HISTOGRAM_ENUMERATION("Arc.IntentHandlerDestinationPlatform",
+                            static_cast<int>(platform),
+                            static_cast<int>(Platform::SIZE));
+}
+
 // static
 bool ArcNavigationThrottle::ShouldOverrideUrlLoadingForTesting(
     const GURL& previous_url,

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

@@ -43,6 +43,15 @@ class ArcNavigationThrottle : public content::NavigationThrottle {
     INVALID = SIZE,
   };
 
+  // As for CloseReason, these define the buckets for an UMA histogram, so this
+  // must be treated in an append-only fashion. This helps especify where a
+  // navigation will continue.
+  enum class Platform : int {
+    ARC = 0,
+    CHROME = 1,
+    SIZE,
+  };
+
   // Restricts the amount of apps displayed to the user without the need of a
   // ScrollView.
   enum { kMaxAppResults = 3 };
@@ -71,6 +80,16 @@ class ArcNavigationThrottle : public content::NavigationThrottle {
   static size_t GetAppIndex(
       const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers,
       const std::string& selected_app_package);
+  // Determines the destination of the current navigation. We know that if the
+  // |close_reason| is either ERROR or DIALOG_DEACTIVATED the navigation MUST
+  // stay in Chrome, otherwise we can assume the navigation goes to ARC with the
+  // exception of the |selected_app_package| being Chrome.
+  static Platform GetDestinationPlatform(
+      const std::string& selected_app_package,
+      CloseReason close_reason);
+  // Records intent picker usage statistics as well as whether navigations are
+  // continued or redirected to Chrome or ARC respectively, via UMA histograms.
+  static void RecordUma(CloseReason close_reason, Platform platform);
 
   static bool IsAppAvailableForTesting(
       const mojo::Array<mojom::IntentHandlerInfoPtr>& handlers);

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

@@ -143,6 +143,58 @@ TEST(ArcNavigationThrottleTest, TestGetAppIndex) {
       2u, ArcNavigationThrottle::GetAppIndex(CreateArray(3, 2), package_name));
 }
 
+TEST(ArcNavigationThrottleTest, TestGetDestinationPlatform) {
+  const std::string chrome_app =
+      ArcIntentHelperBridge::kArcIntentHelperPackageName;
+  const std::string non_chrome_app = "fake_package";
+
+  // When the CloseReason is either ERROR or DIALOG_DEACTIVATED we MUST stay in
+  // Chrome not taking into account the selected_app_package.
+  EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                chrome_app, ArcNavigationThrottle::CloseReason::ERROR));
+  EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                non_chrome_app, ArcNavigationThrottle::CloseReason::ERROR));
+  EXPECT_EQ(
+      ArcNavigationThrottle::Platform::CHROME,
+      ArcNavigationThrottle::GetDestinationPlatform(
+          chrome_app, ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED));
+  EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                non_chrome_app,
+                ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED));
+
+  // Under any other CloseReason, stay in Chrome only if the package is Chrome.
+  // Otherwise redirect to ARC.
+  EXPECT_EQ(
+      ArcNavigationThrottle::Platform::CHROME,
+      ArcNavigationThrottle::GetDestinationPlatform(
+          chrome_app, ArcNavigationThrottle::CloseReason::ALWAYS_PRESSED));
+  EXPECT_EQ(
+      ArcNavigationThrottle::Platform::CHROME,
+      ArcNavigationThrottle::GetDestinationPlatform(
+          chrome_app, ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED));
+  EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                chrome_app,
+                ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND));
+
+  // Go to ARC on any other case.
+  EXPECT_EQ(
+      ArcNavigationThrottle::Platform::ARC,
+      ArcNavigationThrottle::GetDestinationPlatform(
+          non_chrome_app, ArcNavigationThrottle::CloseReason::ALWAYS_PRESSED));
+  EXPECT_EQ(ArcNavigationThrottle::Platform::ARC,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                non_chrome_app,
+                ArcNavigationThrottle::CloseReason::JUST_ONCE_PRESSED));
+  EXPECT_EQ(ArcNavigationThrottle::Platform::ARC,
+            ArcNavigationThrottle::GetDestinationPlatform(
+                non_chrome_app,
+                ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND));
+}
+
 TEST(ArcNavigationThrottleTest, TestIsSwapElementsNeeded) {
   std::pair<size_t, size_t> indices;
   for (size_t i = 1; i <= ArcNavigationThrottle::kMaxAppResults; ++i) {

tools/metrics/histograms/histograms.xml

@@ -1218,7 +1218,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 <histogram name="Arc.IntentHandlerAction" enum="ArcIntentHandlerAction">
   <owner>elijahtaylor@google.com</owner>
   <owner>mitsuji@google.com</owner>
-  <summary>Arc intent handler action taken by user.</summary>
+  <summary>ARC intent handler action taken by user.</summary>
+</histogram>
+
+<histogram name="Arc.IntentHandlerDestinationPlatform"
+    enum="ArcIntentHandlerDestinationPlatform">
+  <owner>elijahtaylor@google.com</owner>
+  <owner>mitsuji@google.com</owner>
+  <summary>
+    ARC intent handler destination platform. The destination may be specified
+    due to the user explicit selection or a previously stored preference.
+  </summary>
 </histogram>
 
 <histogram name="Arc.LowMemoryKiller.Count">
@@ -73881,6 +73891,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <int value="4" label="Preferred activity found"/>
 </enum>
 
+<enum name="ArcIntentHandlerDestinationPlatform" type="int">
+  <summary>
+    Defines ARC intent handler platforms to continue the navigation.
+  </summary>
+  <int value="0" label="ARC"/>
+  <int value="1" label="Chrome"/>
+</enum>
+
 <enum name="ArcOptInAction" type="int">
   <summary>Defines Arc OptIn actions</summary>
   <int value="0" label="Opted Out"/>