Intents: Clean up default link capturing feature flag

This flag has been enabled for a few milestones and can be cleaned up.
Updates to the link capturing setting default state from ARC will now be
ignored, so that ARC apps default to "Open in Browser".

Bug: 1411121
Change-Id: I3e4757d1610e62bace83bd1a8b00724d8d346fc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4202268
Commit-Queue: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Maggie Cai <mxcai@chromium.org>
Auto-Submit: Tim Sergeant <tsergeant@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098452}

chrome/browser/about_flags.cc

@@ -8396,13 +8396,6 @@ const FeatureEntry kFeatureEntries[] = {
      FEATURE_VALUE_TYPE(ash::features::kDesksTemplates)},
 #endif
 
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-    {"default-link-capturing-in-browser",
-     flag_descriptions::kDefaultLinkCapturingInBrowserName,
-     flag_descriptions::kDefaultLinkCapturingInBrowserDescription, kOsCrOS,
-     FEATURE_VALUE_TYPE(features::kDefaultLinkCapturingInBrowser)},
-#endif
-
     {"large-favicon-from-google",
      flag_descriptions::kLargeFaviconFromGoogleName,
      flag_descriptions::kLargeFaviconFromGoogleDescription, kOsAndroid,

chrome/browser/apps/app_service/publishers/arc_apps.cc

@@ -1288,13 +1288,11 @@ void ArcApps::OnArcSupportedLinksChanged(
       continue;
     }
 
-    // When kDefaultLinkCapturingInBrowser is enabled, ignore any requests from
-    // ARC to set an app as handling supported links by default. We allow
-    // requests if they were initiated by user action, or if the app already
-    // has a non-default setting on the Ash side.
+    // Ignore any requests from the ARC system to set an app as handling
+    // supported links by default. We allow requests if they were initiated by
+    // user action, or if the app already has a non-default setting on the Ash
+    // side.
     bool should_ignore_update =
-        base::FeatureList::GetInstance()->IsEnabled(
-            features::kDefaultLinkCapturingInBrowser) &&
         AppShouldDefaultHandleLinksInBrowser(app_id) &&
         source == arc::mojom::SupportedLinkChangeSource::kArcSystem &&
         !proxy()->PreferredAppsList().IsPreferredAppForSupportedLinks(app_id);

chrome/browser/apps/app_service/publishers/arc_apps_unittest.cc

@@ -14,7 +14,6 @@
 #include "base/functional/callback_helpers.h"
 #include "base/strings/string_util.h"
 #include "base/test/bind.h"
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/apps/app_service/app_service_test.h"
@@ -28,7 +27,6 @@
 #include "chrome/browser/ash/file_manager/path_util.h"
 #include "chrome/browser/web_applications/test/fake_web_app_provider.h"
 #include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
-#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/arc/intent_helper/arc_intent_helper_bridge.h"
 #include "components/arc/intent_helper/intent_constants.h"
@@ -124,8 +122,6 @@ class ArcAppsPublisherTest : public testing::Test {
  public:
   void SetUp() override {
     testing::Test::SetUp();
-    scoped_feature_list_.InitAndDisableFeature(
-        features::kDefaultLinkCapturingInBrowser);
 
     // Do not destroy the ArcServiceManager during TearDown, so that Arc
     // KeyedServices can be correctly destroyed during profile shutdown.
@@ -221,7 +217,6 @@ class ArcAppsPublisherTest : public testing::Test {
 
  private:
   content::BrowserTaskEnvironment task_environment_;
-  base::test::ScopedFeatureList scoped_feature_list_;
   ArcAppTest arc_test_;
   TestingProfile profile_;
   apps::AppServiceTest app_service_test_;
@@ -230,9 +225,9 @@ class ArcAppsPublisherTest : public testing::Test {
   std::unique_ptr<arc::ArcFileSystemBridge> arc_file_system_bridge_;
 };
 
-// Verifies that a call to set the supported links preference from ARC persists
-// the setting in app service.
-TEST_F(ArcAppsPublisherTest, SetSupportedLinksFromArc) {
+// Verifies that a call to set the supported links preference from the ARC
+// system doesn't change the setting in app service.
+TEST_F(ArcAppsPublisherTest, SetSupportedLinksFromArcSystem) {
   constexpr char kTestAuthority[] = "www.example.com";
   const auto& fake_apps = arc_test()->fake_apps();
   std::string package_name = fake_apps[0]->package_name;
@@ -249,8 +244,8 @@ TEST_F(ArcAppsPublisherTest, SetSupportedLinksFromArc) {
       CreateSupportedLinks(package_name), {},
       arc::mojom::SupportedLinkChangeSource::kArcSystem);
 
-  ASSERT_EQ(app_id, preferred_apps().FindPreferredAppForUrl(
-                        GURL("https://www.example.com/foo")));
+  ASSERT_EQ(absl::nullopt, preferred_apps().FindPreferredAppForUrl(
+                               GURL("https://www.example.com/foo")));
 }
 
 // Verifies that a call to set the supported links preference from App Service
@@ -273,36 +268,9 @@ TEST_F(ArcAppsPublisherTest, SetSupportedLinksFromAppService) {
       intent_helper_instance()->verified_links().find(package_name)->second);
 }
 
-// Verifies that when the behavior to open links in the browser by default is
-// enabled, Android apps are not set to handle links during first install.
-TEST_F(ArcAppsPublisherTest, SetSupportedLinksDefaultBrowserBehavior) {
-  base::test::ScopedFeatureList scoped_features(
-      features::kDefaultLinkCapturingInBrowser);
-
-  constexpr char kTestAuthority[] = "www.example.com";
-  const auto& fake_apps = arc_test()->fake_apps();
-  std::string package_name = fake_apps[0]->package_name;
-  std::string app_id = ArcAppListPrefs::GetAppId(fake_apps[0]->package_name,
-                                                 fake_apps[0]->activity);
-  arc_test()->app_instance()->SendRefreshAppList(fake_apps);
-
-  // Update intent filters and supported links for the app, as if it was just
-  // installed.
-  intent_helper()->OnIntentFiltersUpdatedForPackage(
-      package_name, CreateFilterList(package_name, {kTestAuthority}));
-  VerifyIntentFilters(app_id, {kTestAuthority});
-  intent_helper()->OnSupportedLinksChanged(
-      CreateSupportedLinks(package_name), {},
-      arc::mojom::SupportedLinkChangeSource::kArcSystem);
-
-  ASSERT_EQ(absl::nullopt, preferred_apps().FindPreferredAppForUrl(
-                               GURL("https://www.example.com/foo")));
-}
-
-// Verifies that when the behavior to open links in the browser by default is
-// enabled, apps which are already preferred can still update their filters.
-TEST_F(ArcAppsPublisherTest,
-       SetSupportedLinksDefaultBrowserBehaviorAllowsUpdates) {
+// Verifies that the ARC system can still update preferred intent filters for
+// apps which are already preferred.
+TEST_F(ArcAppsPublisherTest, SetSupportedLinksAllowsUpdates) {
   constexpr char kTestAuthority[] = "www.example.com";
   constexpr char kTestAuthority2[] = "www.newexample.com";
   const auto& fake_apps = arc_test()->fake_apps();
@@ -316,12 +284,10 @@ TEST_F(ArcAppsPublisherTest,
   intent_helper()->OnIntentFiltersUpdatedForPackage(
       package_name, CreateFilterList(package_name, {kTestAuthority}));
   VerifyIntentFilters(app_id, {kTestAuthority});
-  intent_helper()->OnSupportedLinksChanged(
-      CreateSupportedLinks(package_name), {},
-      arc::mojom::SupportedLinkChangeSource::kArcSystem);
 
-  base::test::ScopedFeatureList scoped_features(
-      features::kDefaultLinkCapturingInBrowser);
+  // Set a user preference for the app.
+  apps::AppServiceProxyFactory::GetForProfile(profile())
+      ->SetSupportedLinksPreference(app_id);
 
   // Update filters with a new authority added.
   intent_helper()->OnIntentFiltersUpdatedForPackage(
@@ -332,17 +298,13 @@ TEST_F(ArcAppsPublisherTest,
       CreateSupportedLinks(package_name), {},
       arc::mojom::SupportedLinkChangeSource::kArcSystem);
 
+  // Verify that the user preference has been extended to the new filter.
   ASSERT_EQ(app_id, preferred_apps().FindPreferredAppForUrl(
                         GURL("https://www.newexample.com/foo")));
 }
 
-// Verifies that when the behavior to open links in the browser by default is
-// enabled, the user can still set an app as preferred through ARC settings.
-TEST_F(ArcAppsPublisherTest,
-       SetSupportedLinksDefaultBrowserBehaviorAllowsUserChanges) {
-  base::test::ScopedFeatureList scoped_features(
-      features::kDefaultLinkCapturingInBrowser);
-
+// Verifies that the user can set an app as preferred through ARC settings.
+TEST_F(ArcAppsPublisherTest, SetSupportedLinksAllowsUserChanges) {
   constexpr char kTestAuthority[] = "www.example.com";
   const auto& fake_apps = arc_test()->fake_apps();
   std::string package_name = fake_apps[0]->package_name;
@@ -362,13 +324,8 @@ TEST_F(ArcAppsPublisherTest,
                         GURL("https://www.example.com/foo")));
 }
 
-// Verifies that when the behavior to open links in the browser by default is
-// enabled, the Play Store app can still be set as preferred by the system.
-TEST_F(ArcAppsPublisherTest,
-       SetSupportedLinksDefaultBrowserBehaviorAllowsPlayStore) {
-  base::test::ScopedFeatureList scoped_features(
-      features::kDefaultLinkCapturingInBrowser);
-
+// Verifies that the Play Store app can be set as preferred by the system.
+TEST_F(ArcAppsPublisherTest, SetSupportedLinksAllowsPlayStoreDefault) {
   constexpr char kTestAuthority[] = "play.google.com";
 
   std::vector<arc::mojom::AppInfoPtr> apps;

chrome/browser/flag-metadata.json

@@ -1332,11 +1332,6 @@
     "owners": ["vkovalova"],
     "expiry_milestone": 104
   },
-  {
-    "name": "default-link-capturing-in-browser",
-    "owners": [ "tsergeant", "chromeos-apps-foundation-team@google.com"],
-    "expiry_milestone": 112
-  },
   {
     "name": "deferred-font-shaping",
     "owners": [ "tkent", "layout-dev" ],

chrome/browser/flag_descriptions.cc

@@ -4882,12 +4882,6 @@ const char kCalendarModelDebugModeDescription[] =
     "the system logs, where they are potentially visible to all users of the "
     "device.";
 
-const char kDefaultLinkCapturingInBrowserName[] =
-    "Default link capturing in the browser";
-const char kDefaultLinkCapturingInBrowserDescription[] =
-    "When enabled, newly installed apps will not capture links clicked in the "
-    "browser.";
-
 extern const char kDesks16Name[] = "Enable up to 16 virtual desks";
 extern const char kDesks16Description[] =
     "When enabled, up to 16 virtual desks are allowed.";

chrome/browser/flag_descriptions.h

@@ -2797,9 +2797,6 @@ extern const char kCalendarViewDescription[];
 extern const char kCalendarModelDebugModeName[];
 extern const char kCalendarModelDebugModeDescription[];
 
-extern const char kDefaultLinkCapturingInBrowserName[];
-extern const char kDefaultLinkCapturingInBrowserDescription[];
-
 extern const char kCaptureModeDemoToolsName[];
 extern const char kCaptureModeDemoToolsDescription[];
 

chrome/common/chrome_features.cc

@@ -203,15 +203,6 @@ BASE_FEATURE(kDataLeakPreventionFilesRestriction,
              base::FEATURE_ENABLED_BY_DEFAULT);
 #endif
 
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-// When enabled, newly installed ARC apps will not capture links clicked in the
-// browser by default. Users can still enable link capturing for apps through
-// the intent picker or settings.
-BASE_FEATURE(kDefaultLinkCapturingInBrowser,
-             "DefaultLinkCapturingInBrowser",
-             base::FEATURE_ENABLED_BY_DEFAULT);
-#endif
-
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 // Enables passing additional user authentication in requests to DMServer
 // (policy fetch, status report upload).

chrome/common/chrome_features.h

@@ -119,11 +119,6 @@ COMPONENT_EXPORT(CHROME_FEATURES)
 BASE_DECLARE_FEATURE(kDataLeakPreventionFilesRestriction);
 #endif
 
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-COMPONENT_EXPORT(CHROME_FEATURES)
-BASE_DECLARE_FEATURE(kDefaultLinkCapturingInBrowser);
-#endif
-
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 COMPONENT_EXPORT(CHROME_FEATURES)
 BASE_DECLARE_FEATURE(kDMServerOAuthForChildUser);