Ensure the ExtensionSystem is ready before initializing an ExtensionAppModelBuilder

chrome/browser/extensions/extension_service_unittest.cc

 // Copyright (c) 2013 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/extensions/extension_service_unittest.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 460 matching lines @@
   }
   data_dir_ = test_data_dir.AppendASCII("extensions");
 }
 
 ExtensionServiceTestBase::~ExtensionServiceTestBase() {
   service_ = NULL;
 }
 
 void ExtensionServiceTestBase::InitializeExtensionService(
     const ExtensionServiceTestBase::ExtensionServiceInitParams& params) {
+  InitializeExtensionServiceForProfile(params, &profile_, &service_);
+  management_policy_ =
+      ExtensionSystem::Get(profile_.get())->management_policy();
+  extensions_install_dir_ = params.extensions_install_dir;
+  expected_extensions_count_ = 0;
+}
+
+// static
+void ExtensionServiceTestBase::InitializeExtensionServiceForProfile(
+    const ExtensionServiceInitParams& params,
+    scoped_ptr<TestingProfile>* profile_ptr,

[stevenjb, 2013/11/20 19:41:15]

This is pretty confusing. Use of scoped_ptr<>& and *& below is especially
confusing. I think we should break this up into a function that creates the
profile and returns a scoped_ptr<TestingProfile>, and a function that creates,
initializes, and returns the ExtensionService, i.e. something like:

profile_ = CreateTestingProfile(params);
service_ = InitializeExtensionServiceForProfile(params, profile_.get());

[benwells, 2013/11/21 00:47:47]

Yep, seems like you could encapsulate the bits you need in a more clean way.

[tapted, 2013/11/21 03:29:56]

Done.

+    ExtensionService** service_ptr) {
   TestingProfile::Builder profile_builder;
   // Create a PrefService that only contains user defined preference values.
   PrefServiceMockFactory factory;
   // If pref_file is empty, TestingProfile automatically creates
   // TestingPrefServiceSyncable instance.
   if (!params.pref_file.empty()) {
     factory.SetUserPrefsFile(params.pref_file,
                              base::MessageLoopProxy::current().get());
     scoped_refptr<user_prefs::PrefRegistrySyncable> registry(
         new user_prefs::PrefRegistrySyncable);
     scoped_ptr<PrefServiceSyncable> prefs(
         factory.CreateSyncable(registry.get()));
     chrome::RegisterUserProfilePrefs(registry.get());
     profile_builder.SetPrefService(prefs.Pass());
   }
 
   if (params.profile_is_managed)
     profile_builder.SetManagedUserId("asdf");
 
   profile_builder.SetPath(params.profile_path);
-  profile_ = profile_builder.Build();
+  scoped_ptr<TestingProfile>& profile = *profile_ptr;
+  profile = profile_builder.Build();
 
   TestExtensionSystem* system = static_cast<TestExtensionSystem*>(
-      ExtensionSystem::Get(profile_.get()));
+      ExtensionSystem::Get(profile.get()));
   if (!params.is_first_run) {
     ExtensionPrefs* prefs = system->CreateExtensionPrefs(
         CommandLine::ForCurrentProcess(),
         params.extensions_install_dir);
     prefs->SetAlertSystemFirstRun();
   }
 
-  service_ = system->CreateExtensionService(
+  ExtensionService*& service = *service_ptr;
+  service = system->CreateExtensionService(

[benwells, 2013/11/21 00:47:47]

huh? Why so many *'s and &'s??
more importantly, why assign twice in a row?

[tapted, 2013/11/21 03:29:56]

All fixed :) - at this stage I was still aiming on getting bots to report in..

       CommandLine::ForCurrentProcess(),
       params.extensions_install_dir,
       params.autoupdate_enabled);
 
-  service_->SetFileTaskRunnerForTesting(
+  service->SetFileTaskRunnerForTesting(
       base::MessageLoopProxy::current().get());
-  service_->set_extensions_enabled(true);
-  service_->set_show_extensions_prompts(false);
-  service_->set_install_updates_when_idle_for_test(false);
-
-  management_policy_ =
-      ExtensionSystem::Get(profile_.get())->management_policy();
-
-  extensions_install_dir_ = params.extensions_install_dir;
+  service->set_extensions_enabled(true);
+  service->set_show_extensions_prompts(false);
+  service->set_install_updates_when_idle_for_test(false);
 
   // When we start up, we want to make sure there is no external provider,
   // since the ExtensionService on Windows will use the Registry as a default
   // provider and if there is something already registered there then it will
   // interfere with the tests. Those tests that need an external provider
   // will register one specifically.
-  service_->ClearProvidersForTesting();
+  service->ClearProvidersForTesting();
 
 #if defined(OS_CHROMEOS)
-  extensions::InstallLimiter::Get(profile_.get())->DisableForTest();
+  extensions::InstallLimiter::Get(profile.get())->DisableForTest();
 #endif
-
-  expected_extensions_count_ = 0;
 }
 
 void ExtensionServiceTestBase::InitializeInstalledExtensionService(
     const base::FilePath& prefs_file,
     const base::FilePath& source_install_dir) {
   EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
   base::FilePath path = temp_dir_.path();
   path = path.Append(FILE_PATH_LITERAL("TestingExtensionsPath"));
   EXPECT_TRUE(base::DeleteFile(path, true));
   base::PlatformFileError error = base::PLATFORM_FILE_OK;
@@ skipping 55 matching lines @@
   ExtensionErrorReporter::GetInstance()->ClearErrors();
   content::RenderProcessHost::SetRunRendererInProcess(true);
 }
 
 void ExtensionServiceTestBase::TearDown() {
   content::RenderProcessHost::SetRunRendererInProcess(false);
 }
 
 ExtensionServiceTestBase::ExtensionServiceInitParams
 ExtensionServiceTestBase::CreateDefaultInitParams() {
+  return CreateDefaultInitParamsInTempDir(&temp_dir_);
+}
+
+// static
+ExtensionServiceTestBase::ExtensionServiceInitParams
+ExtensionServiceTestBase::CreateDefaultInitParamsInTempDir(
+    base::ScopedTempDir* temp_dir_) {
   ExtensionServiceInitParams params;
-  EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
-  base::FilePath path = temp_dir_.path();
+  EXPECT_TRUE(temp_dir_->CreateUniqueTempDir());
+  base::FilePath path = temp_dir_->path();
   path = path.Append(FILE_PATH_LITERAL("TestingExtensionsPath"));
   EXPECT_TRUE(base::DeleteFile(path, true));
   base::PlatformFileError error = base::PLATFORM_FILE_OK;
   EXPECT_TRUE(file_util::CreateDirectoryAndGetError(path, &error)) << error;
   base::FilePath prefs_filename =
       path.Append(FILE_PATH_LITERAL("TestPreferences"));
   base::FilePath extensions_install_dir =
       path.Append(FILE_PATH_LITERAL("Extensions"));
   EXPECT_TRUE(base::DeleteFile(extensions_install_dir, true));
   EXPECT_TRUE(file_util::CreateDirectoryAndGetError(extensions_install_dir,
@@ skipping 6111 matching lines @@
   service_->ReconcileKnownDisabled();
   expected_extensions.insert(good2);
   expected_disabled_extensions.erase(good2);
 
   EXPECT_EQ(expected_extensions, service_->extensions()->GetIDs());
   EXPECT_EQ(expected_disabled_extensions,
             service_->disabled_extensions()->GetIDs());
 }
 
 #endif // #if !(defined(OS_LINUX) && defined(USE_AURA) && !defined(OS_CHROMEOS))