iwa: Guard IWA-KD component by features::kIsolatedWebApps on Windows

These two features will be launched together anyway; this reshuffle aims
to simplify the testing process.

This CL also scopes the kIwaKeyDistributionComponent feature visibility
on about://flags to MacOS and Linux.

Bug: 399048404
Change-Id: I73146e05ee5d7f791242ffbf11d9cb5fc233183c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6298259
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Simon Hangl <simonha@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Andrew Rayskiy <greengrape@google.com>
Cr-Commit-Position: refs/heads/main@{#1426374}

PRESUBMIT.py

@@ -1904,16 +1904,18 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
         explanation=(
             'Do not use `features::kIsolatedWebApps` directly to guard Isolated ',
             'Web App code. ',
-            'Use `content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled()` in ',
-            'the browser process or check the `kEnableIsolatedWebAppsInRenderer` ',
-            'command line flag in the renderer process.',
+            'Use `content::AreIsolatedWebAppsEnabled()` in the browser process '
+            'or check the `kEnableIsolatedWebAppsInRenderer` command line flag '
+            'in the renderer process.',
         ),
         treat_as_error=True,
-        excluded_paths=_TEST_CODE_EXCLUDED_PATHS +
-        ('^chrome/browser/about_flags.cc',
-         '^chrome/browser/web_applications/isolated_web_apps/chrome_content_browser_client_isolated_web_apps_part.cc',
-         '^chrome/browser/ui/startup/bad_flags_prompt.cc',
-         '^content/shell/browser/shell_content_browser_client.cc')),
+        excluded_paths=_TEST_CODE_EXCLUDED_PATHS + (
+            '^chrome/browser/about_flags.cc',
+            '^chrome/browser/component_updater/iwa_key_distribution_component_installer.cc',
+            '^chrome/browser/web_applications/isolated_web_apps/chrome_content_browser_client_isolated_web_apps_part.cc',
+            '^chrome/browser/ui/startup/bad_flags_prompt.cc',
+            '^content/shell/browser/shell_content_browser_client.cc',
+        )),
     BanRule(
         pattern=r'features::kIsolatedWebAppDevMode',
         explanation=(

chrome/browser/about_flags.cc

@@ -5205,11 +5205,13 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kEnableIsolatedWebAppDevModeName,
      flag_descriptions::kEnableIsolatedWebAppDevModeDescription, kOsDesktop,
      FEATURE_VALUE_TYPE(features::kIsolatedWebAppDevMode)},
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
     {"enable-iwa-key-distribution-component",
      flag_descriptions::kEnableIwaKeyDistributionComponentName,
      flag_descriptions::kEnableIwaKeyDistributionComponentDescription,
      kOsDesktop,
      FEATURE_VALUE_TYPE(component_updater::kIwaKeyDistributionComponent)},
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
     {"iwa-key-distribution-component-exp-cohort",
      flag_descriptions::kIwaKeyDistributionComponentExpCohortName,
      flag_descriptions::kIwaKeyDistributionComponentExpCohortDescription,

chrome/browser/component_updater/iwa_key_distribution_component_installer.cc

@@ -29,6 +29,10 @@
 #include "components/crx_file/id_util.h"
 #include "components/update_client/update_client.h"
 
+#if BUILDFLAG(IS_WIN)
+#include "content/public/common/content_features.h"
+#endif  // BUILDFLAG(IS_WIN)
+
 namespace {
 
 // The SHA256 of the SubjectPublicKeyInfo used to sign the extension.
@@ -52,6 +56,7 @@ void OnDemandUpdateCompleted(update_client::Error err) {
 
 namespace component_updater {
 
+#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 BASE_FEATURE(kIwaKeyDistributionComponent,
              "IwaKeyDistributionComponent",
 #if BUILDFLAG(IS_CHROMEOS)
@@ -60,18 +65,34 @@ BASE_FEATURE(kIwaKeyDistributionComponent,
              base::FEATURE_DISABLED_BY_DEFAULT
 #endif  // !BUILDFLAG(IS_CHROMEOS)
 );
+#endif  // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
 IwaKeyDistributionComponentInstallerPolicy::
     IwaKeyDistributionComponentInstallerPolicy() = default;
 IwaKeyDistributionComponentInstallerPolicy::
     ~IwaKeyDistributionComponentInstallerPolicy() = default;
 
+// static
+bool IwaKeyDistributionComponentInstallerPolicy::IsSupported() {
+  // kIwaKeyDistributionComponent feature flag is somewhat useless without
+  // features::kIsolatedWebApps. On ChromeOS, it's kept separately for the time
+  // being as a kill switch and will be retired shortly; on Mac/Linux, the
+  // component logic is not fully supported, so it has to be kept separated from
+  // the main IWA feature.
+#if BUILDFLAG(IS_WIN)
+  return base::FeatureList::IsEnabled(features::kIsolatedWebApps);
+#elif BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+  return base::FeatureList::IsEnabled(kIwaKeyDistributionComponent);
+#else
+  return false;
+#endif
+}
+
 // static
 bool IwaKeyDistributionComponentInstallerPolicy::QueueOnDemandUpdate(
     base::PassKey<web_app::IwaKeyDistributionInfoProvider>) {
   // static
-  if (!g_browser_process ||
-      !base::FeatureList::IsEnabled(kIwaKeyDistributionComponent)) {
+  if (!g_browser_process || !IsSupported()) {
     return false;
   }
 
@@ -155,13 +176,12 @@ IwaKeyDistributionComponentInstallerPolicy::GetInstallerAttributes() const {
 }
 
 void RegisterIwaKeyDistributionComponent(ComponentUpdateService* cus) {
-  if (!base::FeatureList::IsEnabled(kIwaKeyDistributionComponent)) {
+  if (!IwaKeyDistributionComponentInstallerPolicy::IsSupported()) {
     return;
   }
 
   base::MakeRefCounted<ComponentInstaller>(
-      std::make_unique<IwaKeyDistributionComponentInstallerPolicy>(),
-      /*action_handler=*/nullptr, base::TaskPriority::USER_VISIBLE)
+      std::make_unique<IwaKeyDistributionComponentInstallerPolicy>())
       ->Register(cus, base::DoNothing());
 }
 

chrome/browser/component_updater/iwa_key_distribution_component_installer.h

@@ -27,7 +27,9 @@ class IwaKeyDistributionInfoProvider;
 
 namespace component_updater {
 
+#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 BASE_DECLARE_FEATURE(kIwaKeyDistributionComponent);
+#endif  // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
 inline constexpr char kIwaKeyDistributionComponentExpCohort[] =
     "iwa-kd-component-exp-cohort";
@@ -51,6 +53,10 @@ class IwaKeyDistributionComponentInstallerPolicy
   IwaKeyDistributionComponentInstallerPolicy operator=(
       const IwaKeyDistributionComponentInstallerPolicy&) = delete;
 
+  // Tells whether the component is supported on a particular platform wrt to
+  // the feature flags.
+  static bool IsSupported();
+
   // Triggers an on-demand update for the component. Returns whether the update
   // has been queued.
   // This function is supposed to be used by `IwaKeyDistributionInfoProvider`.

chrome/browser/flag_descriptions.cc

@@ -1503,11 +1503,13 @@ const char kEnableIsolatedWebAppDevModeName[] =
 const char kEnableIsolatedWebAppDevModeDescription[] =
     "Enables the installation of unverified Isolated Web Apps";
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 const char kEnableIwaKeyDistributionComponentName[] =
     "Enable the Iwa Key Distribution component";
 const char kEnableIwaKeyDistributionComponentDescription[] =
     "Enables the Iwa Key Distribution component that supplies key rotation "
     "data for Isolated Web Apps.";
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
 const char kIwaKeyDistributionComponentExpCohortName[] =
     "Experimental cohort for the Iwa Key Distribution component";

chrome/browser/flag_descriptions.h

@@ -918,8 +918,10 @@ extern const char kEnableIsolatedWebAppManagedGuestSessionInstallDescription[];
 extern const char kEnableIsolatedWebAppDevModeName[];
 extern const char kEnableIsolatedWebAppDevModeDescription[];
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 extern const char kEnableIwaKeyDistributionComponentName[];
 extern const char kEnableIwaKeyDistributionComponentDescription[];
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
 extern const char kIwaKeyDistributionComponentExpCohortName[];
 extern const char kIwaKeyDistributionComponentExpCohortDescription[];

chrome/browser/web_applications/isolated_web_apps/isolated_web_app_install_prepare_apply_update_browsertest.cc

@@ -128,8 +128,10 @@ class IsolatedWebAppInstallPrepareApplyUpdateCommandBrowserTest
 
   bool is_dev_mode_ = GetParam();
 
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
   base::test::ScopedFeatureList features_{
       component_updater::kIwaKeyDistributionComponent};
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
   // Override the pre-install component directory and its alternative directory
   // so that the component update will not find the pre-installed key dist

chrome/browser/web_applications/isolated_web_apps/isolated_web_app_update_manager_browsertest.cc

@@ -1198,8 +1198,10 @@ class IsolatedWebAppUpdateManagerWithKeyRotationBrowserTest
   }
 
   IsolatedWebAppUpdateServerMixin update_server_mixin_{&mixin_host_};
-  base::test::ScopedFeatureList scoped_feature_list_{
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+  base::test::ScopedFeatureList features_{
       component_updater::kIwaKeyDistributionComponent};
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
   web_package::SignedWebBundleId web_bundle_id_ =
       test::GetDefaultEd25519WebBundleId();

chrome/browser/web_applications/isolated_web_apps/key_distribution/iwa_key_distribution_component_installer_browsertest.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/web_applications/isolated_web_apps/test/key_distribution/test_utils.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/component_updater/component_updater_paths.h"
+#include "content/public/common/content_features.h"
 #include "content/public/test/browser_test.h"
 
 namespace web_app {
@@ -53,8 +54,15 @@ IwaKeyDistribution CreateValidData() {
 class IwaKeyDistributionComponentInstallBrowserTest
     : public InProcessBrowserTest {
  private:
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
   base::test::ScopedFeatureList features_{
       component_updater::kIwaKeyDistributionComponent};
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+
+// TODO(crbug.com/393102554): Remove this after launch.
+#if BUILDFLAG(IS_WIN)
+  base::test::ScopedFeatureList features_{features::kIsolatedWebApps};
+#endif  // BUILDFLAG(IS_WIN)
 };
 
 IN_PROC_BROWSER_TEST_F(

chrome/browser/web_applications/isolated_web_apps/key_distribution/iwa_key_distribution_info_provider.cc

@@ -210,8 +210,8 @@ void IwaKeyDistributionInfoProvider::RotateKeyForDevMode(
 
 base::OneShotEvent&
 IwaKeyDistributionInfoProvider::OnMaybeDownloadedComponentDataReady() {
-  if (!base::FeatureList::IsEnabled(
-          component_updater::kIwaKeyDistributionComponent) ||
+  if (!component_updater::IwaKeyDistributionComponentInstallerPolicy::
+          IsSupported() ||
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableComponentUpdate)) {
     // `switches::kDisableComponentUpdate` is set by default in browsertests.

chrome/browser/web_applications/isolated_web_apps/key_distribution/iwa_key_distribution_info_provider_unittest.cc

@@ -31,6 +31,7 @@
 #include "components/web_package/test_support/signed_web_bundles/signature_verifier_test_utils.h"
 #include "components/web_package/test_support/signed_web_bundles/web_bundle_signer.h"
 #include "components/web_package/web_bundle_builder.h"
+#include "content/public/common/content_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -443,8 +444,15 @@ class IwaIwaKeyDistributionInfoProviderReadinessTest
   testing::NiceMock<MockOnDemandUpdater> on_demand_updater_;
 
   std::unique_ptr<base::ScopedTempDir> dir_;
-  base::test::ScopedFeatureList feature_list_{
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+  base::test::ScopedFeatureList features_{
       component_updater::kIwaKeyDistributionComponent};
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+
+// TODO(crbug.com/393102554): Remove this after launch.
+#if BUILDFLAG(IS_WIN)
+  base::test::ScopedFeatureList features_{features::kIsolatedWebApps};
+#endif  // BUILDFLAG(IS_WIN)
 
   base::ScopedPathOverride user_dir_override_{
       component_updater::DIR_COMPONENT_USER};

chrome/browser/web_applications/isolated_web_apps/test/isolated_web_app_test.cc

@@ -106,7 +106,10 @@ IsolatedWebAppTest::IsolatedWebAppTest(
 #if !BUILDFLAG(IS_CHROMEOS)
       features::kIsolatedWebApps,
 #endif  // !BUILDFLAG(IS_CHROMEOS)
-      component_updater::kIwaKeyDistributionComponent};
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+      component_updater::kIwaKeyDistributionComponent
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
+  };
   if (dev_mode) {
     enabled_features.push_back(features::kIsolatedWebAppDevMode);
   }

chrome/browser/web_applications/isolated_web_apps/test/key_distribution/test_utils.cc

@@ -106,10 +106,12 @@ base::expected<void, IwaComponentUpdateError> UpdateKeyDistributionInfo(
 base::expected<void, IwaComponentUpdateError>
 InstallIwaKeyDistributionComponent(const base::Version& version,
                                    const IwaKeyDistribution& kd_proto) {
+#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
   CHECK(base::FeatureList::IsEnabled(
       component_updater::kIwaKeyDistributionComponent))
       << "The `IwaKeyDistribution` feature must be enabled for the component "
          "installation to succeed.";
+#endif  // BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX)
 
   using Installer =
       component_updater::IwaKeyDistributionComponentInstallerPolicy;