Reworked ExtensionsService::AddPendingExtension().

Made it take options for enabled/incognito enabled state to apply
on install.

BUG=46515
TEST=unittests, manual (newly-synced disabled extensions should stay
disabled)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50804

Review URL: http://codereview.chromium.org/2819023

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

10 files changed, 225 insertions, 124 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index ce0f37d..d3494da 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -81,7 +81,8 @@ bool ExtensionBrowserTest::LoadExtensionImpl(const FilePath& path,
     // OnExtensionInstalled ensures the other extension prefs are set up with
     // the defaults.
     Extension* extension = service->extensions()->at(num_after - 1);
-    service->extension_prefs()->OnExtensionInstalled(extension);
+    service->extension_prefs()->OnExtensionInstalled(
+        extension, Extension::ENABLED, false);
     service->SetIsIncognitoEnabled(extension, true);
   }
 
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 0f9661c..93f29c4 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -444,13 +444,14 @@ void ExtensionPrefs::SetToolbarOrder(
   prefs_->ScheduleSavePersistentPrefs();
 }
 
-void ExtensionPrefs::OnExtensionInstalled(Extension* extension) {
+void ExtensionPrefs::OnExtensionInstalled(
+    Extension* extension, Extension::State initial_state,
+    bool initial_incognito_enabled) {
   const std::string& id = extension->id();
-  // Make sure we don't enable a disabled extension.
-  if (GetExtensionState(extension->id()) != Extension::DISABLED) {
-    UpdateExtensionPref(id, kPrefState,
-                        Value::CreateIntegerValue(Extension::ENABLED));
-  }
+  UpdateExtensionPref(id, kPrefState,
+                      Value::CreateIntegerValue(initial_state));
+  UpdateExtensionPref(id, kPrefIncognitoEnabled,
+                      Value::CreateBooleanValue(initial_incognito_enabled));
   UpdateExtensionPref(id, kPrefLocation,
                       Value::CreateIntegerValue(extension->location()));
   FilePath::StringType path = MakePathRelative(install_directory_,
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 77487e9..f7733c3 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -47,7 +47,9 @@ class ExtensionPrefs {
   void SetToolbarOrder(const std::vector<std::string>& extension_ids);
 
   // Called when an extension is installed, so that prefs get created.
-  void OnExtensionInstalled(Extension* extension);
+  void OnExtensionInstalled(Extension* extension,
+                            Extension::State initial_state,
+                            bool initial_incognito_enabled);
 
   // Called when an extension is uninstalled, so that prefs get cleaned up.
   void OnExtensionUninstalled(const std::string& extension_id,
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index 5f45ad5..26df5b1 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -345,3 +345,26 @@ class ExtensionPrefsAppToolbars : public ExtensionPrefsTest {
   std::string extension_id_overridden_off_;
 };
 TEST_F(ExtensionPrefsAppToolbars, ExtensionPrefsAppToolbars) {}
+
+class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest {
+ public:
+  virtual void Initialize() {
+    extension_.reset(prefs_.AddExtension("on_extension_installed"));
+    EXPECT_EQ(Extension::ENABLED,
+              prefs()->GetExtensionState(extension_->id()));
+    EXPECT_FALSE(prefs()->IsIncognitoEnabled(extension_->id()));
+    prefs()->OnExtensionInstalled(extension_.get(),
+                                  Extension::DISABLED, true);
+  }
+
+  virtual void Verify() {
+    EXPECT_EQ(Extension::DISABLED,
+              prefs()->GetExtensionState(extension_->id()));
+    EXPECT_TRUE(prefs()->IsIncognitoEnabled(extension_->id()));
+  }
+
+ private:
+  scoped_ptr<Extension> extension_;
+};
+TEST_F(ExtensionPrefsOnExtensionInstalled,
+       ExtensionPrefsOnExtensionInstalled) {}
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 9c428e3..95177c6 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -169,7 +169,12 @@ void ManifestFetchesBuilder::AddExtension(const Extension& extension) {
 void ManifestFetchesBuilder::AddPendingExtension(
     const std::string& id,
     const PendingExtensionInfo& info) {
-  AddExtensionData(Extension::INTERNAL, id, info.version,
+  // Use a zero version to ensure that a pending extension will always
+  // be updated, and thus installed (assuming all extensions have
+  // non-zero versions).
+  scoped_ptr<Version> version(
+      Version::GetVersionFromString("0.0.0.0"));
+  AddExtensionData(Extension::INTERNAL, id, *version,
                    false, info.is_theme, info.update_url);
 }
 
@@ -716,12 +721,6 @@ bool ExtensionUpdater::GetExistingVersion(const std::string& id,
       WideToASCII(prefs_->GetString(kExtensionBlacklistUpdateVersion));
     return true;
   }
-  PendingExtensionMap::const_iterator it =
-      service_->pending_extensions().find(id);
-  if (it != service_->pending_extensions().end()) {
-    *version = it->second.version.GetString();
-    return true;
-  }
   Extension* extension = service_->GetExtensionById(id, false);
   if (!extension) {
     return false;
@@ -745,20 +744,24 @@ std::vector<int> ExtensionUpdater::DetermineUpdates(
     if (!fetch_data.Includes(update->extension_id))
       continue;
 
-    std::string version;
-    if (!GetExistingVersion(update->extension_id, &version))
-      continue;
+    if (service_->pending_extensions().find(update->extension_id) ==
+        service_->pending_extensions().end()) {
+      // If we're not installing pending extension, and the update
+      // version is the same or older than what's already installed,
+      // we don't want it.
+      std::string version;
+      if (!GetExistingVersion(update->extension_id, &version))
+        continue;
 
-    // If the update version is the same or older than what's already installed,
-    // we don't want it.
-    scoped_ptr<Version> existing_version(
-        Version::GetVersionFromString(version));
-    scoped_ptr<Version> update_version(
-        Version::GetVersionFromString(update->version));
+      scoped_ptr<Version> existing_version(
+          Version::GetVersionFromString(version));
+      scoped_ptr<Version> update_version(
+          Version::GetVersionFromString(update->version));
 
-    if (!update_version.get() ||
-        update_version->CompareTo(*(existing_version.get())) <= 0) {
-      continue;
+      if (!update_version.get() ||
+          update_version->CompareTo(*(existing_version.get())) <= 0) {
+        continue;
+      }
     }
 
     // If the update specifies a browser minimum version, do we qualify?
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 78fbbf3..5266888 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -115,13 +115,12 @@ void CreateTestPendingExtensions(int count, const GURL& update_url,
   for (int i = 1; i <= count; i++) {
     bool is_theme = (i % 2) == 0;
     const bool kInstallSilently = true;
-    scoped_ptr<Version> version(
-        Version::GetVersionFromString(StringPrintf("%d.0.0.0", i)));
-    ASSERT_TRUE(version.get());
+    const Extension::State kInitialState = Extension::ENABLED;
+    const bool kInitialIncognitoEnabled = false;
     std::string id = GenerateId(StringPrintf("extension%i", i));
     (*pending_extensions)[id] =
-        PendingExtensionInfo(update_url, *version,
-                             is_theme, kInstallSilently);
+        PendingExtensionInfo(update_url, is_theme, kInstallSilently,
+                             kInitialState, kInitialIncognitoEnabled);
   }
 }
 
@@ -326,7 +325,7 @@ class ExtensionUpdaterTest : public testing::Test {
     ExtractParameters(decoded, &params);
     if (pending) {
       EXPECT_EQ(pending_extensions.begin()->first, params["id"]);
-      EXPECT_EQ("1.0.0.0", params["v"]);
+      EXPECT_EQ("0.0.0.0", params["v"]);
     } else {
       EXPECT_EQ(extensions[0]->id(), params["id"]);
       EXPECT_EQ(extensions[0]->VersionString(), params["v"]);
@@ -446,16 +445,15 @@ class ExtensionUpdaterTest : public testing::Test {
     UpdateManifest::Results updates;
     for (PendingExtensionMap::const_iterator it = pending_extensions.begin();
          it != pending_extensions.end(); ++it) {
-      fetch_data.AddExtension(it->first,
-                              it->second.version.GetString(),
+      fetch_data.AddExtension(it->first, "1.0.0.0",
                               ManifestFetchData::kNeverPinged);
       AddParseResult(it->first,
                      "1.1", "http://localhost/e1_1.1.crx", &updates);
     }
     std::vector<int> updateable =
         updater->DetermineUpdates(fetch_data, updates);
-    // Only the first one is updateable.
-    EXPECT_EQ(1u, updateable.size());
+    // All the apps should be updateable.
+    EXPECT_EQ(3u, updateable.size());
     for (std::vector<int>::size_type i = 0; i < updateable.size(); ++i) {
       EXPECT_EQ(static_cast<int>(i), updateable[i]);
     }
@@ -552,10 +550,12 @@ class ExtensionUpdaterTest : public testing::Test {
     if (pending) {
       const bool kIsTheme = false;
       const bool kInstallSilently = true;
+      const Extension::State kInitialState = Extension::ENABLED;
+      const bool kInitialIncognitoEnabled = false;
       PendingExtensionMap pending_extensions;
       pending_extensions[id] =
-          PendingExtensionInfo(test_url, *version,
-                               kIsTheme, kInstallSilently);
+          PendingExtensionInfo(test_url, kIsTheme, kInstallSilently,
+                               kInitialState, kInitialIncognitoEnabled);
       service.set_pending_extensions(pending_extensions);
     }
 
@@ -880,18 +880,15 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
     STLDeleteElements(&extensions);
   }
 
-  scoped_ptr<Version> version(Version::GetVersionFromString("0"));
-  ASSERT_TRUE(version.get());
-
   // Extensions with invalid update URLs should be rejected.
   builder.AddPendingExtension(
       GenerateId("foo"), PendingExtensionInfo(GURL("http:google.com:foo"),
-                                              *version, false, false));
+                                              false, false, true, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // Extensions with empty IDs should be rejected.
   builder.AddPendingExtension(
-      "", PendingExtensionInfo(GURL(), *version, false, false));
+      "", PendingExtensionInfo(GURL(), false, false, true, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // TODO(akalin): Test that extensions with empty update URLs
@@ -900,7 +897,8 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
   // Extensions with empty update URLs should have a default one
   // filled in.
   builder.AddPendingExtension(
-      GenerateId("foo"), PendingExtensionInfo(GURL(), *version, false, false));
+      GenerateId("foo"), PendingExtensionInfo(GURL(),
+                                              false, false, true, false));
   std::vector<ManifestFetchData*> fetches = builder.GetFetches();
   ASSERT_EQ(1u, fetches.size());
   scoped_ptr<ManifestFetchData> fetch(fetches[0]);
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index b7c0800..5f22ed9 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -74,19 +74,22 @@ static bool ShouldReloadExtensionManifest(const ExtensionInfo& info) {
 }  // namespace
 
 PendingExtensionInfo::PendingExtensionInfo(const GURL& update_url,
-                                           const Version& version,
                                            bool is_theme,
-                                           bool install_silently)
+                                           bool install_silently,
+                                           bool enable_on_install,
+                                           bool enable_incognito_on_install)
     : update_url(update_url),
-      version(version),
       is_theme(is_theme),
-      install_silently(install_silently) {}
+      install_silently(install_silently),
+      enable_on_install(enable_on_install),
+      enable_incognito_on_install(enable_incognito_on_install) {}
 
 PendingExtensionInfo::PendingExtensionInfo()
     : update_url(),
-      version(),
       is_theme(false),
-      install_silently(false) {}
+      install_silently(false),
+      enable_on_install(false),
+      enable_incognito_on_install(false) {}
 
 // ExtensionsService.
 
@@ -272,21 +275,25 @@ void ExtensionsService::UpdateExtension(const std::string& id,
 
 void ExtensionsService::AddPendingExtension(
     const std::string& id, const GURL& update_url,
-    const Version& version, bool is_theme, bool install_silently) {
+    bool is_theme, bool install_silently,
+    bool enable_on_install, bool enable_incognito_on_install) {
   if (GetExtensionByIdInternal(id, true, true)) {
     LOG(DFATAL) << "Trying to add pending extension " << id
                 << " which already exists";
     return;
   }
-  AddPendingExtensionInternal(id, update_url, version,
-                              is_theme, install_silently);
+  AddPendingExtensionInternal(
+      id, update_url, is_theme, install_silently,
+      enable_on_install, enable_incognito_on_install);
 }
 
 void ExtensionsService::AddPendingExtensionInternal(
     const std::string& id, const GURL& update_url,
-    const Version& version, bool is_theme, bool install_silently) {
+    bool is_theme, bool install_silently,
+    bool enable_on_install, bool enable_incognito_on_install) {
   pending_extensions_[id] =
-      PendingExtensionInfo(update_url, version, is_theme, install_silently);
+      PendingExtensionInfo(update_url, is_theme, install_silently,
+                           enable_on_install, enable_incognito_on_install);
 }
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
@@ -908,23 +915,52 @@ void ExtensionsService::UpdateActiveExtensionsInCrashReporter() {
 
 void ExtensionsService::OnExtensionInstalled(Extension* extension,
                                              bool allow_privilege_increase) {
+  // Ensure extension is deleted unless we transfer ownership.
+  scoped_ptr<Extension> scoped_extension(extension);
+  Extension::State initial_state = Extension::DISABLED;
+  bool initial_enable_incognito = false;
   PendingExtensionMap::iterator it =
       pending_extensions_.find(extension->id());
-  if (it != pending_extensions_.end() &&
-      (it->second.is_theme != extension->is_theme())) {
-    LOG(WARNING) << "Not installing pending extension " << extension->id()
-                 << " with is_theme = " << extension->is_theme()
-                 << "; expected is_theme = " << it->second.is_theme;
-    // Delete the extension directory since we're not going to load
-    // it.
-    ChromeThread::PostTask(
-        ChromeThread::FILE, FROM_HERE,
-        NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
-    delete extension;
-    return;
+  if (it != pending_extensions_.end()) {
+    // Set initial state from pending extension data.
+    if (it->second.is_theme != extension->is_theme()) {
+      LOG(WARNING)
+          << "Not installing pending extension " << extension->id()
+          << " with is_theme = " << extension->is_theme()
+          << "; expected is_theme = " << it->second.is_theme;
+      // Delete the extension directory since we're not going to
+      // load it.
+      ChromeThread::PostTask(
+          ChromeThread::FILE, FROM_HERE,
+          NewRunnableFunction(&DeleteFileHelper, extension->path(), true));
+      return;
+    }
+    if (it->second.is_theme) {
+      DCHECK(it->second.enable_on_install);
+      initial_state = Extension::ENABLED;
+      DCHECK(!it->second.enable_incognito_on_install);
+      initial_enable_incognito = false;
+    } else {
+      initial_state =
+          it->second.enable_on_install ?
+          Extension::ENABLED : Extension::DISABLED;
+      initial_enable_incognito =
+          it->second.enable_incognito_on_install;
+    }
+
+    pending_extensions_.erase(it);
+  } else {
+    // Make sure we don't enable a disabled extension.
+    Extension::State existing_state =
+        extension_prefs_->GetExtensionState(extension->id());
+    initial_state =
+        (existing_state == Extension::DISABLED) ?
+        Extension::DISABLED : Extension::ENABLED;
+    initial_enable_incognito = false;
   }
 
-  extension_prefs_->OnExtensionInstalled(extension);
+  extension_prefs_->OnExtensionInstalled(
+      extension, initial_state, initial_enable_incognito);
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
   // to apply it.
@@ -940,16 +976,11 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension,
         Details<Extension>(extension));
   }
 
-  // Also load the extension.
-  OnExtensionLoaded(extension, allow_privilege_increase);
-
-  // Erase any pending extension.
-  if (it != pending_extensions_.end()) {
-    pending_extensions_.erase(it);
-  }
-
   if (profile_->GetTemplateURLModel())
     profile_->GetTemplateURLModel()->RegisterExtensionKeyword(extension);
+
+  // Transfer ownership of |extension| to OnExtensionLoaded.
+  OnExtensionLoaded(scoped_extension.release(), allow_privilege_increase);
 }
 
 Extension* ExtensionsService::GetExtensionByIdInternal(const std::string& id,
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index a5725ef..5385da5 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -48,16 +48,18 @@ class Version;
 // default one is assumed.
 struct PendingExtensionInfo {
   PendingExtensionInfo(const GURL& update_url,
-                       const Version& version,
                        bool is_theme,
-                       bool install_silently);
+                       bool install_silently,
+                       bool enable_on_install,
+                       bool enable_incognito_on_install);
 
   PendingExtensionInfo();
 
   GURL update_url;
-  Version version;
   bool is_theme;
   bool install_silently;
+  bool enable_on_install;
+  bool enable_incognito_on_install;
 };
 
 // A PendingExtensionMap is a map from IDs of pending extensions to
@@ -206,11 +208,12 @@ class ExtensionsService
   // It is an error to call this with an already-installed extension
   // (even a disabled one).
   //
-  // TODO(akalin): Make sure that all the places that check for
-  // existing versions also consult the pending extension info.
+  // TODO(akalin): Replace |install_silently| with a list of
+  // pre-enabled permissions.
   void AddPendingExtension(
       const std::string& id, const GURL& update_url,
-      const Version& version, bool is_theme, bool install_silently);
+      bool is_theme, bool install_silently,
+      bool enable_on_install, bool enable_incognito_on_install);
 
   // Reloads the specified extension.
   void ReloadExtension(const std::string& extension_id);
@@ -363,7 +366,8 @@ class ExtensionsService
   // id is not already installed.
   void AddPendingExtensionInternal(
       const std::string& id, const GURL& update_url,
-      const Version& version, bool is_theme, bool install_silently);
+      bool is_theme, bool install_silently,
+      bool enable_on_install, bool enable_incognito_on_install);
 
   // Handles sending notification that |extension| was loaded.
   void NotifyExtensionLoaded(Extension* extension);
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 7f62914..90350fa 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -429,9 +429,16 @@ class ExtensionsServiceTest
     ExtensionErrorReporter::GetInstance()->ClearErrors();
   }
 
+  enum UpdateState {
+    FAILED_SILENTLY,
+    FAILED,
+    UPDATED,
+    INSTALLED,
+    ENABLED
+  };
+
   void UpdateExtension(const std::string& id, const FilePath& in_path,
-                       bool should_succeed, bool should_install,
-                       bool expect_report_on_failure) {
+                       UpdateState expected_state) {
     ASSERT_TRUE(file_util::PathExists(in_path));
 
     // We need to copy this to a temporary location because Update() will delete
@@ -440,17 +447,39 @@ class ExtensionsServiceTest
     path = path.Append(in_path.BaseName());
     ASSERT_TRUE(file_util::CopyFile(in_path, path));
 
+    int previous_enabled_extension_count =
+        service_->extensions()->size();
+    int previous_installed_extension_count =
+        previous_enabled_extension_count +
+        service_->disabled_extensions()->size();
+
     service_->UpdateExtension(id, path, GURL());
     loop_.RunAllPending();
-    std::vector<std::string> errors = GetErrors();
 
-    if (should_succeed) {
-      EXPECT_EQ(0u, errors.size()) << path.value();
-      EXPECT_EQ(should_install ? 1u : 0u, service_->extensions()->size());
+    std::vector<std::string> errors = GetErrors();
+    int error_count = errors.size();
+    int enabled_extension_count =
+        service_->extensions()->size();
+    int installed_extension_count =
+        enabled_extension_count + service_->disabled_extensions()->size();
+
+    int expected_error_count = (expected_state == FAILED) ? 1 : 0;
+    EXPECT_EQ(expected_error_count, error_count) << path.value();
+
+    if (expected_state <= FAILED) {
+      EXPECT_EQ(previous_enabled_extension_count,
+                enabled_extension_count);
+      EXPECT_EQ(previous_installed_extension_count,
+                installed_extension_count);
     } else {
-      if (expect_report_on_failure) {
-        EXPECT_EQ(1u, errors.size()) << path.value();
-      }
+      int expected_installed_extension_count =
+          (expected_state >= INSTALLED) ? 1 : 0;
+      int expected_enabled_extension_count =
+          (expected_state >= ENABLED) ? 1 : 0;
+      EXPECT_EQ(expected_installed_extension_count,
+                installed_extension_count);
+      EXPECT_EQ(expected_enabled_extension_count,
+                enabled_extension_count);
     }
 
     // Update() should delete the temporary input file.
@@ -1143,7 +1172,7 @@ TEST_F(ExtensionsServiceTest, UpdateExtension) {
   ASSERT_EQ(good_crx, good->id());
 
   path = extensions_path.AppendASCII("good2.crx");
-  UpdateExtension(good_crx, path, true, true, true);
+  UpdateExtension(good_crx, path, ENABLED);
   ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
 }
 
@@ -1155,7 +1184,7 @@ TEST_F(ExtensionsServiceTest, UpdateNotInstalledExtension) {
   extensions_path = extensions_path.AppendASCII("extensions");
 
   FilePath path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(good_crx, path, true, false, true);
+  UpdateExtension(good_crx, path, UPDATED);
   loop_.RunAllPending();
 
   ASSERT_EQ(0u, service_->extensions()->size());
@@ -1179,7 +1208,7 @@ TEST_F(ExtensionsServiceTest, UpdateWillNotDowngrade) {
 
   // Change path from good2.crx -> good.crx
   path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(good_crx, path, false, false, true);
+  UpdateExtension(good_crx, path, FAILED);
   ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString());
 }
 
@@ -1195,7 +1224,7 @@ TEST_F(ExtensionsServiceTest, UpdateToSameVersionIsNoop) {
   InstallExtension(path, true);
   Extension* good = service_->extensions()->at(0);
   ASSERT_EQ(good_crx, good->id());
-  UpdateExtension(good_crx, path, false, false, false);
+  UpdateExtension(good_crx, path, FAILED_SILENTLY);
 }
 
 // Test adding a pending extension.
@@ -1204,21 +1233,20 @@ TEST_F(ExtensionsServiceTest, AddPendingExtension) {
 
   const std::string kFakeId("fake-id");
   const GURL kFakeUpdateURL("http:://fake.update/url");
-  scoped_ptr<Version> fake_version(Version::GetVersionFromString("4.3.2.1"));
-  ASSERT_TRUE(fake_version.get());
   const bool kFakeIsTheme(false);
   const bool kFakeInstallSilently(true);
+  const Extension::State kFakeInitialState(Extension::ENABLED);
+  const bool kFakeInitialIncognitoEnabled(false);
 
-  service_->AddPendingExtension(kFakeId, kFakeUpdateURL,
-                                *fake_version, kFakeIsTheme,
-                                kFakeInstallSilently);
+  service_->AddPendingExtension(
+      kFakeId, kFakeUpdateURL, kFakeIsTheme, kFakeInstallSilently,
+      kFakeInitialState, kFakeInitialIncognitoEnabled);
   PendingExtensionMap::const_iterator it =
       service_->pending_extensions().find(kFakeId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
   EXPECT_EQ(kFakeUpdateURL, it->second.update_url);
   EXPECT_EQ(kFakeIsTheme, it->second.is_theme);
   EXPECT_EQ(kFakeInstallSilently, it->second.install_silently);
-  EXPECT_TRUE(it->second.version.Equals(*fake_version));
 }
 
 namespace {
@@ -1226,27 +1254,36 @@ const char kGoodId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kGoodUpdateURL[] = "http://good.update/url";
 const bool kGoodIsTheme = false;
 const bool kGoodInstallSilently = true;
-const char kGoodVersion[] = "1.2.3.4";
+const Extension::State kGoodInitialState = Extension::DISABLED;
+const bool kGoodInitialIncognitoEnabled = true;
 }  // namespace
 
 // Test updating a pending extension.
 TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
   InitializeEmptyExtensionsService();
-  scoped_ptr<Version> good_version(
-      Version::GetVersionFromString(kGoodVersion));
-  ASSERT_TRUE(good_version.get());
-  service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL),
-                                *good_version, kGoodIsTheme,
-                                kGoodInstallSilently);
+  service_->AddPendingExtension(
+      kGoodId, GURL(kGoodUpdateURL), kGoodIsTheme,
+      kGoodInstallSilently, kGoodInitialState,
+      kGoodInitialIncognitoEnabled);
   EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
 
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
   FilePath path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(kGoodId, path, true, true, false);
+  UpdateExtension(kGoodId, path, INSTALLED);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
+
+  Extension* extension = service_->GetExtensionById(kGoodId, true);
+  ASSERT_TRUE(extension);
+
+  bool enabled = service_->GetExtensionById(kGoodId, false);
+  EXPECT_EQ(kGoodInitialState == Extension::ENABLED, enabled);
+  EXPECT_EQ(kGoodInitialState,
+            service_->extension_prefs()->GetExtensionState(extension->id()));
+  EXPECT_EQ(kGoodInitialIncognitoEnabled,
+            service_->IsIncognitoEnabled(extension));
 }
 
 // TODO(akalin): Test updating a pending extension non-silently once
@@ -1256,20 +1293,18 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
 // Test updating a pending extension with wrong is_theme.
 TEST_F(ExtensionsServiceTest, UpdatePendingExtensionWrongIsTheme) {
   InitializeEmptyExtensionsService();
-  scoped_ptr<Version> good_version(
-      Version::GetVersionFromString(kGoodVersion));
-  ASSERT_TRUE(good_version.get());
   // Add pending extension with a flipped is_theme.
-  service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL),
-                                *good_version, !kGoodIsTheme,
-                                kGoodInstallSilently);
+  service_->AddPendingExtension(
+      kGoodId, GURL(kGoodUpdateURL), !kGoodIsTheme,
+      kGoodInstallSilently, kGoodInitialState,
+      kGoodInitialIncognitoEnabled);
   EXPECT_TRUE(ContainsKey(service_->pending_extensions(), kGoodId));
 
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
   FilePath path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(kGoodId, path, true, false, false);
+  UpdateExtension(kGoodId, path, UPDATED);
 
   // TODO(akalin): Figure out how to check that the extensions
   // directory is cleaned up properly in OnExtensionInstalled().
@@ -1285,7 +1320,7 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionNotPending) {
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
   extensions_path = extensions_path.AppendASCII("extensions");
   FilePath path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(kGoodId, path, true, false, false);
+  UpdateExtension(kGoodId, path, UPDATED);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
 }
@@ -1306,10 +1341,11 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled) {
   // Use AddPendingExtensionInternal() as AddPendingExtension() would
   // balk.
   service_->AddPendingExtensionInternal(
-      good->id(), good->update_url(), *good->version(),
-      good->is_theme(), kGoodInstallSilently);
+      good->id(), good->update_url(), good->is_theme(),
+      kGoodInstallSilently, kGoodInitialState,
+      kGoodInitialIncognitoEnabled);
 
-  UpdateExtension(good->id(), path, true, true, false);
+  UpdateExtension(good->id(), path, INSTALLED);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
 }
@@ -1355,7 +1391,7 @@ TEST_F(ExtensionsServiceTest, UnloadBlacklistedExtension) {
   InstallExtension(path, true);
   Extension* good = service_->extensions()->at(0);
   EXPECT_EQ(good_crx, good->id());
-  UpdateExtension(good_crx, path, false, false, false);
+  UpdateExtension(good_crx, path, FAILED_SILENTLY);
 
   std::vector<std::string> blacklist;
   blacklist.push_back(good_crx);
diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc
index 5a0c9f8..97c47e8 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -69,7 +69,9 @@ Extension* TestExtensionPrefs::AddExtensionWithManifest(
   EXPECT_TRUE(extension->InitFromValue(manifest, false, &errors));
   extension->set_location(Extension::INTERNAL);
   EXPECT_TRUE(Extension::IdIsValid(extension->id()));
-  prefs_->OnExtensionInstalled(extension);
+  const bool kInitialIncognitoEnabled = false;
+  prefs_->OnExtensionInstalled(extension, Extension::ENABLED,
+                               kInitialIncognitoEnabled);
   return extension;
 }