Added SystemDisabledMode policy for CWS

Added support for SystemDisabledMode policy for the Chrome Web Store, so that CWS can now be hidden if that is the preferred disabled mode.

Bug: 1142785
Test: SystemFeaturesPolicyTest.DisableWebStoreAfterInstallWithModes
Change-Id: I5cb731f6d071413f06563b19ea6ad365cb44c053
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622375
Commit-Queue: Sanja Perisic <sanjaperisic@chromium.org>
Reviewed-by: Anqing Zhao <anqing@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844039}

chrome/browser/apps/app_service/extension_apps_chromeos.cc

@@ -173,6 +173,10 @@ void ExtensionAppsChromeOs::Initialize() {
         policy::policy_prefs::kSystemFeaturesDisableList,
         base::BindRepeating(&ExtensionAppsBase::OnSystemFeaturesPrefChanged,
                             GetWeakPtr()));
+    local_state_pref_change_registrar_.Add(
+        policy::policy_prefs::kSystemFeaturesDisableMode,
+        base::BindRepeating(&ExtensionAppsBase::OnSystemFeaturesPrefChanged,
+                            GetWeakPtr()));
     OnSystemFeaturesPrefChanged();
   }
 }
@@ -504,13 +508,21 @@ void ExtensionAppsChromeOs::OnSystemFeaturesPrefChanged() {
     return;
   }
 
-  UpdateAppDisabledState(disabled_system_features_pref,
-                         policy::SystemFeature::kCamera,
-                         extension_misc::kCameraAppId);
+  const bool is_pref_disabled_mode_hidden =
+      local_state->GetString(
+          policy::policy_prefs::kSystemFeaturesDisableMode) ==
+      policy::kHiddenDisableMode;
+  const bool is_disabled_mode_changed =
+      (is_pref_disabled_mode_hidden != is_disabled_apps_mode_hidden_);
+  is_disabled_apps_mode_hidden_ = is_pref_disabled_mode_hidden;
+
+  UpdateAppDisabledState(
+      disabled_system_features_pref, policy::SystemFeature::kCamera,
+      extension_misc::kCameraAppId, is_disabled_mode_changed);
 
   UpdateAppDisabledState(disabled_system_features_pref,
                          policy::SystemFeature::kWebStore,
-                         extensions::kWebStoreAppId);
+                         extensions::kWebStoreAppId, is_disabled_mode_changed);
 }
 
 bool ExtensionAppsChromeOs::Accepts(const extensions::Extension* extension) {
@@ -529,10 +541,10 @@ bool ExtensionAppsChromeOs::ShouldShownInLauncher(
 apps::mojom::AppPtr ExtensionAppsChromeOs::Convert(
     const extensions::Extension* extension,
     apps::mojom::Readiness readiness) {
-  apps::mojom::AppPtr app =
-      ConvertImpl(extension, base::Contains(disabled_apps_, extension->id())
-                                 ? apps::mojom::Readiness::kDisabledByPolicy
-                                 : readiness);
+  const bool is_app_disabled = base::Contains(disabled_apps_, extension->id());
+  apps::mojom::AppPtr app = ConvertImpl(
+      extension,
+      is_app_disabled ? apps::mojom::Readiness::kDisabledByPolicy : readiness);
   bool paused = paused_apps_.IsPaused(extension->id());
   app->icon_key =
       icon_key_factory().MakeIconKey(GetIconEffects(extension, paused));
@@ -543,6 +555,12 @@ apps::mojom::AppPtr ExtensionAppsChromeOs::Convert(
   app->paused = paused ? apps::mojom::OptionalBool::kTrue
                        : apps::mojom::OptionalBool::kFalse;
 
+  if (is_app_disabled && is_disabled_apps_mode_hidden_) {
+    app->show_in_launcher = apps::mojom::OptionalBool::kFalse;
+    app->show_in_search = apps::mojom::OptionalBool::kFalse;
+    app->show_in_shelf = apps::mojom::OptionalBool::kFalse;
+  }
+
   return app;
 }
 
@@ -712,7 +730,8 @@ content::WebContents* ExtensionAppsChromeOs::LaunchImpl(
 void ExtensionAppsChromeOs::UpdateAppDisabledState(
     const base::ListValue* disabled_system_features_pref,
     int feature,
-    const std::string& app_id) {
+    const std::string& app_id,
+    bool is_disabled_mode_changed) {
   const bool is_disabled =
       base::Contains(*disabled_system_features_pref, base::Value(feature));
   // Sometimes the policy is updated before the app is installed, so this way
@@ -720,7 +739,8 @@ void ExtensionAppsChromeOs::UpdateAppDisabledState(
   // and the app will be published with the correct readiness upon its
   // installation.
   const bool should_publish =
-      (base::Contains(disabled_apps_, app_id) != is_disabled);
+      (base::Contains(disabled_apps_, app_id) != is_disabled) ||
+      is_disabled_mode_changed;
 
   if (is_disabled) {
     disabled_apps_.insert(app_id);

chrome/browser/apps/app_service/extension_apps_chromeos.h

@@ -153,7 +153,8 @@ class ExtensionAppsChromeOs : public ExtensionAppsBase,
   void UpdateAppDisabledState(
       const base::ListValue* disabled_system_features_pref,
       int feature,
-      const std::string& app_id);
+      const std::string& app_id,
+      bool is_disabled_mode_changed);
 
   apps::InstanceRegistry* instance_registry_;
   ScopedObserver<extensions::AppWindowRegistry,
@@ -164,6 +165,11 @@ class ExtensionAppsChromeOs : public ExtensionAppsBase,
 
   std::set<std::string> disabled_apps_;
 
+  // Boolean signifying whether the preferred user experience mode of disabled
+  // apps is hidden (true) or blocked (false). The value comes from user pref
+  // and is set by updating SystemDisabledMode policy.
+  bool is_disabled_apps_mode_hidden_ = false;
+
   std::map<extensions::AppWindow*, aura::Window*> app_window_to_aura_window_;
 
   ArcAppListPrefs* arc_prefs_ = nullptr;

chrome/browser/policy/system_features_policy_browsertest.cc

@@ -18,6 +18,7 @@
 #include "chromeos/constants/chromeos_features.h"
 #include "components/policy/core/common/policy_pref_names.h"
 #include "components/policy/policy_constants.h"
+#include "components/services/app_service/public/mojom/types.mojom.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_navigation_observer.h"
@@ -55,18 +56,25 @@ class SystemFeaturesPolicyTest : public PolicyTest {
   }
 
   // Disables specified system features or enables all if system_features is
-  // empty.
-  void UpdateSystemFeaturesDisableList(base::Value system_features) {
+  // empty. Updates disabled mode for disabled system features.
+  void UpdateSystemFeaturesDisableList(base::Value system_features,
+                                       const char* disabled_mode) {
     PolicyMap policies;
     policies.Set(key::kSystemFeaturesDisableList, POLICY_LEVEL_MANDATORY,
                  POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
                  std::move(system_features), nullptr);
+    if (disabled_mode) {
+      policies.Set(key::kSystemFeaturesDisableMode, POLICY_LEVEL_MANDATORY,
+                   POLICY_SCOPE_USER, POLICY_SOURCE_CLOUD,
+                   base::Value(disabled_mode), nullptr);
+    }
     UpdateProviderPolicy(policies);
   }
 
   void VerifyAppState(const char* app_id,
                       apps::mojom::Readiness expected_readiness,
-                      bool blocked_icon) {
+                      bool blocked_icon,
+                      apps::mojom::OptionalBool expected_visibility) {
     auto* profile = browser()->profile();
     extensions::ExtensionRegistry* registry =
         extensions::ExtensionRegistry::Get(profile);
@@ -77,8 +85,8 @@ class SystemFeaturesPolicyTest : public PolicyTest {
     proxy->FlushMojoCallsForTesting();
 
     proxy->AppRegistryCache().ForOneApp(
-        app_id,
-        [&expected_readiness, &blocked_icon](const apps::AppUpdate& update) {
+        app_id, [&expected_readiness, &blocked_icon,
+                 &expected_visibility](const apps::AppUpdate& update) {
           EXPECT_EQ(expected_readiness, update.Readiness());
           if (blocked_icon) {
             EXPECT_TRUE(apps::IconEffects::kBlocked &
@@ -87,6 +95,9 @@ class SystemFeaturesPolicyTest : public PolicyTest {
             EXPECT_FALSE(apps::IconEffects::kBlocked &
                          update.IconKey()->icon_effects);
           }
+          EXPECT_EQ(expected_visibility, update.ShowInLauncher());
+          EXPECT_EQ(expected_visibility, update.ShowInSearch());
+          EXPECT_EQ(expected_visibility, update.ShowInShelf());
         });
   }
 
@@ -97,55 +108,85 @@ class SystemFeaturesPolicyTest : public PolicyTest {
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest, DisableCameraBeforeInstall) {
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kCameraFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
   EnableExtensions(false);
   VerifyAppState(extension_misc::kCameraAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
 
-  UpdateSystemFeaturesDisableList(base::Value());
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   VerifyAppState(extension_misc::kCameraAppId, apps::mojom::Readiness::kReady,
-                 false);
+                 false, apps::mojom::OptionalBool::kTrue);
 }
 
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest, DisableCameraAfterInstall) {
   EnableExtensions(false);
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kCameraFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
 
   VerifyAppState(extension_misc::kCameraAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
 
-  UpdateSystemFeaturesDisableList(base::Value());
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   VerifyAppState(extension_misc::kCameraAppId, apps::mojom::Readiness::kReady,
-                 false);
+                 false, apps::mojom::OptionalBool::kTrue);
 }
 
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest, DisableWebStoreBeforeInstall) {
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kWebStoreFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
   EnableExtensions(true);
   VerifyAppState(extensions::kWebStoreAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
 
-  UpdateSystemFeaturesDisableList(base::Value());
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   VerifyAppState(extensions::kWebStoreAppId, apps::mojom::Readiness::kReady,
-                 false);
+                 false, apps::mojom::OptionalBool::kTrue);
 }
 
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest, DisableWebStoreAfterInstall) {
   EnableExtensions(false);
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kWebStoreFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
 
   VerifyAppState(extensions::kWebStoreAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
+
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
+  VerifyAppState(extensions::kWebStoreAppId, apps::mojom::Readiness::kReady,
+                 false, apps::mojom::OptionalBool::kTrue);
+}
 
-  UpdateSystemFeaturesDisableList(base::Value());
+IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest,
+                       DisableWebStoreAfterInstallWithModes) {
+  EnableExtensions(false);
+  base::Value system_features(base::Value::Type::LIST);
+  system_features.Append(kWebStoreFeature);
+  // Disable app with default mode (blocked)..
+  UpdateSystemFeaturesDisableList(system_features.Clone(), nullptr);
+  VerifyAppState(extensions::kWebStoreAppId,
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
+  // Disable and hide app.
+  UpdateSystemFeaturesDisableList(system_features.Clone(), kHiddenDisableMode);
+  VerifyAppState(extensions::kWebStoreAppId,
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kFalse);
+  // Disable and block app.
+  UpdateSystemFeaturesDisableList(system_features.Clone(), kBlockedDisableMode);
+  VerifyAppState(extensions::kWebStoreAppId,
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
+  // Enable app
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   VerifyAppState(extensions::kWebStoreAppId, apps::mojom::Readiness::kReady,
-                 false);
+                 false, apps::mojom::OptionalBool::kTrue);
 }
 
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest,
@@ -154,29 +195,31 @@ IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest,
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kWebStoreFeature);
   system_features.Append(kCameraFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
 
   VerifyAppState(extensions::kWebStoreAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
   VerifyAppState(extension_misc::kCameraAppId,
-                 apps::mojom::Readiness::kDisabledByPolicy, true);
+                 apps::mojom::Readiness::kDisabledByPolicy, true,
+                 apps::mojom::OptionalBool::kTrue);
 
-  UpdateSystemFeaturesDisableList(base::Value());
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   VerifyAppState(extensions::kWebStoreAppId, apps::mojom::Readiness::kReady,
-                 false);
+                 false, apps::mojom::OptionalBool::kTrue);
 }
 
 IN_PROC_BROWSER_TEST_F(SystemFeaturesPolicyTest, RedirectChromeSettingsURL) {
   PolicyMap policies;
   base::Value system_features(base::Value::Type::LIST);
   system_features.Append(kBrowserSettingsFeature);
-  UpdateSystemFeaturesDisableList(std::move(system_features));
+  UpdateSystemFeaturesDisableList(std::move(system_features), nullptr);
 
   GURL settings_url = GURL(chrome::kChromeUISettingsURL);
   EXPECT_EQ(l10n_util::GetStringUTF16(IDS_CHROME_URLS_DISABLED_PAGE_HEADER),
             GetWebUITitle(settings_url));
 
-  UpdateSystemFeaturesDisableList(base::Value());
+  UpdateSystemFeaturesDisableList(base::Value(), nullptr);
   EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SETTINGS_SETTINGS),
             GetWebUITitle(settings_url));
 }