Install default apps if sync is disabled.

chrome/browser/extensions/external_pref_loader.cc

Index: chrome/browser/extensions/external_pref_loader.cc
diff --git a/chrome/browser/extensions/external_pref_loader.cc b/chrome/browser/extensions/external_pref_loader.cc
index 61be1597a954c58f24198a971bfc220a8f3c3cf8..27b7da0577cef5442254041dee9692ded29b32f0 100644
--- a/chrome/browser/extensions/external_pref_loader.cc
+++ b/chrome/browser/extensions/external_pref_loader.cc
@@ -16,6 +16,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -119,6 +120,10 @@ void ExternalPrefLoader::StartLoading() {
       PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
       DCHECK(prefs);
       syncable_pref_observer_.Add(prefs);
+      ProfileSyncService* service =
+          ProfileSyncServiceFactory::GetForProfile(profile_);

[Andrew T Wilson (Slow), 2015/02/25 14:47:31]

You can't depend on ProfileSyncService existing, as it could be disabled by a
cmd-line flag or via GPO. I think this code will cause a crash if you run with
--disable-sync.

Maybe try calling profile->IsSyncAccessible() first, and if not, treat this like
sync is disabled.

[Ivan Podogov, 2015/02/25 16:34:14]

Funny thing is, IsSyncAccessible() call would also crash in that case. I've
adjusted patch a bit, so that it would behave better, but then again, on test
accounts I have I get some crashes even on an unpatched master when using
--disable-sync.

[Andrew T Wilson (Slow), 2015/02/25 17:47:22]

If you find those, please file bugs. These regress sometimes and we need to fix
them.

I don't think IsSyncAccessible() should ever crash - what's the crash you are
seeing when calling that?

[Ivan Podogov, 2015/02/26 12:50:14]

My bad, I mistook somethig: profile was nullptr, and it was not a crash, but a
mere DCHECK fail with SIGTRAP, that happens independently of --disable-sync...
Brought the IsSyncAccessible back, it should be enough to keep service available
in all cases.

+      DCHECK(service);
+      service->AddObserver(this);
     }
   } else {
     BrowserThread::PostTask(
@@ -131,6 +136,19 @@ void ExternalPrefLoader::OnIsSyncingChanged() {
   PostLoadIfPrioritySyncReady();
 }
 
+void ExternalPrefLoader::OnStateChanged() {
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetForProfile(profile_);
+  DCHECK(service);
+
+  ProfileSyncService::SyncStatusSummary status =
+      service->QuerySyncStatusSummary();
+  if (status == ProfileSyncService::SyncStatusSummary::NOT_ENABLED ||
+      status == ProfileSyncService::SyncStatusSummary::UNRECOVERABLE_ERROR) {

[Andrew T Wilson (Slow), 2015/02/25 14:47:31]

Not sure if perhaps you should be checking for UNKNOWN_ERROR or some of the
other error states? Perhaps ping nzea?

[Ivan Podogov, 2015/02/25 16:34:14]

UNKNOWN_ERROR corresponds to some state that even ProfileSyncService authors are
not sure it is possible to be in... I'm not sure what course of action would be
the best in that case.

[Andrew T Wilson (Slow), 2015/02/25 17:47:21]

What I want to make sure is we aren't left in a state where we never load
default apps. What happens if you treat it like an unrecoverable error (what's
the worst case scenario?)

+    PostLoadAndRemoveObservers();
+  }
+}
+
 bool ExternalPrefLoader::PostLoadIfPrioritySyncReady() {
   DCHECK(options_ & DELAY_LOAD_UNTIL_PRIORITY_SYNC);
   DCHECK(profile_);
@@ -138,16 +156,28 @@ bool ExternalPrefLoader::PostLoadIfPrioritySyncReady() {
   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);
+    PostLoadAndRemoveObservers();
     return true;
   }
 
   return false;
 }
 
+void ExternalPrefLoader::PostLoadAndRemoveObservers() {
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ExternalPrefLoader::LoadOnFileThread, this));
+
+  PrefServiceSyncable* prefs = PrefServiceSyncable::FromProfile(profile_);
+  DCHECK(prefs);
+  syncable_pref_observer_.Remove(prefs);
+
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetForProfile(profile_);
+  DCHECK(service);
+  service->RemoveObserver(this);
+}
+
 void ExternalPrefLoader::LoadOnFileThread() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));