cros: Install screensaver upon demo session start and avoid delay

The goal is to show screensaver immediately after demo session logs in.
In order to avoid the visual delay during the screensaver installation,
we need to:

1. Bypass install_limiter: it's reasonable to make an exception for
   the screensaver app during demo mode. The choices for the size limit
   (1MB) and the delay (5000 ms) are somewhat arbitrary in the first
   place. We'd still like to keep the install limiter in all other
   cases unless there's a real need to bypass it, ie:
   1) All other apps during demo mode,
   2) The screensaver app in non-demo mode (only possible if people
      randomly find it from the web store.)
   3) All other apps in non-demo mode (the majority cases).

2. Initiate installation earlier: if we wait until session state
   becomes active, then its installation will happen later than the
   policy forced ones. Instead, we can observe the profile creation
   and start installation right after profile is created.

   Highlights app should be installed at the same time, but do not
   change it for now because:

   1) Before we disable browser launch for demo sessions, we want
      Highlights app to appear between the screensaver and the
      browser window. This is hard to achieve if installing Highlights
      app at the same time with screensaver (which happens before
      browser launch).

   2) Installing another large app may affect installing the
      screensaver.


Bug: 870851
Change-Id: Ia8f7bb4f02d69e73a4c79839531ee1c190414d3d
Reviewed-on: https://chromium-review.googlesource.com/1212448
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592307}

chrome/browser/chromeos/BUILD.gn

@@ -2091,6 +2091,7 @@ source_set("unit_tests") {
     "extensions/file_manager/drivefs_event_router_unittest.cc",
     "extensions/file_manager/job_event_router_unittest.cc",
     "extensions/gfx_utils_unittest.cc",
+    "extensions/install_limiter_unittest.cc",
     "extensions/permissions_updater_delegate_chromeos_unittest.cc",
     "extensions/public_session_permission_helper_unittest.cc",
     "extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc",

chrome/browser/chromeos/extensions/install_limiter.cc

@@ -10,6 +10,7 @@
 #include "base/files/file_util.h"
 #include "base/task/post_task.h"
 #include "chrome/browser/chromeos/extensions/install_limiter_factory.h"
+#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "extensions/browser/notification_types.h"
@@ -46,10 +47,19 @@ InstallLimiter::DeferredInstall::~DeferredInstall() {
 ////////////////////////////////////////////////////////////////////////////////
 // InstallLimiter
 
+// static
 InstallLimiter* InstallLimiter::Get(Profile* profile) {
   return InstallLimiterFactory::GetForProfile(profile);
 }
 
+// static
+bool InstallLimiter::ShouldDeferInstall(int64_t app_size,
+                                        const std::string& app_id) {
+  constexpr int64_t kBigAppSizeThreshold = 1048576;  // 1MB in bytes
+  return app_size > kBigAppSizeThreshold &&
+         !chromeos::DemoSession::IsScreensaverInDemoMode(app_id);
+}
+
 InstallLimiter::InstallLimiter() : disabled_for_test_(false) {
 }
 
@@ -76,9 +86,7 @@ void InstallLimiter::Add(const scoped_refptr<CrxInstaller>& installer,
 void InstallLimiter::AddWithSize(const scoped_refptr<CrxInstaller>& installer,
                                  const base::FilePath& path,
                                  int64_t size) {
-  const int64_t kBigAppSizeThreshold = 1048576;  // 1MB
-
-  if (size <= kBigAppSizeThreshold) {
+  if (!ShouldDeferInstall(size, installer->expected_id())) {
     RunInstall(installer, path);
 
     // Stop wait timer and let install notification drive deferred installs.

chrome/browser/chromeos/extensions/install_limiter.h

@@ -32,6 +32,11 @@ class InstallLimiter : public KeyedService,
  public:
   static InstallLimiter* Get(Profile* profile);
 
+  // Install should be deferred if the size is larger than 1MB and the app is
+  // not the screensaver in demo mode (which requires instant installation to
+  // avoid visual delay).
+  static bool ShouldDeferInstall(int64_t app_size, const std::string& app_id);
+
   InstallLimiter();
   ~InstallLimiter() override;
 

chrome/browser/chromeos/extensions/install_limiter_unittest.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/extensions/install_limiter.h"
+
+#include "base/macros.h"
+#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
+#include "chrome/browser/chromeos/settings/stub_install_attributes.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "extensions/common/constants.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using extensions::InstallLimiter;
+
+namespace {
+
+constexpr char kRandomExtensionId[] = "abacabadabacabaeabacabadabacabaf";
+constexpr int kLargeExtensionSize = 2000000;
+constexpr int kSmallExtensionSize = 200000;
+
+}  // namespace
+
+class InstallLimiterTest : public testing::Test {
+ public:
+  InstallLimiterTest() = default;
+  ~InstallLimiterTest() override = default;
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  chromeos::ScopedStubInstallAttributes test_install_attributes_;
+
+  DISALLOW_COPY_AND_ASSIGN(InstallLimiterTest);
+};
+
+TEST_F(InstallLimiterTest, ShouldDeferInstall) {
+  // In non-demo mode, all apps larger than 1 MB should be deferred.
+  chromeos::DemoSession::SetDemoConfigForTesting(
+      chromeos::DemoSession::DemoModeConfig::kNone);
+  EXPECT_TRUE(InstallLimiter::ShouldDeferInstall(
+      kLargeExtensionSize, extension_misc::kScreensaverAppId));
+  EXPECT_TRUE(InstallLimiter::ShouldDeferInstall(kLargeExtensionSize,
+                                                 kRandomExtensionId));
+  EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(kSmallExtensionSize,
+                                                  kRandomExtensionId));
+
+  // In demo mode (either online or offline), all apps larger than 1MB except
+  // for the screensaver should be deferred.
+  chromeos::DemoSession::SetDemoConfigForTesting(
+      chromeos::DemoSession::DemoModeConfig::kOnline);
+  EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(
+      kLargeExtensionSize, extension_misc::kScreensaverAppId));
+  EXPECT_TRUE(InstallLimiter::ShouldDeferInstall(kLargeExtensionSize,
+                                                 kRandomExtensionId));
+  EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(kSmallExtensionSize,
+                                                  kRandomExtensionId));
+
+  chromeos::DemoSession::SetDemoConfigForTesting(
+      chromeos::DemoSession::DemoModeConfig::kOffline);
+  EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(
+      kLargeExtensionSize, extension_misc::kScreensaverAppId));
+  EXPECT_TRUE(InstallLimiter::ShouldDeferInstall(kLargeExtensionSize,
+                                                 kRandomExtensionId));
+  EXPECT_FALSE(InstallLimiter::ShouldDeferInstall(kSmallExtensionSize,
+                                                  kRandomExtensionId));
+
+  chromeos::DemoSession::ResetDemoConfigForTesting();
+}
\ No newline at end of file

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

@@ -80,12 +80,14 @@ DemoExtensionsExternalLoader::DemoExtensionsExternalLoader(
 DemoExtensionsExternalLoader::~DemoExtensionsExternalLoader() = default;
 
 void DemoExtensionsExternalLoader::LoadApp(const std::string& app_id) {
+  app_ids_.push_back(app_id);
   base::DictionaryValue prefs;
-  base::DictionaryValue app_dict;
-  app_dict.SetKey(extensions::ExternalProviderImpl::kExternalUpdateUrl,
-                  base::Value(extension_urls::kChromeWebstoreUpdateURL));
-  prefs.SetKey(app_id, std::move(app_dict));
-
+  for (const std::string& app_id : app_ids_) {
+    base::DictionaryValue app_dict;
+    app_dict.SetKey(extensions::ExternalProviderImpl::kExternalUpdateUrl,
+                    base::Value(extension_urls::kChromeWebstoreUpdateURL));
+    prefs.SetKey(app_id, std::move(app_dict));
+  }
   if (!external_cache_) {
     external_cache_ = std::make_unique<ExternalCacheImpl>(
         cache_dir_, g_browser_process->shared_url_loader_factory(),

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

@@ -71,6 +71,9 @@ class DemoExtensionsExternalLoader : public extensions::ExternalLoader,
 
   const base::FilePath cache_dir_;
 
+  // The list of app ids that should be cached by |external_cache_|.
+  std::vector<std::string> app_ids_;
+
   base::WeakPtrFactory<DemoExtensionsExternalLoader> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DemoExtensionsExternalLoader);

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

@@ -33,6 +33,8 @@
 #include "chromeos/dbus/image_loader_client.h"
 #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"
@@ -96,7 +98,7 @@ void InstallDemoMedia(base::FilePath offline_resources_path) {
 
   base::FilePath src_path = offline_resources_path.Append(kPhotosPath);
   base::FilePath dest_path = file_manager::util::GetDownloadsFolderForProfile(
-      ProfileManager::GetPrimaryUserProfile());
+      ProfileManager::GetActiveUserProfile());
 
   if (!base::CopyDirectory(src_path, dest_path, false /* recursive */))
     LOG(ERROR) << "Failed to install demo mode media.";
@@ -243,6 +245,11 @@ DemoSession* DemoSession::Get() {
   return g_demo_session;
 }
 
+// static
+bool DemoSession::IsScreensaverInDemoMode(const std::string& app_id) {
+  return app_id == extension_misc::kScreensaverAppId && IsDeviceInDemoMode();
+}
+
 void DemoSession::EnsureOfflineResourcesLoaded(
     base::OnceClosure load_callback) {
   if (offline_resources_loaded_) {
@@ -324,6 +331,12 @@ bool DemoSession::ShouldIgnorePinPolicy(const std::string& app_id_or_package) {
                    app_id_or_package) != ignore_pin_policy_offline_apps_.end();
 }
 
+void DemoSession::SetExtensionsExternalLoader(
+    scoped_refptr<DemoExtensionsExternalLoader> extensions_external_loader) {
+  extensions_external_loader_ = extensions_external_loader;
+  InstallAppFromUpdateUrl(extension_misc::kScreensaverAppId);
+}
+
 void DemoSession::OverrideIgnorePinPolicyAppsForTesting(
     std::vector<std::string> apps) {
   ignore_pin_policy_offline_apps_ = std::move(apps);
@@ -386,10 +399,10 @@ void DemoSession::LoadAndLaunchHighlightsApp() {
   DCHECK(offline_resources_loaded_);
   if (offline_resources_path_.empty()) {
     LOG(ERROR) << "Offline resources not loaded - no highlights app available.";
-    InstallHighlightsAppFromUpdateUrl();
+    InstallAppFromUpdateUrl(GetHighlightsAppId());
     return;
   }
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
+  Profile* profile = ProfileManager::GetActiveUserProfile();
   DCHECK(profile);
   const base::FilePath resources_path =
       offline_resources_path_.Append(kHighlightsAppPath);
@@ -397,17 +410,24 @@ void DemoSession::LoadAndLaunchHighlightsApp() {
           resources_path, base::CommandLine(base::CommandLine::NO_PROGRAM),
           base::FilePath() /* cur_dir */)) {
     LOG(WARNING) << "Failed to launch highlights app from offline resources.";
-    InstallHighlightsAppFromUpdateUrl();
+    InstallAppFromUpdateUrl(GetHighlightsAppId());
   }
 }
 
-void DemoSession::InstallHighlightsAppFromUpdateUrl() {
+void DemoSession::InstallAppFromUpdateUrl(const std::string& id) {
   if (!extensions_external_loader_)
     return;
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
+  auto* user = user_manager::UserManager::Get()->GetActiveUser();
+  if (!user->is_profile_created()) {
+    user->AddProfileCreatedObserver(
+        base::BindOnce(&DemoSession::InstallAppFromUpdateUrl,
+                       weak_ptr_factory_.GetWeakPtr(), id));
+    return;
+  }
+  Profile* profile = ProfileManager::GetActiveUserProfile();
   DCHECK(profile);
   extension_registry_observer_.Add(extensions::ExtensionRegistry::Get(profile));
-  extensions_external_loader_->LoadApp(GetHighlightsAppId());
+  extensions_external_loader_->LoadApp(id);
 }
 
 void DemoSession::OnSessionStateChanged() {
@@ -425,7 +445,7 @@ void DemoSession::OnExtensionInstalled(content::BrowserContext* browser_context,
                                        bool is_update) {
   if (extension->id() != GetHighlightsAppId())
     return;
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
+  Profile* profile = ProfileManager::GetActiveUserProfile();
   DCHECK(profile);
   OpenApplication(AppLaunchParams(
       profile, extension, extensions::LAUNCH_CONTAINER_WINDOW,

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

@@ -89,6 +89,10 @@ class DemoSession : public session_manager::SessionManagerObserver,
   // StartIfInDemoMode() or PreloadOfflineResourcesIfInDemoMode()).
   static DemoSession* Get();
 
+  // Returns whether |app_id| matches the screensaver app and the device is in
+  // demo mode.
+  static bool IsScreensaverInDemoMode(const std::string& app_id);
+
   // Ensures that the load of offline demo session resources is requested.
   // |load_callback| will be run once the offline resource load finishes.
   void EnsureOfflineResourcesLoaded(base::OnceClosure load_callback);
@@ -118,6 +122,10 @@ class DemoSession : public session_manager::SessionManagerObserver,
   // in Demo Mode and offline.
   bool ShouldIgnorePinPolicy(const std::string& app_id_or_package);
 
+  // Sets |extensions_external_loader_| and starts installing the screensaver.
+  void SetExtensionsExternalLoader(
+      scoped_refptr<DemoExtensionsExternalLoader> extensions_external_loader);
+
   // Sets app IDs and package names that shouldn't be pinned by policy when the
   // device is offline in Demo Mode.
   void OverrideIgnorePinPolicyAppsForTesting(std::vector<std::string> apps);
@@ -128,11 +136,6 @@ class DemoSession : public session_manager::SessionManagerObserver,
 
   bool offline_resources_loaded() const { return offline_resources_loaded_; }
 
-  void set_extensions_external_loader(
-      scoped_refptr<DemoExtensionsExternalLoader> extensions_external_loader) {
-    extensions_external_loader_ = extensions_external_loader;
-  }
-
  private:
   DemoSession();
   ~DemoSession() override;
@@ -161,8 +164,8 @@ class DemoSession : public session_manager::SessionManagerObserver,
   void LoadAndLaunchHighlightsApp();
 
   // Installs the CRX file from an update URL. Observes |ExtensionRegistry| to
-  // launch the highlights app upon installation.
-  void InstallHighlightsAppFromUpdateUrl();
+  // launch the app upon installation.
+  void InstallAppFromUpdateUrl(const std::string& id);
 
   // session_manager::SessionManagerObserver:
   void OnSessionStateChanged() override;

chrome/browser/extensions/external_provider_impl.cc

@@ -707,7 +707,7 @@ void ExternalProviderImpl::CreateExternalProviders(
             Manifest::EXTERNAL_PREF_DOWNLOAD, Extension::NO_FLAGS);
     demo_apps_provider->set_auto_acknowledge(true);
     demo_apps_provider->set_install_immediately(true);
-    chromeos::DemoSession::Get()->set_extensions_external_loader(loader);
+    chromeos::DemoSession::Get()->SetExtensionsExternalLoader(loader);
     provider_list->push_back(std::move(demo_apps_provider));
   }
 

extensions/browser/device_local_account_util.cc

@@ -101,6 +101,7 @@ const char* const kPublicSessionWhitelist[] = {
     // New demo mode:
     "lpmakjfjcconjeehbidjclhdlpjmfjjj",  // Highlights app
     "iggildboghmjpbjcpmobahnkmoefkike",  // Highlights app
+    "mnoijifedipmbjaoekhadjcijipaijjc",  // Screensaver
 
     // Testing extensions:
     "ongnjlefhnoajpbodoldndkbkdgfomlp",  // Show Managed Storage

extensions/common/constants.cc

@@ -119,6 +119,7 @@ const char kGeniusAppId[] = "ljoammodoonkhnehlncldjelhidljdpi";
 const char kHighlightsAppId[] = "lpmakjfjcconjeehbidjclhdlpjmfjjj";
 const char kHighlightsAlt1AppId[] = "iggildboghmjpbjcpmobahnkmoefkike";
 const char kHighlightsAlt2AppId[] = "elhbopodaklenjkeihkdhhfaghalllba";
+const char kScreensaverAppId[] = "mnoijifedipmbjaoekhadjcijipaijjc";
 #endif
 
 const char kProdHangoutsExtensionId[] = "nckgahadagoaajjgafhacjanaoiihapd";

extensions/common/constants.h

@@ -254,6 +254,9 @@ extern const char kHighlightsAlt1AppId[];
 
 // The extension id of an alternate Demo Mode Highlights app.
 extern const char kHighlightsAlt2AppId[];
+
+// The extension id of the default Demo Mode screensaver app.
+extern const char kScreensaverAppId[];
 #endif
 
 // The extension id for the production version of Hangouts.