Clean up ExtensionServiceTest.

chrome/browser/extensions/extension_service_unittest.cc

 // Copyright (c) 2011 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>
 
-#include "base/at_exit.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_value_serializer.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
@@ skipping 351 matching lines @@
 
  private:
   ExtensionService* service_;
   scoped_refptr<ChromeAppCacheService> appcache_service_;
   scoped_refptr<fileapi::FileSystemContext> file_system_context_;
 };
 
 // Our message loop may be used in tests which require it to be an IO loop.
 ExtensionServiceTestBase::ExtensionServiceTestBase()
     : service_(NULL),
-      total_successes_(0),
+      expected_extensions_count_(0),
       loop_(MessageLoop::TYPE_IO),
       ui_thread_(BrowserThread::UI, &loop_),
       db_thread_(BrowserThread::DB, &loop_),
       webkit_thread_(BrowserThread::WEBKIT, &loop_),
       file_thread_(BrowserThread::FILE, &loop_),
       io_thread_(BrowserThread::IO, &loop_) {
   FilePath test_data_dir;
   if (!PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir)) {
     ADD_FAILURE();
     return;
@@ skipping 31 matching lines @@
   service_->set_show_extensions_prompts(false);
   profile->set_extensions_service(service_);
 
   // 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();
 
-  total_successes_ = 0;
+  expected_extensions_count_ = 0;
 }
 
 void ExtensionServiceTestBase::InitializeInstalledExtensionService(
     const FilePath& prefs_file, const FilePath& source_install_dir) {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
   FilePath path_ = temp_dir_.path();
   path_ = path_.Append(FILE_PATH_LITERAL("TestingExtensionsPath"));
   file_util::Delete(path_, true);
   file_util::CreateDirectory(path_);
   FilePath temp_prefs = path_.Append(FILE_PATH_LITERAL("Preferences"));
@@ skipping 101 matching lines @@
   }
 
   void AddMockExternalProvider(ExternalExtensionProviderInterface* provider) {
     service_->AddProviderForTesting(provider);
   }
 
  protected:
   void TestExternalProvider(MockExtensionProvider* provider,
                             Extension::Location location);
 
-  void PackAndInstallCrx(const FilePath& dir_path,
-                         const FilePath& pem_path,
-                         bool should_succeed) {
-    FilePath crx_path;
-    ScopedTempDir temp_dir;
-    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-    crx_path = temp_dir_.path().AppendASCII("temp.crx");
-
+  void PackCrx(const FilePath& dir_path,

[Aaron Boodman, 2011/11/20 21:39:43]

As long as you're in here, can you fix the capitalization (CRX instead of Crx)

[Yoyo Zhou, 2011/11/21 23:14:52]

Why is CRX better? It looks like neither capitalization is a clear winner in the
codebase.

[Aaron Boodman, 2011/11/22 10:02:59]

I did a search a few weeks back of things like "Url" vs "URL" and "Id" vs "ID"
and found that Chromium prefers all caps, so I've been trying to fix things to
be more consistent as I see them.

[Yoyo Zhou, 2011/11/22 19:40:55]

Ok, I'll keep that in mind. Changed.

+               const FilePath& pem_path,
+               const FilePath& crx_path) {
     // Use the existing pem key, if provided.
     FilePath pem_output_path;
     if (pem_path.value().empty()) {
       pem_output_path = crx_path.DirName().AppendASCII("temp.pem");
-      ASSERT_TRUE(file_util::Delete(pem_output_path, false));

[Aaron Boodman, 2011/11/20 21:39:43]

Why remove this?

[Yoyo Zhou, 2011/11/21 23:14:52]

If you look carefully at the code above,

crx_path = temp_dir_... (*not* temp_dir...)

The same tempdir was getting reused, so I assume this was necessary before.
However, if we use a new tempdir properly, it's probably fair to assume a file
path inside it will not exist.

     } else {
       ASSERT_TRUE(file_util::PathExists(pem_path));
     }
 
     ASSERT_TRUE(file_util::Delete(crx_path, false));
 
     scoped_ptr<ExtensionCreator> creator(new ExtensionCreator());
     ASSERT_TRUE(creator->Run(dir_path,
                              crx_path,
                              pem_path,
                              pem_output_path));
 
     ASSERT_TRUE(file_util::PathExists(crx_path));
-    InstallCrx(crx_path, should_succeed);
   }
 
-  void PackAndInstallCrx(const FilePath& dir_path,
-                         bool should_succeed) {
-    PackAndInstallCrx(dir_path, FilePath(), should_succeed);
+  const Extension* PackAndInstallCrx(const FilePath& dir_path,
+                                     const FilePath& pem_path,
+                                     bool should_succeed) {
+    FilePath crx_path;
+    ScopedTempDir temp_dir;
+    EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
+    crx_path = temp_dir.path().AppendASCII("temp.crx");
+
+    PackCrx(dir_path, pem_path, crx_path);
+    return InstallCrx(crx_path, should_succeed);
+  }
+
+  const Extension* PackAndInstallCrx(const FilePath& dir_path,
+                                     bool should_succeed) {
+    return PackAndInstallCrx(dir_path, FilePath(), should_succeed);
   }
 
   // Create a CrxInstaller and start installation. To allow the install
   // to happen, use loop_.RunAllPending();. Most tests will not use this
   // method directly.  Instead, use InstallCrx(), which waits for
   // the crx to be installed and does extra error checking.
   void StartCrxInstall(const FilePath& crx_path) {
     StartCrxInstall(crx_path, false);
   }
 
   void StartCrxInstall(const FilePath& crx_path, bool from_webstore) {
     ASSERT_TRUE(file_util::PathExists(crx_path))
         << "Path does not exist: "<< crx_path.value().c_str();
     scoped_refptr<CrxInstaller> installer(CrxInstaller::Create(service_, NULL));
     installer->set_allow_silent_install(true);
     installer->set_is_gallery_install(from_webstore);
     installer->InstallCrx(crx_path);
   }
 
-  void InstallCrx(const FilePath& path,
-                        bool should_succeed) {
+  const Extension* InstallCrx(const FilePath& path,
+                              bool should_succeed) {
     StartCrxInstall(path);
-    WaitForCrxInstall(path, should_succeed);
+    return WaitForCrxInstall(path, should_succeed);
   }
 
-  void InstallCrxWithLocation(const FilePath& crx_path,
-                              Extension::Location install_location,
-                              bool should_succeed) {
-    ASSERT_TRUE(file_util::PathExists(crx_path))
+  const Extension* InstallCrxFromWebStore(const FilePath& path,
+                                          bool should_succeed) {
+    StartCrxInstall(path, true);
+    return WaitForCrxInstall(path, should_succeed);
+  }
+
+  const Extension* InstallCrxWithLocation(const FilePath& crx_path,
+                                          Extension::Location install_location,
+                                          bool should_succeed) {
+    EXPECT_TRUE(file_util::PathExists(crx_path))
         << "Path does not exist: "<< crx_path.value().c_str();
     // no client (silent install)
     scoped_refptr<CrxInstaller> installer(CrxInstaller::Create(service_, NULL));
 
     installer->set_install_source(install_location);
     installer->InstallCrx(crx_path);
 
-    WaitForCrxInstall(crx_path, should_succeed);
+    return WaitForCrxInstall(crx_path, should_succeed);
   }
 
   // Wait for a CrxInstaller to finish. Used by InstallCrx.
-  void WaitForCrxInstall(const FilePath& path,
-                         bool should_succeed) {
+  // Returns the ID of the extension if the install succeeded, NULL otherwise.

[Aaron Boodman, 2011/11/20 21:39:43]

Comment is incorrect.

[Yoyo Zhou, 2011/11/21 23:14:52]

Oops, fixed.

+  const Extension* WaitForCrxInstall(const FilePath& path,
+                                     bool should_succeed) {
     loop_.RunAllPending();
     std::vector<std::string> errors = GetErrors();
+    const Extension* extension = NULL;
     if (should_succeed) {
-      ++total_successes_;
+      ++expected_extensions_count_;
 
       EXPECT_TRUE(installed_) << path.value();
 
-      ASSERT_EQ(1u, loaded_.size()) << path.value();
+      EXPECT_EQ(1u, loaded_.size()) << path.value();
       EXPECT_EQ(0u, errors.size()) << path.value();
-      EXPECT_EQ(total_successes_, service_->extensions()->size()) <<
+      EXPECT_EQ(expected_extensions_count_, service_->extensions()->size()) <<
           path.value();
-      EXPECT_TRUE(service_->GetExtensionById(loaded_[0]->id(), false)) <<
+      extension = loaded_[0];
+      EXPECT_TRUE(service_->GetExtensionById(extension->id(), false)) <<
           path.value();
       for (std::vector<std::string>::iterator err = errors.begin();
         err != errors.end(); ++err) {
         LOG(ERROR) << *err;
       }
     } else {
       EXPECT_FALSE(installed_) << path.value();
       EXPECT_EQ(0u, loaded_.size()) << path.value();
       EXPECT_EQ(1u, errors.size()) << path.value();
     }
 
     installed_ = NULL;
     loaded_.clear();
     ExtensionErrorReporter::GetInstance()->ClearErrors();
+    return extension;
   }
 
   enum UpdateState {
     FAILED_SILENTLY,
     FAILED,
     UPDATED,
     INSTALLED,
     ENABLED
   };
 
@@ skipping 72 matching lines @@
     size_t pref_key_count = GetPrefKeyCount();
     EXPECT_GT(pref_key_count, 0u);
     ValidateIntegerPref(id, "state", Extension::ENABLED);
 
     // Uninstall it.
     if (use_helper) {
       EXPECT_TRUE(ExtensionService::UninstallExtensionHelper(service_, id));
     } else {
       EXPECT_TRUE(service_->UninstallExtension(id, false, NULL));
     }
-    total_successes_ = 0;
+    expected_extensions_count_ = 0;

[Aaron Boodman, 2011/11/20 21:39:43]

Seems like maybe this isn't needed anymore? It's only tested by
WaitForCrxInstall() which doesn't seem to be called for remainder of test.

[Yoyo Zhou, 2011/11/21 23:14:52]

Well, if we ever have a test that uninstalls an extension, and then installs one
afterward using the helper methods, it would depend on this count. But we don't
actually have any such tests.

However, this just seems wrong. It should be --, not = 0.

[Aaron Boodman, 2011/11/22 10:02:59]

Fair enough.

 
     // We should get an unload notification.
     EXPECT_FALSE(unloaded_id_.empty());
     EXPECT_EQ(id, unloaded_id_);
 
     // Verify uninstalled state.
     size_t new_pref_key_count = GetPrefKeyCount();
     if (new_pref_key_count == pref_key_count) {
       ValidateIntegerPref(id, "location",
                           Extension::EXTERNAL_EXTENSION_UNINSTALLED);
@@ skipping 217 matching lines @@
   ASSERT_TRUE(file_util::PathExists(private_key_path));
 }
 
 // The tests are designed so that we never expect to see a packing error.
 void PackExtensionTestClient::OnPackFailure(const std::string& error_message) {
   FAIL() << "Packing should not fail.";
 }
 
 // Test loading good extensions from the profile directory.
 TEST_F(ExtensionServiceTest, LoadAllExtensionsFromDirectorySuccess) {
-  base::ShadowingAtExitManager at_exit_manager;
   PluginService::GetInstance()->Init();
 
   // Initialize the test dir with a good Preferences/extensions.
   FilePath source_install_dir = data_dir_
       .AppendASCII("good")
       .AppendASCII("Extensions");
   FilePath pref_path = source_install_dir
       .DirName()
       .AppendASCII("Preferences");
   InitializeInstalledExtensionService(pref_path, source_install_dir);
 
   service_->Init();
+  loop_.RunAllPending();

[Aaron Boodman, 2011/11/20 21:39:43]

Why is this important?

[Yoyo Zhou, 2011/11/21 23:14:52]

Not sure, it seems like the other tests that initialize ExtensionService like
this do it. Maybe they don't need it either since loading installed extensions
happens synchronously.

If I remove this from the rest of the tests, only CleanupOnStartup fails. So
I've removed it from the rest of the tests and annotated that one.

[Aaron Boodman, 2011/11/22 10:02:59]

In the old days ExtensionService startup was asynchronous, so this makes sense.
Cool. It makes sense that CleanupOnStartup needs this.

 
   uint32 expected_num_extensions = 3u;
   ASSERT_EQ(expected_num_extensions, loaded_.size());
 
   EXPECT_EQ(std::string(good0), loaded_[0]->id());
   EXPECT_EQ(std::string("My extension 1"),
             loaded_[0]->name());
   EXPECT_EQ(std::string("The first extension that I made."),
             loaded_[0]->description());
   EXPECT_EQ(Extension::INTERNAL, loaded_[0]->location());
@@ skipping 107 matching lines @@
       extension_manifest_errors::kMissingFile)) << GetErrors()[2];
 
   EXPECT_TRUE(MatchPattern(GetErrors()[3],
       std::string("Could not load extension from '*'. ") +
       extension_manifest_errors::kManifestUnreadable)) << GetErrors()[3];
 };
 
 // Test that partially deleted extensions are cleaned up during startup
 // Test loading bad extensions from the profile directory.
 TEST_F(ExtensionServiceTest, CleanupOnStartup) {
-  base::ShadowingAtExitManager at_exit_manager;
   PluginService::GetInstance()->Init();
 
   FilePath source_install_dir = data_dir_
       .AppendASCII("good")
       .AppendASCII("Extensions");
   FilePath pref_path = source_install_dir
       .DirName()
       .AppendASCII("Preferences");
 
   InitializeInstalledExtensionService(pref_path, source_install_dir);
@@ skipping 287 matching lines @@
 
   ExtensionAPIPermissionSet expected_api_perms;
   URLPatternSet expected_host_perms;
 
   // Make sure there aren't any granted permissions before the
   // extension is installed.
   scoped_refptr<ExtensionPermissionSet> known_perms(
       prefs->GetGrantedPermissions(permissions_crx));
   EXPECT_FALSE(known_perms.get());
 
-  PackAndInstallCrx(path, pem_path, true);
+  const Extension* extension = PackAndInstallCrx(path, pem_path, true);
 
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, service_->extensions()->size());
-  std::string extension_id = service_->extensions()->at(0)->id();
-  EXPECT_EQ(permissions_crx, extension_id);
+  EXPECT_EQ(permissions_crx, extension->id());
 
   // Verify that the valid API permissions have been recognized.
   expected_api_perms.insert(ExtensionAPIPermission::kTab);
 
   AddPattern(&expected_host_perms, "http://*.google.com/*");
   AddPattern(&expected_host_perms, "https://*.google.com/*");
   AddPattern(&expected_host_perms, "http://*.google.com.hk/*");
   AddPattern(&expected_host_perms, "http://www.example.com/*");
 
-  known_perms = prefs->GetGrantedPermissions(extension_id);
+  known_perms = prefs->GetGrantedPermissions(extension->id());
   EXPECT_TRUE(known_perms.get());
   EXPECT_FALSE(known_perms->IsEmpty());
   EXPECT_EQ(expected_api_perms, known_perms->apis());
   EXPECT_FALSE(known_perms->HasEffectiveFullAccess());
   EXPECT_EQ(expected_host_perms, known_perms->effective_hosts());
 }
 
 #if !defined(OS_CHROMEOS)
 // Tests that the granted permissions full_access bit gets set correctly when
 // an extension contains an NPAPI plugin. Don't run this test on Chrome OS
 // since they don't support plugins.
 TEST_F(ExtensionServiceTest, GrantedFullAccessPermissions) {
-  base::ShadowingAtExitManager at_exit_manager;
   PluginService::GetInstance()->Init();
 
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_
       .AppendASCII("good")
       .AppendASCII("Extensions")
       .AppendASCII(good1)
       .AppendASCII("2");
 
   ASSERT_TRUE(file_util::PathExists(path));
-  PackAndInstallCrx(path, true);
+  const Extension* extension = PackAndInstallCrx(path, true);
   EXPECT_EQ(0u, GetErrors().size());
   EXPECT_EQ(1u, service_->extensions()->size());
-  const Extension* extension = service_->extensions()->at(0);
-  std::string extension_id = extension->id();
   ExtensionPrefs* prefs = service_->extension_prefs();
 
   scoped_refptr<ExtensionPermissionSet> permissions(
-      prefs->GetGrantedPermissions(extension_id));
+      prefs->GetGrantedPermissions(extension->id()));
   EXPECT_FALSE(permissions->IsEmpty());
   EXPECT_TRUE(permissions->HasEffectiveFullAccess());
   EXPECT_FALSE(permissions->apis().empty());
   EXPECT_TRUE(permissions->HasAPIPermission(ExtensionAPIPermission::kPlugin));
 
   // Full access implies full host access too...
   EXPECT_TRUE(permissions->HasEffectiveAccessToAllHosts());
 }
 #endif
 
 // Tests that the extension is disabled when permissions are missing from
 // the extension's granted permissions preferences. (This simulates updating
 // the browser to a version which recognizes more permissions).
 TEST_F(ExtensionServiceTest, GrantedAPIAndHostPermissions) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_
       .AppendASCII("permissions")
       .AppendASCII("unknown");
 
   ASSERT_TRUE(file_util::PathExists(path));
 
-  PackAndInstallCrx(path, true);
+  const Extension* extension = PackAndInstallCrx(path, true);
 
   EXPECT_EQ(0u, GetErrors().size());
   EXPECT_EQ(1u, service_->extensions()->size());
-  const Extension* extension = service_->extensions()->at(0);
   std::string extension_id = extension->id();
 
   ExtensionPrefs* prefs = service_->extension_prefs();
 
   ExtensionAPIPermissionSet expected_api_permissions;
   URLPatternSet expected_host_permissions;
 
   expected_api_permissions.insert(ExtensionAPIPermission::kTab);
   AddPattern(&expected_host_permissions, "http://*.google.com/*");
   AddPattern(&expected_host_permissions, "https://*.google.com/*");
   AddPattern(&expected_host_permissions, "http://*.google.com.hk/*");
   AddPattern(&expected_host_permissions, "http://www.example.com/*");
 
   std::set<std::string> host_permissions;
 
   // Test that the extension is disabled when an API permission is missing from
   // the extension's granted api permissions preference. (This simulates
   // updating the browser to a version which recognizes a new API permission).
   SetPref(extension_id, "granted_permissions.api",
           new ListValue(), "granted_permissions.api");
   service_->ReloadExtensions();
 
   EXPECT_EQ(1u, service_->disabled_extensions()->size());
-  extension = service_->disabled_extensions()->at(0);
+  extension = *service_->disabled_extensions()->begin();
 
   ASSERT_TRUE(prefs->IsExtensionDisabled(extension_id));
   ASSERT_FALSE(service_->IsExtensionEnabled(extension_id));
   ASSERT_TRUE(prefs->DidExtensionEscalatePermissions(extension_id));
 
   // Now grant and re-enable the extension, making sure the prefs are updated.
   service_->GrantPermissionsAndEnableExtension(extension);
 
   ASSERT_FALSE(prefs->IsExtensionDisabled(extension_id));
   ASSERT_TRUE(service_->IsExtensionEnabled(extension_id));
@@ skipping 22 matching lines @@
   api_permissions->Append(
       Value::CreateIntegerValue(ExtensionAPIPermission::kTab));
   SetPref(extension_id, "granted_permissions.api",
           api_permissions, "granted_permissions.api");
   SetPrefStringSet(
       extension_id, "granted_permissions.scriptable_host", host_permissions);
 
   service_->ReloadExtensions();
 
   EXPECT_EQ(1u, service_->disabled_extensions()->size());
-  extension = service_->disabled_extensions()->at(0);
+  extension = *service_->disabled_extensions()->begin();
 
   ASSERT_TRUE(prefs->IsExtensionDisabled(extension_id));
   ASSERT_FALSE(service_->IsExtensionEnabled(extension_id));
   ASSERT_TRUE(prefs->DidExtensionEscalatePermissions(extension_id));
 
   // Now grant and re-enable the extension, making sure the prefs are updated.
   service_->GrantPermissionsAndEnableExtension(extension);
 
   ASSERT_TRUE(service_->IsExtensionEnabled(extension_id));
   ASSERT_FALSE(prefs->DidExtensionEscalatePermissions(extension_id));
@@ skipping 189 matching lines @@
   // Load.
   InitializeEmptyExtensionService();
   FilePath extension_path = data_dir_
       .AppendASCII("theme_i18n");
 
   extensions::UnpackedInstaller::Create(service_)->Load(extension_path);
   loop_.RunAllPending();
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
   EXPECT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ("name", service_->extensions()->at(0)->name());
-  EXPECT_EQ("description", service_->extensions()->at(0)->description());
+  const Extension* theme = *service_->extensions()->begin();
+  EXPECT_EQ("name", theme->name());
+  EXPECT_EQ("description", theme->description());
 }
 
 TEST_F(ExtensionServiceTest, InstallLocalizedTheme) {
   InitializeEmptyExtensionService();
   FilePath theme_path = data_dir_
       .AppendASCII("theme_i18n");
 
-  PackAndInstallCrx(theme_path, true);
+  const Extension* theme = PackAndInstallCrx(theme_path, true);
 
   EXPECT_EQ(0u, GetErrors().size());
   EXPECT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ("name", service_->extensions()->at(0)->name());
-  EXPECT_EQ("description", service_->extensions()->at(0)->description());
+  EXPECT_EQ("name", theme->name());
+  EXPECT_EQ("description", theme->description());
 }
 
 TEST_F(ExtensionServiceTest, InstallApps) {
   InitializeEmptyExtensionService();
 
   // An empty app.
-  PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
+  const Extension* app = PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
   int pref_count = 0;
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(1u, service_->extensions()->size());
-  std::string id = service_->extensions()->at(0)->id();
-  ValidateIntegerPref(id, "state", Extension::ENABLED);
-  ValidateIntegerPref(id, "location", Extension::INTERNAL);
+  ValidateIntegerPref(app->id(), "state", Extension::ENABLED);
+  ValidateIntegerPref(app->id(), "location", Extension::INTERNAL);
 
   // Another app with non-overlapping extent. Should succeed.
   PackAndInstallCrx(data_dir_.AppendASCII("app2"), true);
   ValidatePrefKeyCount(++pref_count);
 
   // A third app whose extent overlaps the first. Should fail.
   PackAndInstallCrx(data_dir_.AppendASCII("app3"), false);
   ValidatePrefKeyCount(pref_count);
 }
 
 // Tests that file access is OFF by default.
 TEST_F(ExtensionServiceTest, DefaultFileAccess) {
   InitializeEmptyExtensionService();
-  PackAndInstallCrx(data_dir_.AppendASCII("permissions").AppendASCII("files"),
-                    true);
-
+  const Extension* extension =
+      PackAndInstallCrx(data_dir_
+                        .AppendASCII("permissions")
+                        .AppendASCII("files"),
+                        true);
   EXPECT_EQ(0u, GetErrors().size());
   EXPECT_EQ(1u, service_->extensions()->size());
-  std::string id = service_->extensions()->at(0)->id();
-  EXPECT_FALSE(service_->extension_prefs()->AllowFileAccess(id));
+  EXPECT_FALSE(service_->extension_prefs()->AllowFileAccess(extension->id()));
 }
 
 TEST_F(ExtensionServiceTest, UpdateApps) {
   InitializeEmptyExtensionService();
   FilePath extensions_path = data_dir_.AppendASCII("app_update");
 
   // First install v1 of a hosted app.
-  InstallCrx(extensions_path.AppendASCII("v1.crx"), true);
+  const Extension* extension =
+      InstallCrx(extensions_path.AppendASCII("v1.crx"), true);
   ASSERT_EQ(1u, service_->extensions()->size());
-  std::string id = service_->extensions()->at(0)->id();
-  ASSERT_EQ(std::string("1"),
-            service_->extensions()->at(0)->version()->GetString());
+  std::string id = extension->id();
+  ASSERT_EQ(std::string("1"), extension->version()->GetString());
 
   // Now try updating to v2.
   UpdateExtension(id,
                   extensions_path.AppendASCII("v2.crx"),
                   ENABLED);
   ASSERT_EQ(std::string("2"),
-            service_->extensions()->at(0)->version()->GetString());
+            service_->GetExtensionById(id, false)->version()->GetString());
 }
 
-TEST_F(ExtensionServiceTest, InstallAppsWithUnlimtedStorage) {
+TEST_F(ExtensionServiceTest, InstallAppsWithUnlimitedStorage) {
   InitializeEmptyExtensionService();
   InitializeRequestContext();
   EXPECT_TRUE(service_->extensions()->empty());
 
   int pref_count = 0;
 
   // Install app1 with unlimited storage.
-  PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
+  const Extension* extension =
+      PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(1u, service_->extensions()->size());
-  const Extension* extension = service_->extensions()->at(0);
   const std::string id1 = extension->id();
   EXPECT_TRUE(extension->HasAPIPermission(
       ExtensionAPIPermission::kUnlimitedStorage));
   EXPECT_TRUE(extension->web_extent().MatchesURL(
                   extension->GetFullLaunchURL()));
   const GURL origin1(extension->GetFullLaunchURL().GetOrigin());
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageUnlimited(origin1));
 
   // Install app2 from the same origin with unlimited storage.
-  PackAndInstallCrx(data_dir_.AppendASCII("app2"), true);
+  extension = PackAndInstallCrx(data_dir_.AppendASCII("app2"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(2u, service_->extensions()->size());
-  extension = service_->extensions()->at(1);
   const std::string id2 = extension->id();
   EXPECT_TRUE(extension->HasAPIPermission(
       ExtensionAPIPermission::kUnlimitedStorage));
   EXPECT_TRUE(extension->web_extent().MatchesURL(
                   extension->GetFullLaunchURL()));
   const GURL origin2(extension->GetFullLaunchURL().GetOrigin());
   EXPECT_EQ(origin1, origin2);
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageUnlimited(origin2));
 
@@ skipping 13 matching lines @@
       IsStorageUnlimited(origin2));
 }
 
 TEST_F(ExtensionServiceTest, InstallAppsAndCheckStorageProtection) {
   InitializeEmptyExtensionService();
   InitializeRequestContext();
   EXPECT_TRUE(service_->extensions()->empty());
 
   int pref_count = 0;
 
-  PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
+  const Extension* extension =
+      PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(1u, service_->extensions()->size());
-  const Extension* extension = service_->extensions()->at(0);
   EXPECT_TRUE(extension->is_app());
   const std::string id1 = extension->id();
   const GURL origin1(extension->GetFullLaunchURL().GetOrigin());
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageProtected(origin1));
 
   // App 4 has a different origin (maps.google.com).
-  PackAndInstallCrx(data_dir_.AppendASCII("app4"), true);
+  extension = PackAndInstallCrx(data_dir_.AppendASCII("app4"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(2u, service_->extensions()->size());
-  extension = service_->extensions()->at(1);
   const std::string id2 = extension->id();
   const GURL origin2(extension->GetFullLaunchURL().GetOrigin());
   ASSERT_NE(origin1, origin2);
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageProtected(origin2));
 
   UninstallExtension(id1, false);
   EXPECT_EQ(1u, service_->extensions()->size());
 
   UninstallExtension(id2, false);
 
   EXPECT_TRUE(service_->extensions()->empty());
   EXPECT_FALSE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageProtected(origin1));
   EXPECT_FALSE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageProtected(origin2));
 }
 
 // Test that when an extension version is reinstalled, nothing happens.
 TEST_F(ExtensionServiceTest, Reinstall) {
   InitializeEmptyExtensionService();
 
   // A simple extension that should install without error.
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  InstallCrx(path, true);
 
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(0u, GetErrors().size());
   ValidatePrefKeyCount(1);
   ValidateIntegerPref(good_crx, "state", Extension::ENABLED);
   ValidateIntegerPref(good_crx, "location", Extension::INTERNAL);
 
-  installed_ = NULL;
-  loaded_.clear();
-  ExtensionErrorReporter::GetInstance()->ClearErrors();
+  // Reinstall the same version, it should overwrite the previous one.
+  expected_extensions_count_--;
+  InstallCrx(path, true);
 
-  // Reinstall the same version, it should overwrite the previous one.
-  StartCrxInstall(path);
-  loop_.RunAllPending();
-
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(0u, GetErrors().size());
   ValidatePrefKeyCount(1);
   ValidateIntegerPref(good_crx, "state", Extension::ENABLED);
   ValidateIntegerPref(good_crx, "location", Extension::INTERNAL);
 }
 
 // Test that we can determine if extensions came from the
 // Chrome web store.
 TEST_F(ExtensionServiceTest, FromWebStore) {
   InitializeEmptyExtensionService();
 
   // A simple extension that should install without error.
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path, false);  // Not from web store.
-  loop_.RunAllPending();
+  const Extension* extension = InstallCrx(path, true);  // Not from web store.
+  std::string id = extension->id();
 
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(0u, GetErrors().size());
   ValidatePrefKeyCount(1);
   ValidateBooleanPref(good_crx, "from_webstore", false);
-
-  const Extension* extension = service_->extensions()->at(0);
   ASSERT_FALSE(extension->from_webstore());
 
-  installed_ = NULL;
-  loaded_.clear();
-  ExtensionErrorReporter::GetInstance()->ClearErrors();
+  // Test install from web store.
+  expected_extensions_count_--;
+  InstallCrxFromWebStore(path, true);  // From web store.
 
-  // Test install from web store.
-  StartCrxInstall(path, true);  // From web store.
-  loop_.RunAllPending();
-
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(0u, GetErrors().size());
   ValidatePrefKeyCount(1);
   ValidateBooleanPref(good_crx, "from_webstore", true);
 
   // Reload so extension gets reinitialized with new value.
   service_->ReloadExtensions();
-  extension = service_->extensions()->at(0);
+  extension = service_->GetExtensionById(id, false);
   ASSERT_TRUE(extension->from_webstore());
 
   // Upgrade to version 2.0
   path = data_dir_.AppendASCII("good2.crx");
   UpdateExtension(good_crx, path, ENABLED);
   ValidatePrefKeyCount(1);
   ValidateBooleanPref(good_crx, "from_webstore", true);
 }
 
 // Test upgrading a signed extension.
 TEST_F(ExtensionServiceTest, UpgradeSignedGood) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  const Extension* extension = InstallCrx(path, true);
+  std::string id = extension->id();
 
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ("1.0.0.0", loaded_[0]->version()->GetString());
+  ASSERT_EQ("1.0.0.0", extension->version()->GetString());
   ASSERT_EQ(0u, GetErrors().size());
 
-  // Upgrade to version 2.0
+  // Upgrade to version 1.0.0.1
   path = data_dir_.AppendASCII("good2.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  expected_extensions_count_--;

[Aaron Boodman, 2011/11/20 21:39:43]

A little weird to directly modify the count in all these update cases. What if
instead, InstallCrx took an enum describing expected behavior (e.g.,
SUCCEED_NEW, SUCCEED_UPDATE, FAIL).

[Yoyo Zhou, 2011/11/21 23:14:52]

Yeah, I was looking at doing a similar thing to extension_browsertest and it
seems to be better that way.

Unfortunately the ideal names seem as though they will conflict with the
existing UpdateState.

+  InstallCrx(path, true);
+  extension = service_->GetExtensionById(id, false);
 
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
+  ASSERT_EQ("1.0.0.1", extension->version()->GetString());
   ASSERT_EQ(0u, GetErrors().size());
 }
 
 // Test upgrading a signed extension with a bad signature.
 TEST_F(ExtensionServiceTest, UpgradeSignedBad) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
-
-  ASSERT_TRUE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(0u, GetErrors().size());
-  installed_ = NULL;
+  InstallCrx(path, true);
 
   // Try upgrading with a bad signature. This should fail during the unpack,
   // because the key will not match the signature.
   path = data_dir_.AppendASCII("bad_signature.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
-
-  ASSERT_FALSE(installed_);
-  ASSERT_EQ(1u, loaded_.size());
-  ASSERT_EQ(1u, GetErrors().size());
+  InstallCrx(path, false);
 }
 
 // Test a normal update via the UpdateExtension API
 TEST_F(ExtensionServiceTest, UpdateExtension) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
   ASSERT_EQ("1.0.0.0", good->VersionString());
   ASSERT_EQ(good_crx, good->id());
 
   path = data_dir_.AppendASCII("good2.crx");
   UpdateExtension(good_crx, path, ENABLED);
-  ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
+  ASSERT_EQ("1.0.0.1",
+            service_->GetExtensionById(good_crx, false)->
+            version()->GetString());
 }
 
 // Test updating a not-already-installed extension - this should fail
 TEST_F(ExtensionServiceTest, UpdateNotInstalledExtension) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
   UpdateExtension(good_crx, path, UPDATED);
   loop_.RunAllPending();
 
   ASSERT_EQ(0u, service_->extensions()->size());
   ASSERT_FALSE(installed_);
   ASSERT_EQ(0u, loaded_.size());
 }
 
 // Makes sure you can't downgrade an extension via UpdateExtension
 TEST_F(ExtensionServiceTest, UpdateWillNotDowngrade) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good2.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
   ASSERT_EQ("1.0.0.1", good->VersionString());
   ASSERT_EQ(good_crx, good->id());
 
   // Change path from good2.crx -> good.crx
   path = data_dir_.AppendASCII("good.crx");
   UpdateExtension(good_crx, path, FAILED);
-  ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString());
+  ASSERT_EQ("1.0.0.1",
+            service_->GetExtensionById(good_crx, false)->
+            version()->GetString());
 }
 
 // Make sure calling update with an identical version does nothing
 TEST_F(ExtensionServiceTest, UpdateToSameVersionIsNoop) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
   ASSERT_EQ(good_crx, good->id());
   UpdateExtension(good_crx, path, FAILED_SILENTLY);
 }
 
 // Tests that updating an extension does not clobber old state.
 TEST_F(ExtensionServiceTest, UpdateExtensionPreservesState) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
   ASSERT_EQ("1.0.0.0", good->VersionString());
   ASSERT_EQ(good_crx, good->id());
 
   // Disable it and allow it to run in incognito. These settings should carry
   // over to the updated version.
   service_->DisableExtension(good->id());
   service_->SetIsIncognitoEnabled(good->id(), true);
 
   path = data_dir_.AppendASCII("good2.crx");
   UpdateExtension(good_crx, path, INSTALLED);
   ASSERT_EQ(1u, service_->disabled_extensions()->size());
-  const Extension* good2 = service_->disabled_extensions()->at(0);
+  const Extension* good2 = service_->GetExtensionById(good_crx, true);
   ASSERT_EQ("1.0.0.1", good2->version()->GetString());
   EXPECT_TRUE(service_->IsIncognitoEnabled(good2->id()));
 }
 
 // Tests that updating preserves extension location.
 TEST_F(ExtensionServiceTest, UpdateExtensionPreservesLocation) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
 
   ASSERT_EQ("1.0.0.0", good->VersionString());
   ASSERT_EQ(good_crx, good->id());
 
   // Simulate non-internal location.
   const_cast<Extension*>(good)->location_ = Extension::EXTERNAL_PREF;
 
   path = data_dir_.AppendASCII("good2.crx");
   UpdateExtension(good_crx, path, ENABLED);
-  const Extension* good2 = service_->extensions()->at(0);
+  const Extension* good2 = service_->GetExtensionById(good_crx, false);
   ASSERT_EQ("1.0.0.1", good2->version()->GetString());
   EXPECT_EQ(good2->location(), Extension::EXTERNAL_PREF);
 }
 
 // Makes sure that LOAD extension types can downgrade.
 TEST_F(ExtensionServiceTest, LoadExtensionsCanDowngrade) {
   InitializeEmptyExtensionService();
 
   ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
@@ skipping 238 matching lines @@
 
   EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId));
 }
 
 // Test updating a pending extension for one that is already
 // installed.
 TEST_F(ExtensionServiceTest, UpdatePendingExtensionAlreadyInstalled) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
-  InstallCrx(path, true);
+  const Extension* good = InstallCrx(path, true);
   ASSERT_EQ(1u, service_->extensions()->size());
-  const Extension* good = service_->extensions()->at(0);
 
   EXPECT_FALSE(good->is_theme());
 
   // Use AddExtensionImpl() as AddFrom*() would balk.
   service_->pending_extension_manager()->AddExtensionImpl(
       good->id(), good->update_url(), &IsExtension,
       kGoodIsFromSync, kGoodInstallSilently, Extension::INTERNAL);
   UpdateExtension(good->id(), path, ENABLED);
 
   EXPECT_FALSE(service_->pending_extension_manager()->IsIdPending(kGoodId));
@@ skipping 27 matching lines @@
   ValidateBooleanPref(good2, "blacklist", true);
   EXPECT_FALSE(IsPrefExist("invalid_id", "blacklist"));
 }
 
 // Unload installed extension from blacklist.
 TEST_F(ExtensionServiceTest, UnloadBlacklistedExtension) {
   InitializeEmptyExtensionService();
 
   FilePath path = data_dir_.AppendASCII("good.crx");
 
-  InstallCrx(path, true);
-  const Extension* good = service_->extensions()->at(0);
+  const Extension* good = InstallCrx(path, true);
   EXPECT_EQ(good_crx, good->id());
   UpdateExtension(good_crx, path, FAILED_SILENTLY);
 
   std::vector<std::string> blacklist;
   blacklist.push_back(good_crx);
   service_->UpdateExtensionBlacklist(blacklist);
   // Make sure pref is updated
   loop_.RunAllPending();
 
   // Now, the good_crx is blacklisted.
@@ skipping 16 matching lines @@
   blacklist.push_back(good_crx);
   service_->UpdateExtensionBlacklist(blacklist);
   // Make sure pref is updated
   loop_.RunAllPending();
 
   // Now, the good_crx is blacklisted.
   ValidateBooleanPref(good_crx, "blacklist", true);
 
   // We can not install good_crx.
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  InstallCrx(path, false);
   EXPECT_EQ(0u, service_->extensions()->size());
   ValidateBooleanPref(good_crx, "blacklist", true);
 }
 
 // Test loading extensions from the profile directory, except
 // blacklisted ones.
 TEST_F(ExtensionServiceTest, WillNotLoadBlacklistedExtensionsFromDirectory) {
   // Initialize the test dir with a good Preferences/extensions.
   FilePath source_install_dir = data_dir_
       .AppendASCII("good")
@@ skipping 16 matching lines @@
   service_->Init();
   loop_.RunAllPending();
 
   std::vector<std::string> errors = GetErrors();
   for (std::vector<std::string>::iterator err = errors.begin();
     err != errors.end(); ++err) {
     LOG(ERROR) << *err;
   }
   ASSERT_EQ(2u, loaded_.size());
 
-  EXPECT_NE(std::string(good1), loaded_[0]->id());
-  EXPECT_NE(std::string(good1), loaded_[1]->id());
+  EXPECT_FALSE(service_->GetExtensionById(good1, true));
 }
 
 // Will not install extension blacklisted by policy.
 TEST_F(ExtensionServiceTest, BlacklistedByPolicyWillNotInstall) {
   InitializeEmptyExtensionService();
 
   // Blacklist everything.
   {
     ListPrefUpdate update(profile_->GetPrefs(),
                           prefs::kExtensionInstallDenyList);
     ListValue* blacklist = update.Get();
     blacklist->Append(Value::CreateStringValue("*"));
   }
 
   // Blacklist prevents us from installing good_crx.
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  InstallCrx(path, false);
   EXPECT_EQ(0u, service_->extensions()->size());
 
   // Now whitelist this particular extension.
   {
     ListPrefUpdate update(profile_->GetPrefs(),
                           prefs::kExtensionInstallAllowList);
     ListValue* whitelist = update.Get();
     whitelist->Append(Value::CreateStringValue(good_crx));
   }
 
-
   // Ensure we can now install good_crx.
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  InstallCrx(path, true);
   EXPECT_EQ(1u, service_->extensions()->size());
 }
 
 // Extension blacklisted by policy get unloaded after installing.
 TEST_F(ExtensionServiceTest, BlacklistedByPolicyRemovedIfRunning) {
   InitializeEmptyExtensionService();
 
   // Install good_crx.
   FilePath path = data_dir_.AppendASCII("good.crx");
-  StartCrxInstall(path);
-  loop_.RunAllPending();
+  InstallCrx(path, true);
   EXPECT_EQ(1u, service_->extensions()->size());
 
   { // Scope for pref update notification.
     PrefService* prefs = profile_->GetPrefs();
     ListPrefUpdate update(prefs, prefs::kExtensionInstallDenyList);
     ListValue* blacklist = update.Get();
     ASSERT_TRUE(blacklist != NULL);
 
     // Blacklist this extension.
     blacklist->Append(Value::CreateStringValue(good_crx));
@@ skipping 24 matching lines @@
       .AppendASCII(good0)
       .AppendASCII("1.0.0.0");
   std::string manifest;
   ASSERT_TRUE(file_util::ReadFileToString(
       path.Append(Extension::kManifestFilename), &manifest));
   service_->component_loader()->Add(manifest, path);
   service_->Init();
 
   // Extension should be installed despite blacklist.
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+  EXPECT_TRUE(service_->GetExtensionById(good0, false));
 
   // Poke external providers and make sure the extension is still present.
   service_->CheckForExternalUpdates();
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+  EXPECT_TRUE(service_->GetExtensionById(good0, false));
 
   // Extension should not be uninstalled on blacklist changes.
   {
     ListPrefUpdate update(profile_->GetPrefs(),
                           prefs::kExtensionInstallDenyList);
     ListValue* blacklist = update.Get();
     blacklist->Append(Value::CreateStringValue(good0));
   }
   loop_.RunAllPending();
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(good0, service_->extensions()->at(0)->id());
+  EXPECT_TRUE(service_->GetExtensionById(good0, false));
 }
 
 // Tests that policy-installed extensions are not blacklisted by policy.
 TEST_F(ExtensionServiceTest, PolicyInstalledExtensionsWhitelisted) {
   InitializeEmptyExtensionService();
 
   // Blacklist everything.
   {
     ListPrefUpdate update(profile_->GetPrefs(),
                           prefs::kExtensionInstallDenyList);
     ListValue* blacklist = update.Get();
     blacklist->Append(Value::CreateStringValue("*"));
   }
 
   // Have policy force-install an extension.
   MockExtensionProvider* provider =
       new MockExtensionProvider(service_,
                                 Extension::EXTERNAL_POLICY_DOWNLOAD);
   AddMockExternalProvider(provider);
   provider->UpdateOrAddExtension(good_crx, "1.0.0.0",
                                  data_dir_.AppendASCII("good.crx"));
 
   // Reloading extensions should find our externally registered extension
   // and install it.
   service_->CheckForExternalUpdates();
   loop_.RunAllPending();
 
   // Extension should be installed despite blacklist.
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(good_crx, service_->extensions()->at(0)->id());
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, false));
 
   // Blacklist update should not uninstall the extension.
   {
     ListPrefUpdate update(profile_->GetPrefs(),
                           prefs::kExtensionInstallDenyList);
     ListValue* blacklist = update.Get();
     blacklist->Append(Value::CreateStringValue(good0));
   }
   loop_.RunAllPending();
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(good_crx, service_->extensions()->at(0)->id());
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, false));
 }
 
 // Tests disabling extensions
 TEST_F(ExtensionServiceTest, DisableExtension) {
   InitializeEmptyExtensionService();
 
   InstallCrx(data_dir_.AppendASCII("good.crx"), true);
   EXPECT_FALSE(service_->extensions()->empty());
-  EXPECT_TRUE(service_->GetExtensionById(good_crx, true) != NULL);
-  EXPECT_TRUE(service_->GetExtensionById(good_crx, false) != NULL);
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, true));
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, false));
   EXPECT_TRUE(service_->disabled_extensions()->empty());
 
   // Disable it.
   service_->DisableExtension(good_crx);
 
   EXPECT_TRUE(service_->extensions()->empty());
-  EXPECT_TRUE(service_->GetExtensionById(good_crx, true) != NULL);
-  EXPECT_FALSE(service_->GetExtensionById(good_crx, false) != NULL);
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, true));
+  EXPECT_FALSE(service_->GetExtensionById(good_crx, false));
   EXPECT_FALSE(service_->disabled_extensions()->empty());
 }
 
 TEST_F(ExtensionServiceTest, DisableTerminatedExtension) {
   InitializeEmptyExtensionService();
 
   InstallCrx(data_dir_.AppendASCII("good.crx"), true);
   TerminateExtension(good_crx);
   EXPECT_TRUE(service_->GetTerminatedExtension(good_crx));
 
   // Disable it.
   service_->DisableExtension(good_crx);
 
   EXPECT_FALSE(service_->GetTerminatedExtension(good_crx));
-  EXPECT_TRUE(service_->GetExtensionById(good_crx, true) != NULL);
+  EXPECT_TRUE(service_->GetExtensionById(good_crx, true));
   EXPECT_FALSE(service_->disabled_extensions()->empty());
 }
 
 // Tests disabling all extensions (simulating --disable-extensions flag).
 TEST_F(ExtensionServiceTest, DisableAllExtensions) {
   InitializeEmptyExtensionService();
 
-
   FilePath path = data_dir_.AppendASCII("good.crx");
   InstallCrx(path, true);
 
   EXPECT_EQ(1u, service_->extensions()->size());
   EXPECT_EQ(0u, service_->disabled_extensions()->size());
 
   // Disable extensions.
   service_->set_extensions_enabled(false);
   service_->ReloadExtensions();
 
@@ skipping 10 matching lines @@
   EXPECT_EQ(0u, service_->disabled_extensions()->size());
 
   // And then re-enable the extensions.
   service_->set_extensions_enabled(true);
   service_->ReloadExtensions();
 
   EXPECT_EQ(1u, service_->extensions()->size());
   EXPECT_EQ(0u, service_->disabled_extensions()->size());
 }
 
-// Tests reloading extensions
+// Tests reloading extensions.
 TEST_F(ExtensionServiceTest, ReloadExtensions) {
   InitializeEmptyExtensionService();
 
   // Simple extension that should install without error.
   FilePath path = data_dir_.AppendASCII("good.crx");
   InstallCrx(path, true);
   const char* extension_id = good_crx;
   service_->DisableExtension(extension_id);
 
   EXPECT_EQ(0u, service_->extensions()->size());
@@ skipping 80 matching lines @@
 };
 
 // Verifies extension state is removed upon uninstall.
 TEST_F(ExtensionServiceTest, ClearExtensionData) {
   InitializeEmptyExtensionService();
   ExtensionCookieCallback callback;
 
   // Load a test extension.
   FilePath path = data_dir_;
   path = path.AppendASCII("good.crx");
-  InstallCrx(path, true);
-  const Extension* extension = service_->GetExtensionById(good_crx, false);
+  const Extension* extension = InstallCrx(path, true);
   ASSERT_TRUE(extension);
   GURL ext_url(extension->url());
   string16 origin_id =
       webkit_database::DatabaseUtil::GetOriginIdentifier(ext_url);
 
   // Set a cookie for the extension.
   net::CookieMonster* cookie_monster =
       profile_->GetRequestContextForExtensions()->GetURLRequestContext()->
       cookie_store()->GetCookieMonster();
   ASSERT_TRUE(cookie_monster);
@@ skipping 67 matching lines @@
 
 // Verifies app state is removed upon uninstall.
 TEST_F(ExtensionServiceTest, ClearAppData) {
   InitializeEmptyExtensionService();
   InitializeRequestContext();
   ExtensionCookieCallback callback;
 
   int pref_count = 0;
 
   // Install app1 with unlimited storage.
-  PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
+  const Extension* extension =
+      PackAndInstallCrx(data_dir_.AppendASCII("app1"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(1u, service_->extensions()->size());
-  const Extension* extension = service_->extensions()->at(0);
   const std::string id1 = extension->id();
   EXPECT_TRUE(extension->HasAPIPermission(
       ExtensionAPIPermission::kUnlimitedStorage));
   const GURL origin1(extension->GetFullLaunchURL().GetOrigin());
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageUnlimited(origin1));
   string16 origin_id =
       webkit_database::DatabaseUtil::GetOriginIdentifier(origin1);
 
   // Install app2 from the same origin with unlimited storage.
-  PackAndInstallCrx(data_dir_.AppendASCII("app2"), true);
+  extension = PackAndInstallCrx(data_dir_.AppendASCII("app2"), true);
   ValidatePrefKeyCount(++pref_count);
   ASSERT_EQ(2u, service_->extensions()->size());
-  extension = service_->extensions()->at(1);
   const std::string id2 = extension->id();
   EXPECT_TRUE(extension->HasAPIPermission(
       ExtensionAPIPermission::kUnlimitedStorage));
   EXPECT_TRUE(extension->web_extent().MatchesURL(
                   extension->GetFullLaunchURL()));
   const GURL origin2(extension->GetFullLaunchURL().GetOrigin());
   EXPECT_EQ(origin1, origin2);
   EXPECT_TRUE(profile_->GetExtensionSpecialStoragePolicy()->
       IsStorageUnlimited(origin2));
 
@@ skipping 124 matching lines @@
   EXPECT_EQ(id, unloaded_id_);
   ASSERT_EQ(0u, loaded_.size());
   EXPECT_EQ(0u, service_->extensions()->size());
 }
 
 // Tests that we generate IDs when they are not specified in the manifest for
 // --load-extension.
 TEST_F(ExtensionServiceTest, GenerateID) {
   InitializeEmptyExtensionService();
 
-
   FilePath no_id_ext = data_dir_.AppendASCII("no_id");
   extensions::UnpackedInstaller::Create(service_)->Load(no_id_ext);
   loop_.RunAllPending();
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
   ASSERT_TRUE(Extension::IdIsValid(loaded_[0]->id()));
   EXPECT_EQ(loaded_[0]->location(), Extension::LOAD);
 
   ValidatePrefKeyCount(1);
 
@@ skipping 621 matching lines @@
 
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
   EXPECT_EQ(Extension::COMPONENT, loaded_[0]->location());
   EXPECT_EQ(1u, service_->extensions()->size());
 
   // Component extensions shouldn't get recorded in the prefs.
   ValidatePrefKeyCount(0);
 
   // Reload all extensions, and make sure it comes back.
-  std::string extension_id = service_->extensions()->at(0)->id();
+  std::string extension_id = (*service_->extensions()->begin())->id();
   loaded_.clear();
   service_->ReloadExtensions();
   ASSERT_EQ(1u, service_->extensions()->size());
-  EXPECT_EQ(extension_id, service_->extensions()->at(0)->id());
+  EXPECT_EQ(extension_id, (*service_->extensions()->begin())->id());
 }
 
 namespace {
   class TestSyncProcessorStub : public SyncChangeProcessor {
     virtual SyncError ProcessSyncChanges(
         const tracked_objects::Location& from_here,
         const SyncChangeList& change_list) OVERRIDE {
       return SyncError();
     }
   };
@@ skipping 553 matching lines @@
   ASSERT_FALSE(AddPendingSyncInstall());
 
   // Wait for the external source to install.
   WaitForCrxInstall(crx_path_, true);
   ASSERT_TRUE(IsCrxInstalled());
 
   // Now that the extension is installed, sync request should fail
   // because the extension is already installed.
   ASSERT_FALSE(AddPendingSyncInstall());
 }