Delay default apps installation on Chrome OS for first time sign-in

Default apps installation significantly affects session start so sometimes
even priority settings are not able to sync in time.

BUG=424075
TEST=manual

Review URL: https://codereview.chromium.org/650763003

Cr-Commit-Position: refs/heads/master@{#301599}

chrome/browser/extensions/external_pref_loader.cc

@@ -15,6 +15,7 @@
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -90,11 +91,19 @@ base::DictionaryValue* ExtractExtensionPrefs(base::ValueSerializer* serializer,
 
 namespace extensions {
 
-ExternalPrefLoader::ExternalPrefLoader(int base_path_id, Options options)
-    : base_path_id_(base_path_id), options_(options) {
+ExternalPrefLoader::ExternalPrefLoader(int base_path_id,
+                                       Options options,
+                                       Profile* profile)
+    : base_path_id_(base_path_id),
+      options_(options),
+      profile_(profile),
+      syncable_pref_observer_(this) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
+ExternalPrefLoader::~ExternalPrefLoader() {
+}
+
 const base::FilePath ExternalPrefLoader::GetBaseCrxFilePath() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -104,9 +113,39 @@ const base::FilePath ExternalPrefLoader::GetBaseCrxFilePath() {
 
 void ExternalPrefLoader::StartLoading() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&ExternalPrefLoader::LoadOnFileThread, this));
+  if (options_ & DELAY_LOAD_UNTIL_PRIORITY_SYNC) {
+    if (!PostLoadIfPrioritySyncReady()) {
+      DCHECK(profile_);
+      PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+      DCHECK(prefs);
+      syncable_pref_observer_.Add(prefs);
+    }
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&ExternalPrefLoader::LoadOnFileThread, this));
+  }
+}
+
+void ExternalPrefLoader::OnIsSyncingChanged() {
+  PostLoadIfPrioritySyncReady();
+}
+
+bool ExternalPrefLoader::PostLoadIfPrioritySyncReady() {
+  DCHECK(options_ & DELAY_LOAD_UNTIL_PRIORITY_SYNC);
+  DCHECK(profile_);
+
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  DCHECK(prefs);
+  if (prefs->IsPrioritySyncing()) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&ExternalPrefLoader::LoadOnFileThread, this));
+    syncable_pref_observer_.Remove(prefs);
+    return true;
+  }
+
+  return false;
 }
 
 void ExternalPrefLoader::LoadOnFileThread() {

chrome/browser/extensions/external_pref_loader.h

@@ -5,13 +5,17 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTERNAL_PREF_LOADER_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTERNAL_PREF_LOADER_H_
 
-#include "chrome/browser/extensions/external_loader.h"
-
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/external_loader.h"
+#include "chrome/browser/prefs/pref_service_syncable_observer.h"
+
+class PrefServiceSyncable;
+class Profile;
 
 namespace extensions {
 
@@ -19,7 +23,8 @@ namespace extensions {
 // look up which external extensions are registered.
 // Instances of this class are expected to be created and destroyed on the UI
 // thread and they are expecting public method calls from the UI thread.
-class ExternalPrefLoader : public ExternalLoader {
+class ExternalPrefLoader : public ExternalLoader,
+                           public PrefServiceSyncableObserver {
  public:
   enum Options {
     NONE = 0,
@@ -27,18 +32,23 @@ class ExternalPrefLoader : public ExternalLoader {
     // Ensure that only root can force an external install by checking
     // that all components of the path to external extensions files are
     // owned by root and not writable by any non-root user.
-    ENSURE_PATH_CONTROLLED_BY_ADMIN = 1 << 0
+    ENSURE_PATH_CONTROLLED_BY_ADMIN = 1 << 0,
+
+    // Delay external preference load. It delays default apps installation
+    // to not overload the system on first time user login.
+    DELAY_LOAD_UNTIL_PRIORITY_SYNC = 1 << 1,
   };
 
   // |base_path_id| is the directory containing the external_extensions.json
   // file or the standalone extension manifest files. Relative file paths to
-  // extension files are resolved relative to this path.
-  ExternalPrefLoader(int base_path_id, Options options);
+  // extension files are resolved relative to this path. |profile| is used to
+  // wait priority sync if DELAY_LOAD_UNTIL_PRIORITY_SYNC set.
+  ExternalPrefLoader(int base_path_id, Options options, Profile* profile);
 
   const base::FilePath GetBaseCrxFilePath() override;
 
  protected:
-  ~ExternalPrefLoader() override {}
+  ~ExternalPrefLoader() override;
 
   void StartLoading() override;
   bool IsOptionSet(Options option) {
@@ -54,6 +64,12 @@ class ExternalPrefLoader : public ExternalLoader {
  private:
   friend class base::RefCountedThreadSafe<ExternalLoader>;
 
+  // PrefServiceSyncableObserver:
+  void OnIsSyncingChanged() override;
+
+  // If priority sync ready posts LoadOnFileThread and return true.
+  bool PostLoadIfPrioritySyncReady();
+
   // Actually searches for and loads candidate standalone extension preference
   // files in the path corresponding to |base_path_id|.
   // Must be called on the file thread.
@@ -76,6 +92,14 @@ class ExternalPrefLoader : public ExternalLoader {
   // describing which extensions to load.
   base::FilePath base_path_;
 
+  // Profile that loads these external prefs.
+  // Needed for waiting for waiting priority sync.
+  Profile* profile_;
+
+  // Used for registering observer for PrefServiceSyncable.
+  ScopedObserver<PrefServiceSyncable, PrefServiceSyncableObserver>
+      syncable_pref_observer_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalPrefLoader);
 };
 

chrome/browser/extensions/external_provider_impl.cc

@@ -482,7 +482,8 @@ void ExternalProviderImpl::CreateExternalProviders(
                 service,
                 new ExternalPrefLoader(
                     chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS,
-                    ExternalPrefLoader::NONE),
+                    ExternalPrefLoader::NONE,
+                    NULL),
                 profile,
                 Manifest::EXTERNAL_PREF,
                 Manifest::EXTERNAL_PREF_DOWNLOAD,
@@ -495,11 +496,15 @@ void ExternalProviderImpl::CreateExternalProviders(
     int external_apps_path_id = profile->IsSupervised() ?
         chrome::DIR_SUPERVISED_USERS_DEFAULT_APPS :
         chrome::DIR_STANDALONE_EXTERNAL_EXTENSIONS;
+    ExternalPrefLoader::Options pref_load_flags = profile->IsNewProfile() ?
+        ExternalPrefLoader::DELAY_LOAD_UNTIL_PRIORITY_SYNC :
+        ExternalPrefLoader::NONE;
     provider_list->push_back(
         linked_ptr<ExternalProviderInterface>(new ExternalProviderImpl(
             service,
             new ExternalPrefLoader(external_apps_path_id,
-                                   ExternalPrefLoader::NONE),
+                                   pref_load_flags,
+                                   profile),
             profile,
             Manifest::EXTERNAL_PREF,
             Manifest::EXTERNAL_PREF_DOWNLOAD,
@@ -541,7 +546,8 @@ void ExternalProviderImpl::CreateExternalProviders(
             new ExternalProviderImpl(
                 service,
                 new ExternalPrefLoader(chrome::DIR_EXTERNAL_EXTENSIONS,
-                                       check_admin_permissions_on_mac),
+                                       check_admin_permissions_on_mac,
+                                       NULL),
                 profile,
                 Manifest::EXTERNAL_PREF,
                 Manifest::EXTERNAL_PREF_DOWNLOAD,
@@ -555,7 +561,8 @@ void ExternalProviderImpl::CreateExternalProviders(
             new ExternalProviderImpl(
                 service,
                 new ExternalPrefLoader(chrome::DIR_USER_EXTERNAL_EXTENSIONS,
-                                       ExternalPrefLoader::NONE),
+                                       ExternalPrefLoader::NONE,
+                                       NULL),
                 profile,
                 Manifest::EXTERNAL_PREF,
                 Manifest::EXTERNAL_PREF_DOWNLOAD,
@@ -583,7 +590,8 @@ void ExternalProviderImpl::CreateExternalProviders(
                 profile,
                 service,
                 new ExternalPrefLoader(chrome::DIR_DEFAULT_APPS,
-                                       ExternalPrefLoader::NONE),
+                                       ExternalPrefLoader::NONE,
+                                       NULL),
                 Manifest::INTERNAL,
                 Manifest::INTERNAL,
                 Extension::FROM_WEBSTORE |

chrome/browser/extensions/external_provider_impl_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/extensions/extension_service_test_base.h"
 #include "chrome/browser/extensions/updater/extension_cache_fake.h"
 #include "chrome/browser/extensions/updater/extension_updater.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -61,7 +62,7 @@ class ExternalProviderImplTest : public ExtensionServiceTestBase {
     chromeos::ScopedUserManagerEnabler scoped_user_manager(
         new chromeos::FakeUserManager);
 #endif
-    InitializeExtensionServiceWithUpdater();
+    InitializeExtensionServiceWithUpdaterAndPrefs();
 
     service()->updater()->SetExtensionCacheForTesting(
         test_extension_cache_.get());
@@ -82,6 +83,17 @@ class ExternalProviderImplTest : public ExtensionServiceTestBase {
     }
   }
 
+  void InitializeExtensionServiceWithUpdaterAndPrefs() {
+    ExtensionServiceInitParams params = CreateDefaultInitParams();
+    params.autoupdate_enabled = true;
+    // Create prefs file to make the profile not new.
+    const char prefs[] = "{}";
+    EXPECT_EQ(base::WriteFile(params.pref_file, prefs, sizeof(prefs)),
+              int(sizeof(prefs)));
+    InitializeExtensionService(params);
+    service_->updater()->Start();
+  }
+
   // ExtensionServiceTestBase overrides:
   virtual void SetUp() override {
     ExtensionServiceTestBase::SetUp();