Revert 184967

> Initialize BrowserPolicyConnector after the IOThread is created.
> 
> The import process on Windows spawns a subprocess and spins a nested message loop waiting for it to complete. If this subprocess takes longer than 5 seconds then the message loop will execute a delayed initialization task of the policy code, that grabs the system request context; however, since the IOThread hasn't been created yet this leads to a crash.
> 
> BUG=177843
> 
> Review URL: https://chromiumcodereview.appspot.com/12310100

TBR=joaodasilva@chromium.org
Review URL: https://codereview.chromium.org/12328146

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@184970 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 106 insertions, 119 deletions

diff --git a/chrome/browser/policy/async_policy_provider.cc b/chrome/browser/policy/async_policy_provider.cc
index fd3c05d..9163464 100644
--- a/chrome/browser/policy/async_policy_provider.cc
+++ b/chrome/browser/policy/async_policy_provider.cc
@@ -40,12 +40,11 @@ void AsyncPolicyProvider::Init() {
       base::Bind(&AsyncPolicyProvider::LoaderUpdateCallback,
                  base::MessageLoopProxy::current(),
                  weak_factory_.GetWeakPtr());
-  bool post = BrowserThread::PostTask(
+  BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&AsyncPolicyLoader::Init,
                  base::Unretained(loader_),
                  callback));
-  DCHECK(post) << "AsyncPolicyProvider::Init() called with threads not running";
 }
 
 void AsyncPolicyProvider::Shutdown() {
diff --git a/chrome/browser/policy/browser_policy_connector.cc b/chrome/browser/policy/browser_policy_connector.cc
index a9638c1..bcf6d26 100644
--- a/chrome/browser/policy/browser_policy_connector.cc
+++ b/chrome/browser/policy/browser_policy_connector.cc
@@ -28,11 +28,9 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
-#include "content/public/browser/browser_thread.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "grit/generated_resources.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "policy/policy_constants.h"
 #include "third_party/icu/public/i18n/unicode/regex.h"
 
@@ -70,8 +68,6 @@
 #include "chrome/browser/policy/user_cloud_policy_manager_factory.h"
 #endif
 
-using content::BrowserThread;
-
 namespace policy {
 
 namespace {
@@ -103,41 +99,7 @@ ConfigurationPolicyProvider* g_testing_provider = NULL;
 
 BrowserPolicyConnector::BrowserPolicyConnector()
     : is_initialized_(false),
-      local_state_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
-  // GetPolicyService() must be ready after the constructor is done.
-  // The connector is created very early during startup, when the browser
-  // threads aren't running yet; initialize components that need local_state,
-  // the system request context or other threads (e.g. FILE) at Init().
-
-  platform_provider_.reset(CreatePlatformProvider());
-
-  device_management_service_.reset(
-      new DeviceManagementService(GetDeviceManagementUrl()));
-
-#if defined(OS_CHROMEOS)
-  chromeos::CrosLibrary* cros_library = chromeos::CrosLibrary::Get();
-  // |cros_library| may be NULL on unit tests.
-  if (cros_library) {
-    chromeos::CryptohomeLibrary* cryptohome =
-        cros_library->GetCryptohomeLibrary();
-    install_attributes_.reset(new EnterpriseInstallAttributes(cryptohome));
-    base::FilePath install_attrs_file;
-    CHECK(PathService::Get(chrome::FILE_INSTALL_ATTRIBUTES,
-                           &install_attrs_file));
-    install_attributes_->ReadCacheFile(install_attrs_file);
-
-    scoped_ptr<DeviceCloudPolicyStoreChromeOS> device_cloud_policy_store(
-        new DeviceCloudPolicyStoreChromeOS(
-            chromeos::DeviceSettingsService::Get(),
-            install_attributes_.get()));
-    device_cloud_policy_manager_.reset(
-        new DeviceCloudPolicyManagerChromeOS(
-            device_cloud_policy_store.Pass(),
-            install_attributes_.get()));
-  }
-#endif
-}
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {}
 
 BrowserPolicyConnector::~BrowserPolicyConnector() {
   if (is_initialized()) {
@@ -149,40 +111,33 @@ BrowserPolicyConnector::~BrowserPolicyConnector() {
   }
 }
 
-void BrowserPolicyConnector::Init(
-    PrefService* local_state,
-    scoped_refptr<net::URLRequestContextGetter> request_context) {
-  // Initialization of some of the providers requires the FILE thread; make
-  // sure that threading is ready at this point.
-  DCHECK(BrowserThread::IsWellKnownThread(BrowserThread::FILE));
+void BrowserPolicyConnector::Init() {
   DCHECK(!is_initialized()) << "BrowserPolicyConnector::Init() called twice.";
+  platform_provider_.reset(CreatePlatformProvider());
 
-  local_state_ = local_state;
-  request_context_ = request_context;
-
-  device_management_service_->ScheduleInitialization(
-      kServiceInitializationStartupDelay);
-
-  if (g_testing_provider)
-    g_testing_provider->Init();
-  if (platform_provider_)
-    platform_provider_->Init();
+  if (!device_management_service_.get()) {
+    device_management_service_.reset(
+        new DeviceManagementService(GetDeviceManagementUrl()));
+    device_management_service_->ScheduleInitialization(
+        kServiceInitializationStartupDelay);
+  }
 
 #if defined(OS_CHROMEOS)
-  global_user_cloud_policy_provider_.Init();
-
-  if (device_cloud_policy_manager_) {
-    device_cloud_policy_manager_->Init();
-    scoped_ptr<CloudPolicyClient::StatusProvider> status_provider(
-        new DeviceStatusCollector(
-            local_state_,
-            chromeos::system::StatisticsProvider::GetInstance(),
-            NULL));
-    device_cloud_policy_manager_->Connect(
-        local_state_,
-        device_management_service_.get(),
-        status_provider.Pass());
-  }
+  chromeos::CryptohomeLibrary* cryptohome =
+      chromeos::CrosLibrary::Get()->GetCryptohomeLibrary();
+  install_attributes_.reset(new EnterpriseInstallAttributes(cryptohome));
+  base::FilePath install_attrs_file;
+  CHECK(PathService::Get(chrome::FILE_INSTALL_ATTRIBUTES, &install_attrs_file));
+  install_attributes_->ReadCacheFile(install_attrs_file);
+
+  scoped_ptr<DeviceCloudPolicyStoreChromeOS> device_cloud_policy_store(
+      new DeviceCloudPolicyStoreChromeOS(
+          chromeos::DeviceSettingsService::Get(),
+          install_attributes_.get()));
+  device_cloud_policy_manager_.reset(
+      new DeviceCloudPolicyManagerChromeOS(
+          device_cloud_policy_store.Pass(),
+          install_attributes_.get()));
 
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (!command_line->HasSwitch(switches::kDisableLocalAccounts)) {
@@ -190,21 +145,14 @@ void BrowserPolicyConnector::Init(
         new DeviceLocalAccountPolicyService(
             chromeos::DBusThreadManager::Get()->GetSessionManagerClient(),
             chromeos::DeviceSettingsService::Get()));
-    device_local_account_policy_service_->Connect(
-        device_management_service_.get());
   }
-
-  GetAppPackUpdater();
-
-  SetTimezoneIfPolicyAvailable();
 #endif
 
-  policy_statistics_collector_.reset(
-      new policy::PolicyStatisticsCollector(
-          GetPolicyService(),
-          local_state_,
-          MessageLoop::current()->message_loop_proxy()));
-  policy_statistics_collector_->Initialize();
+  // Complete the initialization once the message loops are spinning.
+  MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&BrowserPolicyConnector::CompleteInitialization,
+                 weak_ptr_factory_.GetWeakPtr()));
 
   is_initialized_ = true;
 }
@@ -262,16 +210,17 @@ PolicyService* BrowserPolicyConnector::GetPolicyService() {
 
 #if defined(OS_CHROMEOS)
 bool BrowserPolicyConnector::IsEnterpriseManaged() {
-  return install_attributes_ && install_attributes_->IsEnterpriseDevice();
+  return install_attributes_.get() && install_attributes_->IsEnterpriseDevice();
 }
 
 std::string BrowserPolicyConnector::GetEnterpriseDomain() {
-  return install_attributes_ ? install_attributes_->GetDomain() : std::string();
+  return install_attributes_.get() ? install_attributes_->GetDomain()
+                                   : std::string();
 }
 
 DeviceMode BrowserPolicyConnector::GetDeviceMode() {
-  return install_attributes_ ? install_attributes_->GetMode()
-                             : DEVICE_MODE_NOT_SET;
+  return install_attributes_.get() ? install_attributes_->GetMode()
+                                   : DEVICE_MODE_NOT_SET;
 }
 #endif
 
@@ -279,7 +228,7 @@ void BrowserPolicyConnector::ScheduleServiceInitialization(
     int64 delay_milliseconds) {
   // Skip device initialization if the BrowserPolicyConnector was never
   // initialized (unit tests).
-  if (device_management_service_)
+  if (device_management_service_.get())
     device_management_service_->ScheduleInitialization(delay_milliseconds);
 }
 
@@ -298,7 +247,7 @@ void BrowserPolicyConnector::InitializeUserPolicy(
   // (a) Existing profiles may hold pointers to |user_cloud_policy_manager_|.
   // (b) Implementing UserCloudPolicyManager::IsInitializationComplete()
   //     correctly is impossible for re-initialization.
-  CHECK(!user_cloud_policy_manager_);
+  CHECK(!user_cloud_policy_manager_.get());
 
   CommandLine* command_line = CommandLine::ForCurrentProcess();
 
@@ -314,7 +263,7 @@ void BrowserPolicyConnector::InitializeUserPolicy(
 
   if (wait_for_policy_fetch)
     device_management_service_->ScheduleInitialization(0);
-  if (is_public_account && device_local_account_policy_service_) {
+  if (is_public_account && device_local_account_policy_service_.get()) {
     device_local_account_policy_provider_.reset(
         new DeviceLocalAccountPolicyProvider(
             user_name, device_local_account_policy_service_.get()));
@@ -333,7 +282,7 @@ void BrowserPolicyConnector::InitializeUserPolicy(
                                            wait_for_policy_fetch));
 
     user_cloud_policy_manager_->Init();
-    user_cloud_policy_manager_->Connect(local_state_,
+    user_cloud_policy_manager_->Connect(g_browser_process->local_state(),
                                         device_management_service_.get(),
                                         GetUserAffiliation(user_name));
     global_user_cloud_policy_provider_.SetDelegate(
@@ -350,7 +299,7 @@ const ConfigurationPolicyHandlerList*
 UserAffiliation BrowserPolicyConnector::GetUserAffiliation(
     const std::string& user_name) {
 #if defined(OS_CHROMEOS)
-  if (install_attributes_ &&
+  if (install_attributes_.get() &&
       gaia::ExtractDomainName(gaia::CanonicalizeEmail(user_name)) ==
           install_attributes_->GetDomain()) {
     return USER_AFFILIATION_MANAGED;
@@ -362,10 +311,14 @@ UserAffiliation BrowserPolicyConnector::GetUserAffiliation(
 
 #if defined(OS_CHROMEOS)
 AppPackUpdater* BrowserPolicyConnector::GetAppPackUpdater() {
-  // request_context_ is NULL in unit tests.
-  if (!app_pack_updater_ && request_context_) {
-    app_pack_updater_.reset(
-        new AppPackUpdater(request_context_, install_attributes_.get()));
+  if (!app_pack_updater_.get()) {
+    // system_request_context() is NULL in unit tests.
+    net::URLRequestContextGetter* request_context =
+        g_browser_process->system_request_context();
+    if (request_context) {
+      app_pack_updater_.reset(
+          new AppPackUpdater(request_context, install_attributes_.get()));
+    }
   }
   return app_pack_updater_.get();
 }
@@ -374,9 +327,9 @@ AppPackUpdater* BrowserPolicyConnector::GetAppPackUpdater() {
 #if defined(OS_CHROMEOS)
 NetworkConfigurationUpdater*
     BrowserPolicyConnector::GetNetworkConfigurationUpdater() {
-  if (!network_configuration_updater_) {
+  if (!network_configuration_updater_.get()) {
     network_configuration_updater_.reset(new NetworkConfigurationUpdater(
-        GetPolicyService(),
+        g_browser_process->policy_service(),
         chromeos::CrosLibrary::Get()->GetNetworkLibrary()));
   }
   return network_configuration_updater_.get();
@@ -467,6 +420,50 @@ void BrowserPolicyConnector::RegisterPrefs(PrefRegistrySimple* registry) {
 #endif
 }
 
+void BrowserPolicyConnector::CompleteInitialization() {
+  if (g_testing_provider)
+    g_testing_provider->Init();
+  if (platform_provider_)
+    platform_provider_->Init();
+
+#if defined(OS_CHROMEOS)
+  global_user_cloud_policy_provider_.Init();
+
+  // Create the AppPackUpdater to start updating the cache. It requires the
+  // system request context, which isn't available in Init(); therefore it is
+  // created only once the loops are running.
+  GetAppPackUpdater();
+
+  if (device_cloud_policy_manager_.get()) {
+    device_cloud_policy_manager_->Init();
+    scoped_ptr<CloudPolicyClient::StatusProvider> status_provider(
+        new DeviceStatusCollector(g_browser_process->local_state(),
+            chromeos::system::StatisticsProvider::GetInstance(),
+            NULL));
+    device_cloud_policy_manager_->Connect(
+        g_browser_process->local_state(),
+        device_management_service_.get(),
+        status_provider.Pass());
+  }
+
+  if (device_local_account_policy_service_.get()) {
+    device_local_account_policy_service_->Connect(
+        device_management_service_.get());
+  }
+
+  SetTimezoneIfPolicyAvailable();
+#endif
+
+  // TODO: Do not use g_browser_process once policy service is moved to
+  // BrowserPolicyConnector (http://crbug.com/128999).
+  policy_statistics_collector_.reset(
+      new policy::PolicyStatisticsCollector(
+          g_browser_process->policy_service(),
+          g_browser_process->local_state(),
+          MessageLoop::current()->message_loop_proxy()));
+  policy_statistics_collector_->Initialize();
+}
+
 void BrowserPolicyConnector::SetTimezoneIfPolicyAvailable() {
 #if defined(OS_CHROMEOS)
   typedef chromeos::CrosSettingsProvider Provider;
@@ -501,7 +498,7 @@ scoped_ptr<PolicyService>
       providers.push_back(platform_provider_.get());
 
 #if defined(OS_CHROMEOS)
-    if (device_cloud_policy_manager_)
+    if (device_cloud_policy_manager_.get())
       providers.push_back(device_cloud_policy_manager_.get());
     if (!user_cloud_policy_provider)
       user_cloud_policy_provider = &global_user_cloud_policy_provider_;
diff --git a/chrome/browser/policy/browser_policy_connector.h b/chrome/browser/policy/browser_policy_connector.h
index a2fbe19..992492c 100644
--- a/chrome/browser/policy/browser_policy_connector.h
+++ b/chrome/browser/policy/browser_policy_connector.h
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/policy/cloud_policy_constants.h"
@@ -16,13 +15,8 @@
 #include "chrome/browser/policy/proxy_policy_provider.h"
 
 class PrefRegistrySimple;
-class PrefService;
 class Profile;
 
-namespace net {
-class URLRequestContextGetter;
-}
-
 namespace policy {
 
 class ConfigurationPolicyProvider;
@@ -51,10 +45,10 @@ class BrowserPolicyConnector {
   // Invoke Shutdown() before deleting, see below.
   virtual ~BrowserPolicyConnector();
 
-  // Finalizes the initialization of the connector. This call can be skipped on
-  // tests that don't require the full policy system running.
-  void Init(PrefService* local_state,
-            scoped_refptr<net::URLRequestContextGetter> request_context);
+  // Creates the policy providers and finalizes the initialization of the
+  // connector. This call can be skipped on tests that don't require the full
+  // policy system running.
+  void Init();
 
   // Stops the policy providers and cleans up the connector before it can be
   // safely deleted. This must be invoked before the destructor and while the
@@ -157,6 +151,10 @@ class BrowserPolicyConnector {
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
  private:
+  // Complete initialization once the message loops are running and the
+  // local_state is initialized.
+  void CompleteInitialization();
+
   // Set the timezone as soon as the policies are available.
   void SetTimezoneIfPolicyAvailable();
 
@@ -172,9 +170,6 @@ class BrowserPolicyConnector {
   // Whether Init() but not Shutdown() has been invoked.
   bool is_initialized_;
 
-  PrefService* local_state_;
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
-
   // Used to convert policies to preferences. The providers declared below
   // may trigger policy updates during shutdown, which will result in
   // |handler_list_| being consulted for policy translation.
diff --git a/chrome/browser/policy/user_policy_signin_service_unittest.cc b/chrome/browser/policy/user_policy_signin_service_unittest.cc
index 446e6c1..0fc0510 100644
--- a/chrome/browser/policy/user_policy_signin_service_unittest.cc
+++ b/chrome/browser/policy/user_policy_signin_service_unittest.cc
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
-#include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
@@ -33,7 +32,6 @@
 #include "google_apis/gaia/gaia_constants.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
-#include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -63,7 +61,7 @@ static const char kHostedDomainResponse[] =
 class UserPolicySigninServiceTest : public testing::Test {
  public:
   UserPolicySigninServiceTest()
-      : loop_(MessageLoop::TYPE_IO),
+      : loop_(MessageLoop::TYPE_UI),
         ui_thread_(content::BrowserThread::UI, &loop_),
         file_thread_(content::BrowserThread::FILE, &loop_),
         io_thread_(content::BrowserThread::IO, &loop_),
@@ -90,15 +88,13 @@ class UserPolicySigninServiceTest : public testing::Test {
         SetDeviceManagementServiceForTesting(
             scoped_ptr<DeviceManagementService>(device_management_service_));
 
+    g_browser_process->browser_policy_connector()->Init();
+
     local_state_.reset(new TestingPrefServiceSimple);
     chrome::RegisterLocalState(local_state_->registry());
     TestingBrowserProcess::GetGlobal()->SetLocalState(
         local_state_.get());
 
-    scoped_refptr<net::URLRequestContextGetter> system_request_context;
-    g_browser_process->browser_policy_connector()->Init(
-        local_state_.get(), system_request_context);
-
     // Create a testing profile with cloud-policy-on-signin enabled, and bring
     // up a UserCloudPolicyManager with a MockUserCloudPolicyStore.
     scoped_ptr<TestingPrefServiceSyncable> prefs(