Move the IntentPickerResponse callback to AppsNavigationThrottle.

This CL completes the refactoring of the ArcNavigationThrottle to be
app-platform agnostic by moving the callback run when the user responds
to the intent picker from ArcNavigationThrottle to
AppsNavigationThrottle. This will enable a future CL where the
AppsNavigationThrottle can insert non-ARC++ apps into the intent
picker, and then launch those apps if the user selects them.

There should be no functional changes from this CL. Two implementation
changes include:

1) the intent picker UI now returns an app-platform-generic CloseReason,
   and a boolean indicating if the user wanted to persist their choice.
   The AppsNavigationThrottle passes these to the ARC++ code, which
   interpolates them to the existing histogram buckets for metrics
   (which were formerly returned directly by the UI). This simplifies
   code in the UI layer, and moves responsibility for computing
   histogram values into the app-platform-specific code.

2) IntentPickerResponse is changed to be a OnceCallback, which removes
   some book-keeping in the UI layer for enforcing that it is only run
   once.

BUG=824598
TBR=sky@chromium.org

Change-Id: I1de4fd85c30e9ed09622ead11b709b9d6cdcfa98
Reviewed-on: https://chromium-review.googlesource.com/983197
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: David Jacobo <djacobo@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548306}

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

@@ -116,6 +116,21 @@ void AppsNavigationThrottle::ShowIntentPickerBubble(const Browser* browser,
                      browser, url));
 }
 
+// static
+void AppsNavigationThrottle::OnIntentPickerClosed(
+    const GURL& url,
+    const std::string& launch_name,
+    AppType app_type,
+    IntentPickerCloseReason close_reason,
+    bool should_persist) {
+  // Always call the ARC method as it records UMA stats.
+  // TODO(crbug.com/824598) consider cost of losing historical data against
+  // migrating the UMA metric out of the ARC namespace so the name is more
+  // sensible when other app platforms are added.
+  arc::ArcNavigationThrottle::OnIntentPickerClosed(
+      url, launch_name, app_type, close_reason, should_persist);
+}
+
 // static
 bool AppsNavigationThrottle::ShouldOverrideUrlLoadingForTesting(
     const GURL& previous_url,
@@ -169,11 +184,9 @@ void AppsNavigationThrottle::ShowIntentPickerBubbleForApps(
   if (apps.empty())
     return;
 
-  // TODO(crbug.com/824598): move the IntentPickerResponse callback and
-  // CloseReason enum/UMA to be in this class.
   chrome::QueryAndDisplayArcApps(
       browser, apps,
-      base::Bind(&arc::ArcNavigationThrottle::OnIntentPickerClosed, url));
+      base::BindOnce(&AppsNavigationThrottle::OnIntentPickerClosed, url));
 }
 
 void AppsNavigationThrottle::CancelNavigation() {

chrome/browser/chromeos/apps/intent_helper/apps_navigation_throttle.h

@@ -45,6 +45,17 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
   // bubble in |browser|.
   static void ShowIntentPickerBubble(const Browser* browser, const GURL& url);
 
+  // Called when the intent picker is closed for |url|, with |launch_name| as
+  // the (possibly empty) action to be triggered based on |app_type|.
+  // |close_reason| gives the reason for the picker being closed, and
+  // |should_persist| is true if the user indicated they wish to remember the
+  // choice made.
+  static void OnIntentPickerClosed(const GURL& url,
+                                   const std::string& launch_name,
+                                   AppType app_type,
+                                   IntentPickerCloseReason close_reason,
+                                   bool should_persist);
+
   static bool ShouldOverrideUrlLoadingForTesting(const GURL& previous_url,
                                                  const GURL& current_url);
 

chrome/browser/chromeos/apps/intent_helper/apps_navigation_types.h

@@ -13,10 +13,32 @@
 namespace chromeos {
 
 enum class AppType {
+  // Used for error scenarios and other cases where the app type isn't going to
+  // be used (e.g. not launching an app).
+  INVALID,
+
   // An Android app.
   ARC,
 };
 
+// Describes the possible ways for the intent picker to be closed.
+enum class IntentPickerCloseReason {
+  // There was an error in showing the intent picker.
+  ERROR,
+
+  // The user dismissed the picker without making a choice.
+  DIALOG_DEACTIVATED,
+
+  // A preferred app was found for launch.
+  PREFERRED_APP_FOUND,
+
+  // The user chose to stay in Chrome.
+  STAY_IN_CHROME,
+
+  // The user chose to open an app.
+  OPEN_APP,
+};
+
 enum class AppsNavigationAction {
   // The current navigation should be cancelled.
   CANCEL,

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

@@ -257,7 +257,9 @@ void OnIntentPickerClosed(int render_process_host_id,
                           const GURL& url,
                           std::vector<mojom::IntentHandlerInfoPtr> handlers,
                           const std::string& selected_app_package,
-                          ArcNavigationThrottle::CloseReason close_reason) {
+                          chromeos::AppType app_type,
+                          chromeos::IntentPickerCloseReason reason,
+                          bool should_persist) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // If the user selected an app to continue the navigation, confirm that the
@@ -273,18 +275,13 @@ void OnIntentPickerClosed(int render_process_host_id,
         arc_service_manager->arc_bridge_service()->intent_helper(), HandleUrl);
   }
 
-  if (!instance) {
-    close_reason = ArcNavigationThrottle::CloseReason::ERROR;
-  } else if (close_reason == ArcNavigationThrottle::CloseReason::
-                                 ARC_APP_PREFERRED_PRESSED ||
-             close_reason ==
-                 ArcNavigationThrottle::CloseReason::ARC_APP_PRESSED ||
-             close_reason ==
-                 ArcNavigationThrottle::CloseReason::CHROME_PREFERRED_PRESSED ||
-             close_reason ==
-                 ArcNavigationThrottle::CloseReason::CHROME_PRESSED) {
+  if (!instance)
+    reason = chromeos::IntentPickerCloseReason::ERROR;
+
+  if (reason == chromeos::IntentPickerCloseReason::OPEN_APP ||
+      reason == chromeos::IntentPickerCloseReason::STAY_IN_CHROME) {
     if (selected_app_index == handlers.size()) {
-      close_reason = ArcNavigationThrottle::CloseReason::ERROR;
+      reason = chromeos::IntentPickerCloseReason::ERROR;
     } else {
       // The user has made a selection. Clear g_last_* variables.
       g_last_url.Get() = GURL();
@@ -292,54 +289,49 @@ void OnIntentPickerClosed(int render_process_host_id,
     }
   }
 
-  switch (close_reason) {
-    case ArcNavigationThrottle::CloseReason::ARC_APP_PREFERRED_PRESSED: {
+  switch (reason) {
+    case chromeos::IntentPickerCloseReason::OPEN_APP:
+      // Only ARC apps are offered in the external protocol intent picker, so if
+      // the user decided to open in app the type must be ARC.
+      DCHECK_EQ(chromeos::AppType::ARC, app_type);
       DCHECK(arc_service_manager);
-      if (ARC_GET_INSTANCE_FOR_METHOD(
-              arc_service_manager->arc_bridge_service()->intent_helper(),
-              AddPreferredPackage)) {
-        instance->AddPreferredPackage(
-            handlers[selected_app_index]->package_name);
+
+      if (should_persist) {
+        if (ARC_GET_INSTANCE_FOR_METHOD(
+                arc_service_manager->arc_bridge_service()->intent_helper(),
+                AddPreferredPackage)) {
+          instance->AddPreferredPackage(
+              handlers[selected_app_index]->package_name);
+        }
       }
-      FALLTHROUGH;
-    }
-    case ArcNavigationThrottle::CloseReason::ARC_APP_PRESSED: {
+
       // Launch the selected app.
       HandleUrl(render_process_host_id, routing_id, url, false, handlers,
                 selected_app_index, nullptr);
       break;
-    }
-    case ArcNavigationThrottle::CloseReason::CHROME_PREFERRED_PRESSED:
-    case ArcNavigationThrottle::CloseReason::CHROME_PRESSED: {
+    case chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND:
+      // We shouldn't be here if a preferred app was found.
+      NOTREACHED();
+      return;  // no UMA recording.
+    case chromeos::IntentPickerCloseReason::STAY_IN_CHROME:
       LOG(ERROR) << "Chrome is not a valid option for external protocol URLs";
-      FALLTHROUGH;
-    }
-    case ArcNavigationThrottle::CloseReason::OBSOLETE_ALWAYS_PRESSED:
-    case ArcNavigationThrottle::CloseReason::OBSOLETE_JUST_ONCE_PRESSED:
-    case ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND:
-    case ArcNavigationThrottle::CloseReason::INVALID: {
       NOTREACHED();
       return;  // no UMA recording.
-    }
-    case ArcNavigationThrottle::CloseReason::ERROR: {
+    case chromeos::IntentPickerCloseReason::ERROR:
       LOG(ERROR) << "IntentPickerBubbleView returned CloseReason::ERROR: "
                  << "instance=" << instance
                  << ", selected_app_index=" << selected_app_index
                  << ", handlers.size=" << handlers.size();
       FALLTHROUGH;
-    }
-    case ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED: {
+    case chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED:
       // The user didn't select any ARC activity.
       OnIntentPickerDialogDeactivated(render_process_host_id, routing_id,
                                       handlers);
       break;
-    }
   }
 
-  ArcNavigationThrottle::Platform platform =
-      ArcNavigationThrottle::GetDestinationPlatform(selected_app_package,
-                                                    close_reason);
-  ArcNavigationThrottle::RecordUma(close_reason, platform);
+  ArcNavigationThrottle::RecordUma(selected_app_package, app_type, reason,
+                                   should_persist);
 }
 
 // Called when ARC returned activity icons for the |handlers|.
@@ -408,8 +400,9 @@ void OnUrlHandlerList(int render_process_host_id,
                 handlers, handlers.size(), &result)) {
     if (result == GetActionResult::HANDLE_URL_IN_ARC) {
       ArcNavigationThrottle::RecordUma(
-          ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND,
-          ArcNavigationThrottle::Platform::ARC);
+          std::string(), chromeos::AppType::ARC,
+          chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND,
+          false /* should_persist */);
     }
     return;  // the |url| has been handled.
   }

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

@@ -90,6 +90,57 @@ bool ArcNavigationThrottle::ShouldDeferRequest(
   return true;
 }
 
+// static
+ArcNavigationThrottle::Platform ArcNavigationThrottle::GetDestinationPlatform(
+    const std::string& selected_app_package,
+    PickerAction picker_action) {
+  switch (picker_action) {
+    case PickerAction::ARC_APP_PRESSED:
+    case PickerAction::ARC_APP_PREFERRED_PRESSED:
+      return Platform::ARC;
+    case PickerAction::ERROR:
+    case PickerAction::DIALOG_DEACTIVATED:
+    case PickerAction::CHROME_PRESSED:
+    case PickerAction::CHROME_PREFERRED_PRESSED:
+      return Platform::CHROME;
+    default:
+      return ArcIntentHelperBridge::IsIntentHelperPackage(selected_app_package)
+                 ? Platform::CHROME
+                 : Platform::ARC;
+  }
+  NOTREACHED();
+  return Platform::SIZE;
+}
+
+// static
+ArcNavigationThrottle::PickerAction ArcNavigationThrottle::GetPickerAction(
+    chromeos::AppType app_type,
+    chromeos::IntentPickerCloseReason close_reason,
+    bool should_persist) {
+  switch (close_reason) {
+    case chromeos::IntentPickerCloseReason::ERROR:
+      return PickerAction::ERROR;
+    case chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED:
+      return PickerAction::DIALOG_DEACTIVATED;
+    case chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND:
+      return PickerAction::PREFERRED_ACTIVITY_FOUND;
+    case chromeos::IntentPickerCloseReason::STAY_IN_CHROME:
+      return should_persist ? PickerAction::CHROME_PREFERRED_PRESSED
+                            : PickerAction::CHROME_PRESSED;
+    case chromeos::IntentPickerCloseReason::OPEN_APP:
+      switch (app_type) {
+        case chromeos::AppType::INVALID:
+          return PickerAction::INVALID;
+        case chromeos::AppType::ARC:
+          return should_persist ? PickerAction::ARC_APP_PREFERRED_PRESSED
+                                : PickerAction::ARC_APP_PRESSED;
+      }
+  }
+
+  NOTREACHED();
+  return PickerAction::INVALID;
+}
+
 void ArcNavigationThrottle::OnAppCandidatesReceived(
     content::NavigationHandle* handle,
     chromeos::AppsNavigationCallback callback,
@@ -99,7 +150,9 @@ void ArcNavigationThrottle::OnAppCandidatesReceived(
     // This scenario shouldn't be accessed as ArcNavigationThrottle is created
     // iff there are ARC apps which can actually handle the given URL.
     DVLOG(1) << "There are no app candidates for this URL: " << url;
-    RecordUma(CloseReason::ERROR, Platform::CHROME);
+    RecordUma(std::string(), chromeos::AppType::INVALID,
+              chromeos::IntentPickerCloseReason::ERROR,
+              false /* should_persist */);
     std::move(callback).Run(chromeos::AppsNavigationAction::RESUME, {});
     return;
   }
@@ -128,7 +181,7 @@ bool ArcNavigationThrottle::DidLaunchPreferredArcApp(
   bool cancel_navigation = false;
   const size_t index = FindPreferredApp(app_candidates, url);
   if (index != app_candidates.size()) {
-    CloseReason close_reason = CloseReason::PREFERRED_ACTIVITY_FOUND;
+    auto close_reason = chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND;
     const std::string package_name = app_candidates[index]->package_name;
 
     // Make sure that the instance at least supports HandleUrl.
@@ -141,7 +194,7 @@ bool ArcNavigationThrottle::DidLaunchPreferredArcApp(
     }
 
     if (!instance) {
-      close_reason = CloseReason::ERROR;
+      close_reason = chromeos::IntentPickerCloseReason::ERROR;
     } else if (ArcIntentHelperBridge::IsIntentHelperPackage(package_name)) {
       chrome::SetIntentPickerViewVisibility(
           chrome::FindBrowserWithWebContents(web_contents), true);
@@ -149,9 +202,8 @@ bool ArcNavigationThrottle::DidLaunchPreferredArcApp(
       instance->HandleUrl(url.spec(), package_name);
       cancel_navigation = true;
     }
-
-    Platform platform = GetDestinationPlatform(package_name, close_reason);
-    RecordUma(close_reason, platform);
+    RecordUma(package_name, chromeos::AppType::ARC, close_reason,
+              false /* should_persist */);
   }
   return cancel_navigation;
 }
@@ -167,7 +219,9 @@ void ArcNavigationThrottle::ArcAppIconQuery(
       web_contents->GetBrowserContext());
   if (!intent_helper_bridge) {
     LOG(ERROR) << "Cannot get an instance of ArcIntentHelperBridge";
-    RecordUma(CloseReason::ERROR, Platform::CHROME);
+    RecordUma(std::string(), chromeos::AppType::INVALID,
+              chromeos::IntentPickerCloseReason::ERROR,
+              false /* should_persist */);
     std::move(callback).Run({});
     return;
   }
@@ -209,61 +263,36 @@ void ArcNavigationThrottle::OnAppIconsReceived(
 void ArcNavigationThrottle::OnIntentPickerClosed(
     const GURL& url,
     const std::string& pkg,
-    arc::ArcNavigationThrottle::CloseReason close_reason) {
+    chromeos::AppType app_type,
+    chromeos::IntentPickerCloseReason reason,
+    bool should_persist) {
   auto* arc_service_manager = arc::ArcServiceManager::Get();
   arc::mojom::IntentHelperInstance* instance = nullptr;
   if (arc_service_manager) {
     instance = ARC_GET_INSTANCE_FOR_METHOD(
         arc_service_manager->arc_bridge_service()->intent_helper(), HandleUrl);
   }
+
   if (!instance)
-    close_reason = CloseReason::ERROR;
+    reason = chromeos::IntentPickerCloseReason::ERROR;
 
-  switch (close_reason) {
-    case CloseReason::ERROR:
-    case CloseReason::CHROME_PRESSED:
-    case CloseReason::DIALOG_DEACTIVATED: {
-      break;
-    }
-    case CloseReason::PREFERRED_ACTIVITY_FOUND: {
-      if (!arc::ArcIntentHelperBridge::IsIntentHelperPackage(pkg))
-        instance->HandleUrl(url.spec(), pkg);
-      break;
-    }
-    case arc::ArcNavigationThrottle::CloseReason::ARC_APP_PREFERRED_PRESSED: {
-      DCHECK(arc_service_manager);
-      if (ARC_GET_INSTANCE_FOR_METHOD(
-              arc_service_manager->arc_bridge_service()->intent_helper(),
-              AddPreferredPackage)) {
-        instance->AddPreferredPackage(pkg);
-      }
-      instance->HandleUrl(url.spec(), pkg);
-      break;
-    }
-    case arc::ArcNavigationThrottle::CloseReason::CHROME_PREFERRED_PRESSED: {
+  if (reason == chromeos::IntentPickerCloseReason::STAY_IN_CHROME ||
+      (reason == chromeos::IntentPickerCloseReason::OPEN_APP &&
+       app_type == chromeos::AppType::ARC)) {
+    if (should_persist) {
       DCHECK(arc_service_manager);
       if (ARC_GET_INSTANCE_FOR_METHOD(
               arc_service_manager->arc_bridge_service()->intent_helper(),
               AddPreferredPackage)) {
         instance->AddPreferredPackage(pkg);
       }
-      break;
-    }
-    case arc::ArcNavigationThrottle::CloseReason::ARC_APP_PRESSED: {
-      instance->HandleUrl(url.spec(), pkg);
-      break;
-    }
-    case arc::ArcNavigationThrottle::CloseReason::OBSOLETE_ALWAYS_PRESSED:
-    case arc::ArcNavigationThrottle::CloseReason::OBSOLETE_JUST_ONCE_PRESSED:
-    case arc::ArcNavigationThrottle::CloseReason::INVALID: {
-      NOTREACHED();
-      return;
     }
   }
 
-  arc::ArcNavigationThrottle::Platform platform =
-      arc::ArcNavigationThrottle::GetDestinationPlatform(pkg, close_reason);
-  arc::ArcNavigationThrottle::RecordUma(close_reason, platform);
+  if (reason == chromeos::IntentPickerCloseReason::OPEN_APP)
+    instance->HandleUrl(url.spec(), pkg);
+
+  RecordUma(pkg, app_type, reason, should_persist);
 }
 
 // static
@@ -278,26 +307,19 @@ size_t ArcNavigationThrottle::GetAppIndex(
 }
 
 // static
-ArcNavigationThrottle::Platform ArcNavigationThrottle::GetDestinationPlatform(
+void ArcNavigationThrottle::RecordUma(
     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;
-}
+    chromeos::AppType app_type,
+    chromeos::IntentPickerCloseReason close_reason,
+    bool should_persist) {
+  PickerAction action = GetPickerAction(app_type, close_reason, should_persist);
+  Platform platform = GetDestinationPlatform(selected_app_package, action);
 
-// 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));
+  UMA_HISTOGRAM_ENUMERATION("Arc.IntentHandlerAction", action,
+                            PickerAction::SIZE);
+
+  UMA_HISTOGRAM_ENUMERATION("Arc.IntentHandlerDestinationPlatform", platform,
+                            Platform::SIZE);
 }
 
 // static

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

@@ -28,10 +28,63 @@ namespace arc {
 // traffic initiated on Chrome browser, either on Chrome or an ARC's app.
 class ArcNavigationThrottle {
  public:
+  ArcNavigationThrottle();
+  ~ArcNavigationThrottle();
+
+  // Returns true if the navigation request represented by |handle| should be
+  // deferred while ARC is queried for apps, and if so, |callback| will be run
+  // asynchronously with the action for the navigation. |callback| will not be
+  // run if false is returned.
+  bool ShouldDeferRequest(content::NavigationHandle* handle,
+                          chromeos::AppsNavigationCallback callback);
+
+  // Finds |selected_app_package| from the |app_candidates| array and returns
+  // the index. If the app is not found, returns |app_candidates.size()|.
+  static size_t GetAppIndex(
+      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates,
+      const std::string& selected_app_package);
+
+  // Records intent picker usage statistics as well as whether navigations are
+  // continued or redirected to an app via UMA histograms.
+  static void RecordUma(const std::string& selected_app_package,
+                        chromeos::AppType app_type,
+                        chromeos::IntentPickerCloseReason close_reason,
+                        bool should_persist);
+
+  // TODO(djacobo): Remove this function and instead stop ARC from returning
+  // Chrome as a valid app candidate.
+  // Records true if |app_candidates| contain one or more apps. When this
+  // function is called from OnAppCandidatesReceived, |app_candidates| always
+  // contains Chrome (aka intent helper), but the same function doesn't treat
+  // this as an app.
+  static bool IsAppAvailable(
+      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
+
+  static bool IsAppAvailableForTesting(
+      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
+  static size_t FindPreferredAppForTesting(
+      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
+  static void QueryArcApps(const Browser* browser,
+                           const GURL& url,
+                           chromeos::QueryAppsCallback callback);
+
+  // Called to record UMA for the intent picker, and launch an ARC app if it was
+  // selected by the user.
+  static void OnIntentPickerClosed(const GURL& url,
+                                   const std::string& package_name,
+                                   chromeos::AppType app_type,
+                                   chromeos::IntentPickerCloseReason reason,
+                                   bool should_persist);
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(ArcNavigationThrottleTest, TestGetPickerAction);
+  FRIEND_TEST_ALL_PREFIXES(ArcNavigationThrottleTest,
+                           TestGetDestinationPlatform);
+
   // These enums are used to define the buckets for an enumerated UMA histogram
   // and need to be synced with histograms.xml. This enum class should also be
   // treated as append-only.
-  enum class CloseReason : int {
+  enum class PickerAction : int {
     ERROR = 0,
     // DIALOG_DEACTIVATED keeps track of the user dismissing the UI via clicking
     // the close button or clicking outside of the IntentPickerBubbleView
@@ -55,7 +108,7 @@ class ArcNavigationThrottle {
     INVALID = SIZE,
   };
 
-  // As for CloseReason, these define the buckets for an UMA histogram, so this
+  // As for PickerAction, 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 {
@@ -64,53 +117,21 @@ class ArcNavigationThrottle {
     SIZE,
   };
 
-  ArcNavigationThrottle();
-  ~ArcNavigationThrottle();
-
-  // Returns true if the navigation request represented by |handle| should be
-  // deferred while ARC is queried for apps, and if so, |callback| will be run
-  // asynchronously with the action for the navigation. |callback| will not be
-  // run if false is returned.
-  bool ShouldDeferRequest(content::NavigationHandle* handle,
-                          chromeos::AppsNavigationCallback callback);
-
-  // Finds |selected_app_package| from the |app_candidates| array and returns
-  // the index. If the app is not found, returns |app_candidates.size()|.
-  static size_t GetAppIndex(
-      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates,
-      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
+  // |picker_action| 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);
-  // TODO(djacobo): Remove this function and instead stop ARC from returning
-  // Chrome as a valid app candidate.
-  // Records true if |app_candidates| contain one or more apps. When this
-  // function is called from OnAppCandidatesReceived, |app_candidates| always
-  // contains Chrome (aka intent helper), but the same function doesn't treat
-  // this as an app.
-  static bool IsAppAvailable(
-      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
+      PickerAction picker_action);
 
-  static bool IsAppAvailableForTesting(
-      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
-  static size_t FindPreferredAppForTesting(
-      const std::vector<mojom::IntentHandlerInfoPtr>& app_candidates);
-  static void QueryArcApps(const Browser* browser,
-                           const GURL& url,
-                           chromeos::QueryAppsCallback callback);
-  static void OnIntentPickerClosed(
-      const GURL& url,
-      const std::string& package_name,
-      arc::ArcNavigationThrottle::CloseReason close_reason);
+  // Converts the provided |app_type|, |close_reason| and |should_persist|
+  // boolean to a PickerAction value for recording in UMA.
+  static PickerAction GetPickerAction(
+      chromeos::AppType app_type,
+      chromeos::IntentPickerCloseReason close_reason,
+      bool should_persist);
 
- private:
   // Determines whether we should open a preferred app or show the intent
   // picker. Resume/Cancel the navigation which was put in DEFER. Close the
   // current tab only if we continue the navigation on ARC and the current tab

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

@@ -100,58 +100,169 @@ TEST(ArcNavigationThrottleTest, TestGetAppIndex) {
       2u, ArcNavigationThrottle::GetAppIndex(CreateArray(3, 2), package_name));
 }
 
+TEST(ArcNavigationThrottleTest, TestGetPickerAction) {
+  // Expect PickerAction::ERROR if the close_reason is ERROR.
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ERROR,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::ERROR, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ERROR,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::ERROR, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ERROR,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::ERROR, false));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ERROR,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::ERROR, false));
+
+  // Expect PickerAction::DIALOG_DEACTIVATED if the close_reason is
+  // DIALOG_DEACTIVATED.
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, false));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, false));
+
+  // Expect PickerAction::PREFERRED_ACTIVITY_FOUND if the close_reason is
+  // PREFERRED_APP_FOUND.
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND, false));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::PREFERRED_APP_FOUND, false));
+
+  // Expect PREFERRED depending on the value of |should_persist|, and |app_type|
+  // to be ignored if reason is STAY_IN_CHROME.
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::CHROME_PREFERRED_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::STAY_IN_CHROME, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::CHROME_PREFERRED_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::STAY_IN_CHROME, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::CHROME_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::STAY_IN_CHROME, false));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::CHROME_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::STAY_IN_CHROME, false));
+
+  // Expect PREFERRED depending on the value of |should_persist|, and
+  // INVALID/ARC to be chosen if reason is OPEN_APP.
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::INVALID,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::OPEN_APP, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ARC_APP_PREFERRED_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::OPEN_APP, true));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::INVALID,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::INVALID,
+                chromeos::IntentPickerCloseReason::OPEN_APP, false));
+
+  EXPECT_EQ(ArcNavigationThrottle::PickerAction::ARC_APP_PRESSED,
+            ArcNavigationThrottle::GetPickerAction(
+                chromeos::AppType::ARC,
+                chromeos::IntentPickerCloseReason::OPEN_APP, false));
+}
+
 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
+  // When the PickerAction 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));
+                chrome_app, ArcNavigationThrottle::PickerAction::ERROR));
   EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
             ArcNavigationThrottle::GetDestinationPlatform(
-                non_chrome_app, ArcNavigationThrottle::CloseReason::ERROR));
+                non_chrome_app, ArcNavigationThrottle::PickerAction::ERROR));
   EXPECT_EQ(
       ArcNavigationThrottle::Platform::CHROME,
       ArcNavigationThrottle::GetDestinationPlatform(
-          chrome_app, ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED));
+          chrome_app, ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED));
   EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
             ArcNavigationThrottle::GetDestinationPlatform(
                 non_chrome_app,
-                ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED));
+                ArcNavigationThrottle::PickerAction::DIALOG_DEACTIVATED));
 
-  // Under any other CloseReason, stay in Chrome only if the package is Chrome.
+  // Under any other PickerAction, stay in Chrome only if the package is Chrome.
   // Otherwise redirect to ARC.
   EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
             ArcNavigationThrottle::GetDestinationPlatform(
                 chrome_app,
-                ArcNavigationThrottle::CloseReason::OBSOLETE_ALWAYS_PRESSED));
+                ArcNavigationThrottle::PickerAction::OBSOLETE_ALWAYS_PRESSED));
   EXPECT_EQ(
       ArcNavigationThrottle::Platform::CHROME,
       ArcNavigationThrottle::GetDestinationPlatform(
           chrome_app,
-          ArcNavigationThrottle::CloseReason::OBSOLETE_JUST_ONCE_PRESSED));
+          ArcNavigationThrottle::PickerAction::OBSOLETE_JUST_ONCE_PRESSED));
   EXPECT_EQ(ArcNavigationThrottle::Platform::CHROME,
             ArcNavigationThrottle::GetDestinationPlatform(
                 chrome_app,
-                ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND));
+                ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND));
 
   // Go to ARC on any other case.
   EXPECT_EQ(ArcNavigationThrottle::Platform::ARC,
             ArcNavigationThrottle::GetDestinationPlatform(
                 non_chrome_app,
-                ArcNavigationThrottle::CloseReason::OBSOLETE_ALWAYS_PRESSED));
+                ArcNavigationThrottle::PickerAction::OBSOLETE_ALWAYS_PRESSED));
   EXPECT_EQ(
       ArcNavigationThrottle::Platform::ARC,
       ArcNavigationThrottle::GetDestinationPlatform(
           non_chrome_app,
-          ArcNavigationThrottle::CloseReason::OBSOLETE_JUST_ONCE_PRESSED));
+          ArcNavigationThrottle::PickerAction::OBSOLETE_JUST_ONCE_PRESSED));
   EXPECT_EQ(ArcNavigationThrottle::Platform::ARC,
             ArcNavigationThrottle::GetDestinationPlatform(
                 non_chrome_app,
-                ArcNavigationThrottle::CloseReason::PREFERRED_ACTIVITY_FOUND));
+                ArcNavigationThrottle::PickerAction::PREFERRED_ACTIVITY_FOUND));
 }
 
 }  // namespace arc

chrome/browser/ui/browser_commands.cc

@@ -1234,7 +1234,7 @@ void QueryAndDisplayArcApps(
     const Browser* browser,
     const std::vector<chromeos::IntentPickerAppInfo>& app_info,
     IntentPickerResponse callback) {
-  browser->window()->ShowIntentPickerBubble(app_info, callback);
+  browser->window()->ShowIntentPickerBubble(app_info, std::move(callback));
 }
 
 void SetIntentPickerViewVisibility(Browser* browser, bool visible) {

chrome/browser/ui/browser_dialogs.h

@@ -312,14 +312,16 @@ void ShowChromeCleanerRebootPrompt(
 
 #if defined(OS_CHROMEOS)
 
-// This callback informs the package name of the app selected by the user, along
-// with the reason why the Bubble was closed. The string param must have a valid
-// package name, except when the CloseReason is ERROR or DIALOG_DEACTIVATED, for
-// these cases we return a dummy value which won't be used at all and has no
-// significance.
+// This callback informs the launch name and type of the app selected by the
+// user, along with the reason why the Bubble was closed and whether the
+// decision should be persisted. When the reason is ERROR or DIALOG_DEACTIVATED,
+// the values of the launch name, app type, and persistence boolean are all
+// ignored.
 using IntentPickerResponse =
-    base::Callback<void(const std::string&,
-                        arc::ArcNavigationThrottle::CloseReason)>;
+    base::OnceCallback<void(const std::string&,
+                            chromeos::AppType,
+                            chromeos::IntentPickerCloseReason,
+                            bool should_persist)>;
 
 // TODO(djacobo): Decide whether or not refactor as base::RepeatableCallback.
 // Return a pointer to the IntentPickerBubbleView::ShowBubble method, which in
@@ -332,7 +334,7 @@ using BubbleShowPtr =
                        content::WebContents*,
                        const std::vector<chromeos::IntentPickerAppInfo>&,
                        bool disable_display_in_chrome,
-                       const IntentPickerResponse&);
+                       IntentPickerResponse);
 
 BubbleShowPtr ShowIntentPickerBubble();
 

chrome/browser/ui/views/frame/browser_view.cc

@@ -1157,7 +1157,7 @@ void BrowserView::ShowUpdateChromeDialog() {
 void BrowserView::ShowIntentPickerBubble(
     const std::vector<IntentPickerBubbleView::AppInfo>& app_info,
     IntentPickerResponse callback) {
-  toolbar_->ShowIntentPickerBubble(app_info, callback);
+  toolbar_->ShowIntentPickerBubble(app_info, std::move(callback));
 }
 
 void BrowserView::SetIntentPickerViewVisibility(bool visible) {

chrome/browser/ui/views/intent_picker_bubble_view.cc

@@ -4,9 +4,9 @@
 
 #include "chrome/browser/ui/views/intent_picker_bubble_view.h"
 
-#include "base/bind.h"
+#include <utility>
+
 #include "base/i18n/rtl.h"
-#include "base/logging.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/apps/intent_helper/apps_navigation_throttle.h"
@@ -19,7 +19,6 @@
 #include "content/public/browser/navigation_handle.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/views/animation/ink_drop_host_view.h"
 #include "ui/views/border.h"
@@ -115,7 +114,7 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
     content::WebContents* web_contents,
     const std::vector<AppInfo>& app_info,
     bool disable_stay_in_chrome,
-    const IntentPickerResponse& intent_picker_cb) {
+    IntentPickerResponse intent_picker_cb) {
   if (intent_picker_bubble_) {
     views::Widget* widget =
         views::BubbleDialogDelegateView::CreateBubble(intent_picker_bubble_);
@@ -124,13 +123,15 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
   }
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
   if (!browser || !BrowserView::GetBrowserViewForBrowser(browser)) {
-    intent_picker_cb.Run(kInvalidLaunchName,
-                         arc::ArcNavigationThrottle::CloseReason::ERROR);
+    std::move(intent_picker_cb)
+        .Run(kInvalidLaunchName, chromeos::AppType::INVALID,
+             chromeos::IntentPickerCloseReason::ERROR, false);
     return nullptr;
   }
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
-  intent_picker_bubble_ = new IntentPickerBubbleView(
-      app_info, intent_picker_cb, web_contents, disable_stay_in_chrome);
+  intent_picker_bubble_ =
+      new IntentPickerBubbleView(app_info, std::move(intent_picker_cb),
+                                 web_contents, disable_stay_in_chrome);
   intent_picker_bubble_->set_margins(gfx::Insets());
 
   if (anchor_view) {
@@ -162,13 +163,13 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
 
 // static
 std::unique_ptr<IntentPickerBubbleView>
-IntentPickerBubbleView::CreateBubbleView(
-    const std::vector<AppInfo>& app_info,
-    bool disable_stay_in_chrome,
-    const IntentPickerResponse& intent_picker_cb,
-    content::WebContents* web_contents) {
-  std::unique_ptr<IntentPickerBubbleView> bubble(new IntentPickerBubbleView(
-      app_info, intent_picker_cb, web_contents, disable_stay_in_chrome));
+IntentPickerBubbleView::CreateBubbleView(const std::vector<AppInfo>& app_info,
+                                         bool disable_stay_in_chrome,
+                                         IntentPickerResponse intent_picker_cb,
+                                         content::WebContents* web_contents) {
+  std::unique_ptr<IntentPickerBubbleView> bubble(
+      new IntentPickerBubbleView(app_info, std::move(intent_picker_cb),
+                                 web_contents, disable_stay_in_chrome));
   bubble->Init();
   return bubble;
 }
@@ -185,28 +186,26 @@ void IntentPickerBubbleView::CloseBubble() {
 }
 
 bool IntentPickerBubbleView::Accept() {
-  RunCallback(
-      app_info_[selected_app_tag_].launch_name,
-      remember_selection_checkbox_->checked()
-          ? arc::ArcNavigationThrottle::CloseReason::ARC_APP_PREFERRED_PRESSED
-          : arc::ArcNavigationThrottle::CloseReason::ARC_APP_PRESSED);
+  RunCallback(app_info_[selected_app_tag_].launch_name,
+              app_info_[selected_app_tag_].type,
+              chromeos::IntentPickerCloseReason::OPEN_APP,
+              remember_selection_checkbox_->checked());
   return true;
 }
 
 bool IntentPickerBubbleView::Cancel() {
-  RunCallback(
-      arc::ArcIntentHelperBridge::kArcIntentHelperPackageName,
-      remember_selection_checkbox_->checked()
-          ? arc::ArcNavigationThrottle::CloseReason::CHROME_PREFERRED_PRESSED
-          : arc::ArcNavigationThrottle::CloseReason::CHROME_PRESSED);
+  RunCallback(arc::ArcIntentHelperBridge::kArcIntentHelperPackageName,
+              chromeos::AppType::INVALID,
+              chromeos::IntentPickerCloseReason::STAY_IN_CHROME,
+              remember_selection_checkbox_->checked());
   return true;
 }
 
 bool IntentPickerBubbleView::Close() {
   // Whenever closing the bubble without pressing |Just once| or |Always| we
   // need to report back that the user didn't select anything.
-  RunCallback(kInvalidLaunchName,
-              arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
+  RunCallback(kInvalidLaunchName, chromeos::AppType::INVALID,
+              chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, false);
   return true;
 }
 
@@ -310,7 +309,7 @@ IntentPickerBubbleView::IntentPickerBubbleView(
     : LocationBarBubbleDelegateView(nullptr /* anchor_view */,
                                     gfx::Point(),
                                     web_contents),
-      intent_picker_cb_(intent_picker_cb),
+      intent_picker_cb_(std::move(intent_picker_cb)),
       selected_app_tag_(0),
       scroll_view_(nullptr),
       app_info_(app_info),
@@ -326,8 +325,8 @@ IntentPickerBubbleView::~IntentPickerBubbleView() {
 // If the widget gets closed without an app being selected we still need to use
 // the callback so the caller can Resume the navigation.
 void IntentPickerBubbleView::OnWidgetDestroying(views::Widget* widget) {
-  RunCallback(kInvalidLaunchName,
-              arc::ArcNavigationThrottle::CloseReason::DIALOG_DEACTIVATED);
+  RunCallback(kInvalidLaunchName, chromeos::AppType::INVALID,
+              chromeos::IntentPickerCloseReason::DIALOG_DEACTIVATED, false);
 }
 
 void IntentPickerBubbleView::ButtonPressed(views::Button* sender,
@@ -377,14 +376,13 @@ IntentPickerLabelButton* IntentPickerBubbleView::GetIntentPickerLabelButtonAt(
 
 void IntentPickerBubbleView::RunCallback(
     const std::string& launch_name,
-    arc::ArcNavigationThrottle::CloseReason close_reason) {
+    chromeos::AppType app_type,
+    chromeos::IntentPickerCloseReason close_reason,
+    bool should_persist) {
   if (!intent_picker_cb_.is_null()) {
-    // We must ensure |intent_picker_cb_| is only Run() once, this is why we
-    // have a temporary |callback| helper, so we can set the original callback
-    // to null and still report back to whoever started the UI.
-    auto callback = intent_picker_cb_;
-    intent_picker_cb_.Reset();
-    callback.Run(launch_name, close_reason);
+    // Calling Run() will make |intent_picker_cb_| null.
+    std::move(intent_picker_cb_)
+        .Run(launch_name, app_type, close_reason, should_persist);
   }
 
   intent_picker_bubble_ = nullptr;

chrome/browser/ui/views/intent_picker_bubble_view.h

@@ -9,12 +9,10 @@
 #include <string>
 #include <vector>
 
-#include "base/callback.h"
 #include "base/macros.h"
 #include "chrome/browser/chromeos/apps/intent_helper/apps_navigation_types.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/gfx/image/image.h"
 #include "ui/views/controls/button/button.h"
@@ -41,8 +39,8 @@ class IntentPickerLabelButton;
 // outside of the bubble allows the user to dismiss the bubble (and stay in
 // Chrome) without remembering any decision.
 //
-// This class comunicates the user's selection with a callback used by
-// ArcNavigationThrottle.
+// This class communicates the user's selection with a callback supplied by
+// AppsNavigationThrottle.
 //   +--------------------------------+
 //   | Open with                  [x] |
 //   |                                |
@@ -62,16 +60,15 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView,
   using AppInfo = chromeos::IntentPickerAppInfo;
 
   ~IntentPickerBubbleView() override;
-  static views::Widget* ShowBubble(
-      views::View* anchor_view,
-      content::WebContents* web_contents,
-      const std::vector<AppInfo>& app_info,
-      bool disable_stay_in_chrome,
-      const IntentPickerResponse& intent_picker_cb);
+  static views::Widget* ShowBubble(views::View* anchor_view,
+                                   content::WebContents* web_contents,
+                                   const std::vector<AppInfo>& app_info,
+                                   bool disable_stay_in_chrome,
+                                   IntentPickerResponse intent_picker_cb);
   static std::unique_ptr<IntentPickerBubbleView> CreateBubbleView(
       const std::vector<AppInfo>& app_info,
       bool disable_stay_in_chrome,
-      const IntentPickerResponse& intent_picker_cb,
+      IntentPickerResponse intent_picker_cb,
       content::WebContents* web_contents);
   static IntentPickerBubbleView* intent_picker_bubble() {
     return intent_picker_bubble_;
@@ -125,7 +122,9 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView,
   // the internal ScrollView.
   IntentPickerLabelButton* GetIntentPickerLabelButtonAt(size_t index);
   void RunCallback(const std::string& launch_name,
-                   arc::ArcNavigationThrottle::CloseReason close_reason);
+                   chromeos::AppType app_type,
+                   chromeos::IntentPickerCloseReason close_reason,
+                   bool should_persist);
 
   // Accessory for |scroll_view_|'s contents size.
   size_t GetScrollViewSize() const;
@@ -146,7 +145,7 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView,
 
   static IntentPickerBubbleView* intent_picker_bubble_;
 
-  // Callback used to respond to ArcNavigationThrottle.
+  // Callback used to respond to AppsNavigationThrottle.
   IntentPickerResponse intent_picker_cb_;
 
   // Pre-select the first app on the list.

chrome/browser/ui/views/intent_picker_bubble_view_unittest.cc

@@ -79,7 +79,9 @@ class IntentPickerBubbleViewTest : public BrowserWithTestWindowTest {
 
   // Dummy method to be called upon bubble closing.
   void OnBubbleClosed(const std::string& selected_app_package,
-                      arc::ArcNavigationThrottle::CloseReason close_reason) {}
+                      chromeos::AppType app_type,
+                      chromeos::IntentPickerCloseReason close_reason,
+                      bool should_persist) {}
 
   std::unique_ptr<IntentPickerBubbleView> bubble_;
   std::vector<AppInfo> app_info_;

chrome/browser/ui/views/toolbar/toolbar_view.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "base/command_line.h"
 #include "base/i18n/number_formatting.h"
@@ -303,7 +304,7 @@ void ToolbarView::ShowIntentPickerBubble(
 
     views::Widget* bubble_widget = IntentPickerBubbleView::ShowBubble(
         intent_picker_view, GetWebContents(), app_info,
-        false /* disable_stay_in_chrome */, callback);
+        false /* disable_stay_in_chrome */, std::move(callback));
     if (bubble_widget && intent_picker_view)
       intent_picker_view->OnBubbleWidgetCreated(bubble_widget);
   }