Intents: Simplify implementation of Intent Picker Auto Display prefs

IntentPickerAutoDisplayService is currently implemented as a
KeyedService, but there's no particular need for this. The service
doesn't store any data, and is cheap to create, so instead we can just
implement it as a collection of static methods.

Bug: 1280507
Change-Id: I8c0bbd8b403615a84b7e386dbdca603b66309526
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3565506
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Auto-Submit: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988509}

chrome/browser/BUILD.gn

@@ -3615,10 +3615,8 @@ static_library("browser") {
       "apps/intent_helper/apps_navigation_throttle.h",
       "apps/intent_helper/apps_navigation_types.cc",
       "apps/intent_helper/apps_navigation_types.h",
-      "apps/intent_helper/intent_picker_auto_display_service.cc",
-      "apps/intent_helper/intent_picker_auto_display_service.h",
-      "apps/intent_helper/intent_picker_auto_display_service_factory.cc",
-      "apps/intent_helper/intent_picker_auto_display_service_factory.h",
+      "apps/intent_helper/intent_picker_auto_display_prefs.cc",
+      "apps/intent_helper/intent_picker_auto_display_prefs.h",
       "apps/intent_helper/intent_picker_constants.h",
       "apps/intent_helper/intent_picker_features.cc",
       "apps/intent_helper/intent_picker_features.h",

chrome/browser/apps/intent_helper/chromeos_intent_picker_helpers.cc

@@ -14,7 +14,7 @@
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/apps/app_service/launch_utils.h"
 #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
-#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_service.h"
+#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_prefs.h"
 #include "chrome/browser/apps/intent_helper/intent_picker_features.h"
 #include "chrome/browser/apps/intent_helper/intent_picker_internal.h"
 #include "chrome/browser/apps/intent_helper/metrics/intent_handling_metrics.h"
@@ -62,26 +62,22 @@ bool ShouldAutoDisplayUi(
   if (!ShouldOverrideUrlLoading(GetStartingGURL(navigation_handle), url))
     return false;
 
-  IntentPickerAutoDisplayService* ui_auto_display_service =
-      IntentPickerAutoDisplayService::Get(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
 
   // 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.
   // TODO(crbug.com/1225828): Handle this for lacros-chrome as well.
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (ash::switches::IsTabletFormFactor()) {
-    if (ui_auto_display_service->GetLastUsedPlatformForTablets(url) ==
-        IntentPickerAutoDisplayService::Platform::kChrome) {
+    if (IntentPickerAutoDisplayPrefs::GetLastUsedPlatformForTablets(
+            profile, url) == IntentPickerAutoDisplayPrefs::Platform::kChrome) {
       return false;
     }
   }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
   // If the preferred app is use browser, do not show the intent picker.
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-
   auto* proxy = AppServiceProxyFactory::GetForProfile(profile);
 
   if (proxy) {
@@ -93,7 +89,7 @@ bool ShouldAutoDisplayUi(
     }
   }
 
-  return ui_auto_display_service->ShouldAutoDisplayUi(url);
+  return IntentPickerAutoDisplayPrefs::ShouldAutoDisplayUi(profile, url);
 }
 
 PickerShowState GetPickerShowState(
@@ -106,15 +102,14 @@ PickerShowState GetPickerShowState(
 }
 
 void OnAppIconsLoaded(content::WebContents* web_contents,
-                      IntentPickerAutoDisplayService* ui_auto_display_service,
                       const GURL& url,
                       std::vector<IntentPickerAppInfo> apps) {
   ShowIntentPickerBubbleForApps(
       web_contents, std::move(apps),
       /*show_stay_in_chrome=*/true,
       /*show_remember_selection=*/true,
       base::BindOnce(&OnIntentPickerClosedChromeOs, web_contents,
-                     ui_auto_display_service, PickerShowState::kPopOut, url));
+                     PickerShowState::kPopOut, url));
 }
 
 }  // namespace
@@ -130,59 +125,51 @@ void MaybeShowIntentPickerBubble(content::NavigationHandle* navigation_handle,
       IntentHandlingMetrics::IntentPickerIconEvent::kAutoPopOut);
 
   content::WebContents* web_contents = navigation_handle->GetWebContents();
-  IntentPickerAutoDisplayService* ui_auto_display_service =
-      IntentPickerAutoDisplayService::Get(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()));
   const GURL& url = navigation_handle->GetURL();
 
   IntentPickerTabHelper::LoadAppIcons(
       web_contents, std::move(apps),
-      base::BindOnce(&OnAppIconsLoaded, web_contents, ui_auto_display_service,
-                     url));
+      base::BindOnce(&OnAppIconsLoaded, web_contents, url));
 }
 
-void OnIntentPickerClosedChromeOs(
-    content::WebContents* web_contents,
-    IntentPickerAutoDisplayService* ui_auto_display_service,
-    PickerShowState show_state,
-    const GURL& url,
-    const std::string& launch_name,
-    PickerEntryType entry_type,
-    IntentPickerCloseReason close_reason,
-    bool should_persist) {
+void OnIntentPickerClosedChromeOs(content::WebContents* web_contents,
+                                  PickerShowState show_state,
+                                  const GURL& url,
+                                  const std::string& launch_name,
+                                  PickerEntryType entry_type,
+                                  IntentPickerCloseReason close_reason,
+                                  bool should_persist) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
 // TODO(crbug.com/1225828): Handle this for lacros-chrome as well.
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (ash::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 = IntentPickerAutoDisplayService::Platform::kNone;
+    auto platform = IntentPickerAutoDisplayPrefs::Platform::kNone;
     if (entry_type == PickerEntryType::kArc) {
-      platform = IntentPickerAutoDisplayService::Platform::kArc;
+      platform = IntentPickerAutoDisplayPrefs::Platform::kArc;
     } else if (entry_type == PickerEntryType::kUnknown &&
                close_reason == IntentPickerCloseReason::STAY_IN_CHROME) {
-      platform = IntentPickerAutoDisplayService::Platform::kChrome;
+      platform = IntentPickerAutoDisplayPrefs::Platform::kChrome;
     }
-    IntentPickerAutoDisplayService::Get(
-        Profile::FromBrowserContext(web_contents->GetBrowserContext()))
-        ->UpdatePlatformForTablets(url, platform);
+    IntentPickerAutoDisplayPrefs::UpdatePlatformForTablets(profile, url,
+                                                           platform);
   }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
   const bool should_launch_app =
       close_reason == IntentPickerCloseReason::OPEN_APP;
 
-  Profile* profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-
   auto* proxy = AppServiceProxyFactory::GetForProfile(profile);
 
   // If the picker was closed without an app being chosen,
   // e.g. due to the tab being closed. Keep count of this scenario so we can
   // stop the UI from showing after 2+ dismissals.
   if (entry_type == PickerEntryType::kUnknown &&
       close_reason == IntentPickerCloseReason::DIALOG_DEACTIVATED &&
-      ui_auto_display_service) {
-    ui_auto_display_service->IncrementPickerUICounter(url);
+      show_state == PickerShowState::kPopOut) {
+    IntentPickerAutoDisplayPrefs::IncrementPickerUICounter(profile, url);
   }
 
   if (should_persist) {
@@ -208,7 +195,7 @@ void LaunchAppFromIntentPickerChromeOs(content::WebContents* web_contents,
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
 
   if (base::FeatureList::IsEnabled(features::kLinkCapturingUiUpdate)) {
-    IntentPickerAutoDisplayService::Get(profile)->ResetIntentChipCounter(url);
+    IntentPickerAutoDisplayPrefs::ResetIntentChipCounter(profile, url);
   }
 
   auto* proxy = AppServiceProxyFactory::GetForProfile(profile);

chrome/browser/apps/intent_helper/chromeos_intent_picker_helpers.h

@@ -10,8 +10,6 @@
 #include "chrome/browser/apps/intent_helper/apps_navigation_types.h"
 #include "url/gurl.h"
 
-class IntentPickerAutoDisplayService;
-
 namespace content {
 class NavigationHandle;
 class WebContents;
@@ -29,15 +27,13 @@ enum class PickerShowState {
   kPopOut = 2,   // show the intent picker icon and pop out bubble
 };
 
-void OnIntentPickerClosedChromeOs(
-    content::WebContents* web_contents,
-    IntentPickerAutoDisplayService* ui_auto_display_service,
-    PickerShowState show_state,
-    const GURL& url,
-    const std::string& launch_name,
-    PickerEntryType entry_type,
-    IntentPickerCloseReason close_reason,
-    bool should_persist);
+void OnIntentPickerClosedChromeOs(content::WebContents* web_contents,
+                                  PickerShowState show_state,
+                                  const GURL& url,
+                                  const std::string& launch_name,
+                                  PickerEntryType entry_type,
+                                  IntentPickerCloseReason close_reason,
+                                  bool should_persist);
 
 void LaunchAppFromIntentPickerChromeOs(content::WebContents* web_contents,
                                        const GURL& url,

chrome/browser/apps/intent_helper/intent_picker_auto_display_service.cc → chrome/browser/apps/intent_helper/intent_picker_auto_display_prefs.cc

@@ -2,10 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_service.h"
+#include "chrome/browser/apps/intent_helper/intent_picker_auto_display_prefs.h"
 
 #include "base/values.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"
 #include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ -44,23 +43,18 @@ base::Value GetAutoDisplayDictForSettings(
 }  // namespace
 
 // static
-IntentPickerAutoDisplayService* IntentPickerAutoDisplayService::Get(
-    Profile* profile) {
-  return IntentPickerAutoDisplayServiceFactory::GetForProfile(profile);
-}
-
-IntentPickerAutoDisplayService::IntentPickerAutoDisplayService(Profile* profile)
-    : profile_(profile) {}
-
-bool IntentPickerAutoDisplayService::ShouldAutoDisplayUi(const GURL& url) {
+bool IntentPickerAutoDisplayPrefs::ShouldAutoDisplayUi(Profile* profile,
+                                                       const GURL& url) {
   base::Value pref_dict = GetAutoDisplayDictForSettings(
-      HostContentSettingsMapFactory::GetForProfile(profile_), url);
+      HostContentSettingsMapFactory::GetForProfile(profile), url);
 
   return pref_dict.FindIntKey(kAutoDisplayKey).value_or(0) < kDismissThreshold;
 }
 
-void IntentPickerAutoDisplayService::IncrementPickerUICounter(const GURL& url) {
-  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile_);
+// static
+void IntentPickerAutoDisplayPrefs::IncrementPickerUICounter(Profile* profile,
+                                                            const GURL& url) {
+  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile);
   base::Value pref_dict = GetAutoDisplayDictForSettings(settings_map, url);
 
   int dismissed_count = pref_dict.FindIntKey(kAutoDisplayKey).value_or(0);
@@ -71,10 +65,11 @@ void IntentPickerAutoDisplayService::IncrementPickerUICounter(const GURL& url) {
       std::move(pref_dict));
 }
 
-IntentPickerAutoDisplayService::ChipState
-IntentPickerAutoDisplayService::GetChipStateAndIncrementCounter(
-    const GURL& url) {
-  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile_);
+// static
+IntentPickerAutoDisplayPrefs::ChipState
+IntentPickerAutoDisplayPrefs::GetChipStateAndIncrementCounter(Profile* profile,
+                                                              const GURL& url) {
+  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile);
   base::Value pref_dict = GetAutoDisplayDictForSettings(settings_map, url);
 
   int display_count = pref_dict.FindIntKey(kIntentChipCountKey).value_or(0);
@@ -91,8 +86,10 @@ IntentPickerAutoDisplayService::GetChipStateAndIncrementCounter(
   return ChipState::kExpanded;
 }
 
-void IntentPickerAutoDisplayService::ResetIntentChipCounter(const GURL& url) {
-  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile_);
+// static
+void IntentPickerAutoDisplayPrefs::ResetIntentChipCounter(Profile* profile,
+                                                          const GURL& url) {
+  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile);
   base::Value pref_dict = GetAutoDisplayDictForSettings(settings_map, url);
 
   pref_dict.SetIntKey(kIntentChipCountKey, 0);
@@ -102,10 +99,12 @@ void IntentPickerAutoDisplayService::ResetIntentChipCounter(const GURL& url) {
       std::move(pref_dict));
 }
 
-IntentPickerAutoDisplayService::Platform
-IntentPickerAutoDisplayService::GetLastUsedPlatformForTablets(const GURL& url) {
+// static
+IntentPickerAutoDisplayPrefs::Platform
+IntentPickerAutoDisplayPrefs::GetLastUsedPlatformForTablets(Profile* profile,
+                                                            const GURL& url) {
   base::Value pref_dict = GetAutoDisplayDictForSettings(
-      HostContentSettingsMapFactory::GetForProfile(profile_), url);
+      HostContentSettingsMapFactory::GetForProfile(profile), url);
   int platform = pref_dict.FindIntKey(kPlatformKey).value_or(0);
 
   DCHECK_GE(platform, static_cast<int>(Platform::kNone));
@@ -114,10 +113,11 @@ IntentPickerAutoDisplayService::GetLastUsedPlatformForTablets(const GURL& url) {
   return static_cast<Platform>(platform);
 }
 
-void IntentPickerAutoDisplayService::UpdatePlatformForTablets(
-    const GURL& url,
-    Platform platform) {
-  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile_);
+// static
+void IntentPickerAutoDisplayPrefs::UpdatePlatformForTablets(Profile* profile,
+                                                            const GURL& url,
+                                                            Platform platform) {
+  auto* settings_map = HostContentSettingsMapFactory::GetForProfile(profile);
   base::Value pref_dict = GetAutoDisplayDictForSettings(settings_map, url);
 
   DCHECK_GE(static_cast<int>(platform), static_cast<int>(Platform::kNone));

chrome/browser/apps/intent_helper/intent_picker_auto_display_service.h → chrome/browser/apps/intent_helper/intent_picker_auto_display_prefs.h

@@ -2,18 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_SERVICE_H_
-#define CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_SERVICE_H_
+#ifndef CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_PREFS_H_
+#define CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_PREFS_H_
 
-#include "base/memory/raw_ptr.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "url/gurl.h"
 
 class Profile;
 
 // Stores and manages user preferences about whether Intent Picker UI should be
 // automatically displayed for each origin.
-class IntentPickerAutoDisplayService : public KeyedService {
+class IntentPickerAutoDisplayPrefs {
  public:
   // The platform selected by the user to handle this URL for devices of tablet
   // form factor.
@@ -23,42 +21,35 @@ class IntentPickerAutoDisplayService : public KeyedService {
   // text) or collapsed (just an icon).
   enum class ChipState { kExpanded = 0, kCollapsed = 1 };
 
-  static IntentPickerAutoDisplayService* Get(Profile* profile);
-
-  explicit IntentPickerAutoDisplayService(Profile* profile);
-  IntentPickerAutoDisplayService(const IntentPickerAutoDisplayService&) =
-      delete;
-  IntentPickerAutoDisplayService& operator=(
-      const IntentPickerAutoDisplayService&) = delete;
-
   // Returns whether or not a likely |url| has triggered the UI 2+ times without
   // the user engaging.
-  bool ShouldAutoDisplayUi(const GURL& url);
+  static bool ShouldAutoDisplayUi(Profile* profile, const GURL& url);
 
   // Keep track of the |url| repetitions.
-  void IncrementPickerUICounter(const GURL& url);
+  static void IncrementPickerUICounter(Profile* profile, const GURL& url);
 
   // Returns a ChipState indicating whether the Intent Chip should be shown as
   // expanded or collapsed for a given URL. Increments an internal counter to
   // track the number of times the chip has been shown for that URL.
-  ChipState GetChipStateAndIncrementCounter(const GURL& url);
+  static ChipState GetChipStateAndIncrementCounter(Profile* profile,
+                                                   const GURL& url);
 
   // Reset the intent chip counter to 0. When this is called, it allows the
   // GetChipStateAndIncrementCounter function will return an Expanded ChipState
   // another 3 times for that |url|.
-  void ResetIntentChipCounter(const GURL& url);
+  static void ResetIntentChipCounter(Profile* profile, 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.
-  Platform GetLastUsedPlatformForTablets(const GURL& url);
+  static Platform GetLastUsedPlatformForTablets(Profile* profile,
+                                                const GURL& url);
 
   // Updates the Platform to |platform| for |url| for devices of
   // tablet form factor.
-  void UpdatePlatformForTablets(const GURL& url, Platform platform);
-
- private:
-  raw_ptr<Profile> profile_;
+  static void UpdatePlatformForTablets(Profile* profile,
+                                       const GURL& url,
+                                       Platform platform);
 };
 
-#endif  // CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_SERVICE_H_
+#endif  // CHROME_BROWSER_APPS_INTENT_HELPER_INTENT_PICKER_AUTO_DISPLAY_PREFS_H_