[Tablet Form Factor] Block initial preferred ARC app launch.

On devices with tablet form factor, links clicked from Chrome
should not automatically launch preferred ARC apps, but instead
open the page with the intent picker such that the user can make
a selection.

BUG=1019549

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

chrome/browser/apps/intent_helper/intent_picker_auto_display_pref.cc

@@ -17,6 +17,7 @@ namespace {
 constexpr int kDismissThreshold = 2;
 
 constexpr char kAutoDisplayKey[] = "picker_auto_display_key";
+constexpr char kPlatformKey[] = "picker_platform_key";
 
 // Retrieves or creates a new dictionary for the specific |origin|.
 std::unique_ptr<base::DictionaryValue> GetAutoDisplayDictForSettings(
@@ -60,11 +61,28 @@ bool IntentPickerAutoDisplayPref::HasExceededThreshold() {
   return ui_dismissed_counter_ < kDismissThreshold;
 }
 
+IntentPickerAutoDisplayPref::Platform
+IntentPickerAutoDisplayPref::GetPlatform() {
+  return platform_;
+}
+
+void IntentPickerAutoDisplayPref::UpdatePlatform(Platform platform) {
+  if (!pref_dict_)
+    return;
+
+  DCHECK_GE(static_cast<int>(platform), static_cast<int>(Platform::kNone));
+  DCHECK_LE(static_cast<int>(platform), static_cast<int>(Platform::kMaxValue));
+  platform_ = platform;
+  pref_dict_->SetInteger(kPlatformKey, static_cast<int>(platform_));
+  Commit();
+}
+
 IntentPickerAutoDisplayPref::IntentPickerAutoDisplayPref(
     const GURL& origin,
     std::unique_ptr<base::DictionaryValue> pref_dict)
     : origin_(origin), pref_dict_(pref_dict.release()) {
   ui_dismissed_counter_ = QueryDismissedCounter();
+  platform_ = QueryPlatform();
 }
 
 int IntentPickerAutoDisplayPref::QueryDismissedCounter() {
@@ -73,6 +91,8 @@ int IntentPickerAutoDisplayPref::QueryDismissedCounter() {
 
   int counter = 0;
   pref_dict_->GetInteger(kAutoDisplayKey, &counter);
+  DCHECK_GE(counter, static_cast<int>(Platform::kNone));
+  DCHECK_LE(counter, static_cast<int>(Platform::kMaxValue));
 
   return counter;
 }
@@ -87,6 +107,19 @@ void IntentPickerAutoDisplayPref::SetDismissedCounter(int new_counter) {
   pref_dict_->SetInteger(kAutoDisplayKey, ui_dismissed_counter_);
 }
 
+IntentPickerAutoDisplayPref::Platform
+IntentPickerAutoDisplayPref::QueryPlatform() {
+  if (!pref_dict_)
+    return Platform::kNone;
+
+  int platform = 0;
+  pref_dict_->GetInteger(kPlatformKey, &platform);
+  DCHECK_GE(platform, static_cast<int>(Platform::kNone));
+  DCHECK_LT(platform, static_cast<int>(Platform::kMaxValue));
+
+  return static_cast<Platform>(platform);
+}
+
 void IntentPickerAutoDisplayPref::Commit() {
   settings_map_->SetWebsiteSettingDefaultScope(
       origin_, origin_, ContentSettingsType::INTENT_PICKER_DISPLAY,

chrome/browser/apps/intent_helper/intent_picker_auto_display_pref.h

@@ -17,6 +17,10 @@ class HostContentSettingsMap;
 // UI for a given origin.
 class IntentPickerAutoDisplayPref final {
  public:
+  // The platform selected by the user to handle this URL for devices of tablet
+  // form factor.
+  enum class Platform { kNone = 0, kArc = 1, kChrome = 2, kMaxValue = kChrome };
+
   IntentPickerAutoDisplayPref(const GURL& origin,
                               HostContentSettingsMap* settings);
   ~IntentPickerAutoDisplayPref();
@@ -25,6 +29,10 @@ class IntentPickerAutoDisplayPref final {
 
   bool HasExceededThreshold();
 
+  Platform GetPlatform();
+
+  void UpdatePlatform(Platform platform);
+
  private:
   // Creates and keep track of the dictionary for this specific origin.
   IntentPickerAutoDisplayPref(const GURL& origin,
@@ -34,6 +42,8 @@ class IntentPickerAutoDisplayPref final {
 
   void SetDismissedCounter(int new_counter);
 
+  Platform QueryPlatform();
+
   void Commit();
 
   // Origin associated to this preference.
@@ -41,6 +51,10 @@ class IntentPickerAutoDisplayPref final {
 
   int ui_dismissed_counter_;
 
+  // The platform selected by the user to handle link navigations with.
+  // This is only for devices of tablet form factor.
+  Platform platform_;
+
   // Dictionary for this particular preference.
   std::unique_ptr<base::DictionaryValue> pref_dict_;
 

chrome/browser/apps/intent_helper/intent_picker_auto_display_service.cc

@@ -6,7 +6,6 @@
 
 #include <memory>
 
-#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_pref.h"
 #include "chrome/browser/apps/intent_helper/intent_picker_auto_display_service_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 
@@ -41,3 +40,14 @@ bool IntentPickerAutoDisplayService::ShouldAutoDisplayUi(const GURL& url) {
 void IntentPickerAutoDisplayService::IncrementCounter(const GURL& url) {
   CreateLocalPreference(profile_, url)->IncrementCounter();
 }
+
+IntentPickerAutoDisplayPref::Platform
+IntentPickerAutoDisplayService::GetLastUsedPlatformForTablets(const GURL& url) {
+  return CreateLocalPreference(profile_, url)->GetPlatform();
+}
+
+void IntentPickerAutoDisplayService::UpdatePlatformForTablets(
+    const GURL& url,
+    IntentPickerAutoDisplayPref::Platform platform) {
+  CreateLocalPreference(profile_, url)->UpdatePlatform(platform);
+}

chrome/browser/apps/intent_helper/intent_picker_auto_display_service.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_SERVICE_H_
 
 #include "base/macros.h"
+#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_pref.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "url/gurl.h"
 
@@ -28,6 +29,17 @@ class IntentPickerAutoDisplayService : public KeyedService {
   // Keep track of the |url| repetitions.
   void IncrementCounter(const GURL& url);
 
+  // Returns the last platform selected by the user to handle |url|.
+  // If it has not been checked then it will return |Platform::kNone|
+  // for devices of tablet form factor.
+  IntentPickerAutoDisplayPref::Platform GetLastUsedPlatformForTablets(
+      const GURL& url);
+
+  // Updates the Platform to |platform| for |url| for devices of
+  // tablet form factor.
+  void UpdatePlatformForTablets(const GURL& url,
+                                IntentPickerAutoDisplayPref::Platform platform);
+
  private:
   Profile* profile_;
 

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

@@ -18,6 +18,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chromeos/constants/chromeos_switches.h"
 #include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 #include "components/arc/metrics/arc_metrics_constants.h"
 #include "content/public/browser/browser_context.h"
@@ -66,6 +67,21 @@ void ChromeOsAppsNavigationThrottle::OnIntentPickerClosed(
     apps::PickerEntryType entry_type,
     apps::IntentPickerCloseReason close_reason,
     bool should_persist) {
+  if (chromeos::switches::IsTabletFormFactor() && should_persist) {
+    // On devices of tablet form factor, until the user has decided to persist
+    // the setting, the browser-side intent picker should always be seen.
+    auto platform = IntentPickerAutoDisplayPref::Platform::kNone;
+    if (entry_type == apps::PickerEntryType::kArc) {
+      platform = IntentPickerAutoDisplayPref::Platform::kArc;
+    } else if (entry_type == apps::PickerEntryType::kUnknown &&
+               close_reason == apps::IntentPickerCloseReason::STAY_IN_CHROME) {
+      platform = IntentPickerAutoDisplayPref::Platform::kChrome;
+    }
+    IntentPickerAutoDisplayService::Get(
+        Profile::FromBrowserContext(web_contents->GetBrowserContext()))
+        ->UpdatePlatformForTablets(url, platform);
+  }
+
   const bool should_launch_app =
       close_reason == apps::IntentPickerCloseReason::OPEN_APP;
   switch (entry_type) {
@@ -217,14 +233,17 @@ void ChromeOsAppsNavigationThrottle::CancelNavigation() {
 bool ChromeOsAppsNavigationThrottle::ShouldDeferNavigationForArc(
     content::NavigationHandle* handle) {
   // Query for ARC apps, and if we are handling a link navigation, allow the
-  // preferred app (if it exists) to be launched.
+  // preferred app (if it exists) to be launched unless we are on a device
+  // of tablet form factor, which will only launch the app if the user has
+  // explicitly set that app as preferred and persisted that setting via the
+  // intent picker previously.
   if (arc_enabled_ &&
       arc::ArcIntentPickerAppFetcher::WillGetArcAppsForNavigation(
           handle,
           base::BindOnce(
               &ChromeOsAppsNavigationThrottle::OnDeferredNavigationProcessed,
               weak_factory_.GetWeakPtr()),
-          /*should_launch_preferred_app=*/navigate_from_link())) {
+          ShouldLaunchPreferredApp(handle->GetURL()))) {
     return true;
   }
   return false;
@@ -260,6 +279,14 @@ ChromeOsAppsNavigationThrottle::GetPickerShowState(
     const std::vector<apps::IntentPickerAppInfo>& apps_for_picker,
     content::WebContents* web_contents,
     const GURL& url) {
+  // On devices with tablet form factor we should not pop out the intent
+  // picker if Chrome has been chosen by the user as the platform for this URL.
+  if (chromeos::switches::IsTabletFormFactor()) {
+    if (ui_auto_display_service_->GetLastUsedPlatformForTablets(url) ==
+        IntentPickerAutoDisplayPref::Platform::kChrome) {
+      return PickerShowState::kOmnibox;
+    }
+  }
   return ShouldAutoDisplayUi(apps_for_picker, web_contents, url) &&
                  navigate_from_link()
              ? PickerShowState::kPopOut
@@ -299,4 +326,18 @@ bool ChromeOsAppsNavigationThrottle::ShouldAutoDisplayUi(
   DCHECK(ui_auto_display_service_);
   return ui_auto_display_service_->ShouldAutoDisplayUi(url);
 }
+
+bool ChromeOsAppsNavigationThrottle::ShouldLaunchPreferredApp(const GURL& url) {
+  DCHECK(ui_auto_display_service_);
+  // Devices of tablet form factor should only launch a preferred app
+  // from Chrome if it has been explicitly set and persisted by the user in the
+  // intent picker previously.
+  if (chromeos::switches::IsTabletFormFactor() &&
+      ui_auto_display_service_->GetLastUsedPlatformForTablets(url) !=
+          IntentPickerAutoDisplayPref::Platform::kArc) {
+    return false;
+  }
+  return navigate_from_link();
+}
+
 }  // namespace chromeos

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

@@ -122,6 +122,9 @@ class ChromeOsAppsNavigationThrottle : public apps::AppsNavigationThrottle {
       content::WebContents* web_contents,
       const GURL& url);
 
+  // Whether or not we should launch preferred ARC apps.
+  bool ShouldLaunchPreferredApp(const GURL& url);
+
   // True if ARC is enabled, false otherwise.
   const bool arc_enabled_;