cros: Remove Chrome web store from NTP in demo mode

There're multiple places that check kHideWebStoreIcon. [1] is the place
we're interested in.

We should change the pref value right after profile is available,
otherwise its value may already be used. (i.e. putting it in
OnSessionStateChanged is too late.)

Alternatively, we could check IsDeviceInDemoMode() when registering the
pref, but I'd like to avoid spreading the checks everywhere when
possible.

[1] https://cs.chromium.org/chromium/src/chrome/browser/history/top_sites_factory.cc?type=cs&q=khidewebstoreicon&sq=package:chromium&g=0&l=75

Bug: 885367
Change-Id: I9454acca4f02d91d5d85665f2cc3c29125ec7fb0
Reviewed-on: https://chromium-review.googlesource.com/c/1333121Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609053}

chrome/browser/chromeos/extensions/install_limiter_unittest.cc

@@ -7,7 +7,9 @@
 #include "base/macros.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_mode_test_helper.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/settings/stub_install_attributes.h"
+#include "components/user_manager/scoped_user_manager.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "extensions/common/constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -25,12 +27,15 @@ constexpr int kSmallExtensionSize = 200000;
 class InstallLimiterTest
     : public testing::TestWithParam<chromeos::DemoSession::DemoModeConfig> {
  public:
-  InstallLimiterTest() = default;
+  InstallLimiterTest()
+      : scoped_user_manager_(
+            std::make_unique<chromeos::FakeChromeUserManager>()) {}
   ~InstallLimiterTest() override = default;
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   chromeos::ScopedStubInstallAttributes test_install_attributes_;
+  user_manager::ScopedUserManager scoped_user_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(InstallLimiterTest);
 };

chrome/browser/chromeos/login/demo_mode/demo_extensions_external_loader_unittest.cc

@@ -140,7 +140,8 @@ class DemoExtensionsExternalLoaderTest : public testing::Test {
   DemoExtensionsExternalLoaderTest()
       : test_shared_loader_factory_(
             base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
-                &test_url_loader_factory_)) {}
+                &test_url_loader_factory_)),
+        scoped_user_manager_(std::make_unique<FakeChromeUserManager>()) {}
 
   ~DemoExtensionsExternalLoaderTest() override = default;
 
@@ -222,6 +223,8 @@ class DemoExtensionsExternalLoaderTest : public testing::Test {
 
   content::InProcessUtilityThreadHelper in_process_utility_thread_helper_;
 
+  user_manager::ScopedUserManager scoped_user_manager_;
+
   DISALLOW_COPY_AND_ASSIGN(DemoExtensionsExternalLoaderTest);
 };
 

chrome/browser/chromeos/login/demo_mode/demo_session.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_resources.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_setup_controller.h"
+#include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/system_tray_client.h"
@@ -38,7 +39,6 @@
 #include "components/prefs/pref_service.h"
 #include "components/session_manager/core/session_manager.h"
 #include "components/user_manager/user.h"
-#include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
@@ -356,6 +356,21 @@ void DemoSession::OverrideIgnorePinPolicyAppsForTesting(
   ignore_pin_policy_offline_apps_ = std::move(apps);
 }
 
+void DemoSession::ActiveUserChanged(const user_manager::User* user) {
+  const base::RepeatingClosure hide_web_store_icon = base::BindRepeating([]() {
+    ProfileManager::GetActiveUserProfile()->GetPrefs()->SetBoolean(
+        prefs::kHideWebStoreIcon, true);
+  });
+  user_manager::User* active_user =
+      user_manager::UserManager::Get()->GetActiveUser();
+  DCHECK_NE(active_user, user);
+  if (!active_user->is_profile_created()) {
+    active_user->AddProfileCreatedObserver(hide_web_store_icon);
+    return;
+  }
+  hide_web_store_icon.Run();
+}
+
 DemoSession::DemoSession()
     : offline_enrolled_(IsDemoModeOfflineEnrolled()),
       ignore_pin_policy_offline_apps_(GetIgnorePinPolicyApps()),
@@ -367,9 +382,12 @@ DemoSession::DemoSession()
     session_manager_observer_.Add(session_manager::SessionManager::Get());
     OnSessionStateChanged();
   }
+  ChromeUserManager::Get()->AddSessionStateObserver(this);
 }
 
-DemoSession::~DemoSession() = default;
+DemoSession::~DemoSession() {
+  ChromeUserManager::Get()->RemoveSessionStateObserver(this);
+}
 
 void DemoSession::InstallDemoResources() {
   DCHECK(demo_resources_->loaded());

chrome/browser/chromeos/login/demo_mode/demo_session.h

@@ -15,6 +15,7 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_extensions_external_loader.h"
 #include "components/session_manager/core/session_manager_observer.h"
+#include "components/user_manager/user_manager.h"
 #include "extensions/browser/extension_registry_observer.h"
 
 class PrefRegistrySimple;
@@ -30,7 +31,8 @@ class DemoResources;
 // Tracks global demo session state, such as whether the demo session has
 // started and the state of demo mode resources.
 class DemoSession : public session_manager::SessionManagerObserver,
-                    extensions::ExtensionRegistryObserver {
+                    public extensions::ExtensionRegistryObserver,
+                    public user_manager::UserManager::UserSessionStateObserver {
  public:
   // Type of demo mode configuration.
   // Warning: DemoModeConfig is stored in local state. Existing entries should
@@ -106,6 +108,9 @@ class DemoSession : public session_manager::SessionManagerObserver,
   // device is offline in Demo Mode.
   void OverrideIgnorePinPolicyAppsForTesting(std::vector<std::string> apps);
 
+  // user_manager::UserManager::UserSessionStateObserver:
+  void ActiveUserChanged(const user_manager::User* user) override;
+
   bool offline_enrolled() const { return offline_enrolled_; }
 
   bool started() const { return started_; }

chrome/browser/chromeos/login/demo_mode/demo_session_unittest.cc

@@ -56,7 +56,9 @@ class DemoSessionTest : public testing::Test {
  public:
   DemoSessionTest()
       : browser_process_platform_part_test_api_(
-            g_browser_process->platform_part()) {}
+            g_browser_process->platform_part()),
+        scoped_user_manager_(std::make_unique<FakeChromeUserManager>()) {}
+
   ~DemoSessionTest() override = default;
 
   void SetUp() override {
@@ -108,6 +110,7 @@ class DemoSessionTest : public testing::Test {
 
  private:
   BrowserProcessPlatformPartTestApi browser_process_platform_part_test_api_;
+  user_manager::ScopedUserManager scoped_user_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(DemoSessionTest);
 };
@@ -371,12 +374,7 @@ TEST_F(DemoSessionTest, MultipleEnsureOfflineResourcesLoaded) {
 
 class DemoSessionLocaleTest : public DemoSessionTest {
  public:
-  DemoSessionLocaleTest() {
-    auto fake_user_manager = std::make_unique<FakeChromeUserManager>();
-    user_manager_ = fake_user_manager.get();
-    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
-        std::move(fake_user_manager));
-  }
+  DemoSessionLocaleTest() = default;
 
   ~DemoSessionLocaleTest() override = default;
 
@@ -397,8 +395,10 @@ class DemoSessionLocaleTest : public DemoSessionTest {
   TestingProfile* LoginDemoUser() {
     const AccountId account_id(
         AccountId::FromUserEmailGaiaId("demo@test.com", "demo_user"));
+    FakeChromeUserManager* user_manager =
+        static_cast<FakeChromeUserManager*>(user_manager::UserManager::Get());
     const user_manager::User* user =
-        user_manager_->AddPublicAccountUser(account_id);
+        user_manager->AddPublicAccountUser(account_id);
 
     auto prefs =
         std::make_unique<sync_preferences::TestingPrefServiceSyncable>();
@@ -410,14 +410,12 @@ class DemoSessionLocaleTest : public DemoSessionTest {
     chromeos::ProfileHelper::Get()->SetUserToProfileMappingForTesting(user,
                                                                       profile);
 
-    user_manager_->LoginUser(account_id);
+    user_manager->LoginUser(account_id);
     profile_manager_->SetLoggedIn(true);
     return profile;
   }
 
  private:
-  FakeChromeUserManager* user_manager_ = nullptr;
-  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
   std::unique_ptr<TestingProfileManager> profile_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(DemoSessionLocaleTest);