Delay loading the SigninManagerFactory until the first-run Import process has completed.

chrome/browser/policy/user_policy_signin_service.cc

Index: chrome/browser/policy/user_policy_signin_service.cc
diff --git a/chrome/browser/policy/user_policy_signin_service.cc b/chrome/browser/policy/user_policy_signin_service.cc
index 81896754a34cbd30384da092aeddf6a650a657fb..b78e0ac76f1efe245795a5afab7d2bd03785334d 100644
--- a/chrome/browser/policy/user_policy_signin_service.cc
+++ b/chrome/browser/policy/user_policy_signin_service.cc
@@ -257,6 +257,12 @@ UserPolicySigninService::UserPolicySigninService(
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_ADDED,
                  content::Source<Profile>(profile));
+
+  // Register a listener for the import finished notification in a first run
+  // scenario, which indicates the profile is ready to be further initialized.
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_IMPORT_FINISHED,
+                 content::Source<Profile>(profile));
 }
 
 UserPolicySigninService::~UserPolicySigninService() {}
@@ -351,6 +357,16 @@ void UserPolicySigninService::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
+  // If an import process is running, wait for NOTIFICATION_IMPORT_FINISHED
+  // before potentially creating the SigninManager. Its dependencies can access
+  // databases that the import process is also accessing, causing conflicts.
+  // Note that the profile manager is NULL in unit tests.
+  if (g_browser_process->profile_manager() &&
+      g_browser_process->profile_manager()->will_import()) {
+    DCHECK_EQ(chrome::NOTIFICATION_PROFILE_ADDED, type);

[Bernhard Bauer, 2013/03/26 12:29:32]

Wait, doesn't this DCHECK fail when we get *any* other notification while the
import process is running?

[Joao da Silva, 2013/03/26 12:52:00]

will_import() signals that the first run import will be performed, which happens
synchronously on the UI thread. So other notifications can't be issued. See
ChromeBrowserMainParts::PreMainMessageLoopRunImpl().

If do_first_run_tasks_ is true, then it sets will_import, creates the profile
(which triggers NOTIFICATION_PROFILE_ADDED), and later runs the import process,
and then unsets the flag (which triggers NOTIFICATION_IMPORT_FINISHED, and by
that time will_import is already returning false). All of this happens in the
same function, and the UI loop isn't being pumped.

Does this make sense to you, or do you see other ways that could possibly end up
with another notification here?

[Bernhard Bauer, 2013/03/26 14:18:22]

Well, I ran into this with a local build that tries to set up Sync for managed
users, which requires passing in an OAuth token, thus triggering the
TOKEN_AVAILABLE notification.

It just seems strange (and brittle) to me that we assume no other notification
will happen, even if it may be completely independent of profile loading.

[Joao da Silva, 2013/03/26 14:35:27]

IIUC, that would only be a problem if you do that while the import is in
progress (or if you post to UI before startup has finished). Can you delay that
until the first_run_tasks are done?
IMO the larger problem is that the import process spins the UI loop. A lot more
places have similar stopgaps to handle the import process spinning the loop
before startup has finished.

I'm not sure what's the best fix here, and Drew is away for a couple of weeks. I
think it's OK to remove the DCHECK, since InitializeForSignedInUser() is called
anyway once the import process is done. If you change this then please check
back with Drew once he's back.

+    return;
+  }
+
   // If using a TestingProfile with no SigninManager or UserCloudPolicyManager,
   // skip initialization.
   if (!GetManager() || !SigninManagerFactory::GetForProfile(profile_)) {
@@ -362,6 +378,7 @@ void UserPolicySigninService::Observe(
     case chrome::NOTIFICATION_GOOGLE_SIGNED_OUT:
       ShutdownUserCloudPolicyManager();
       break;
+    case chrome::NOTIFICATION_IMPORT_FINISHED:
     case chrome::NOTIFICATION_PROFILE_ADDED: {
       // A new profile has been loaded - if it's signed in, then initialize the
       // UCPM, otherwise shut down the UCPM (which deletes any cached policy