Revert r50804 "Reworked ExtensionsService::AddPendingExtension()."

Reverted r50804 as it seems to be related to memory error.

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

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

12 files changed, 142 insertions, 232 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index d3494da..ce0f37d 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -81,8 +81,7 @@ 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, Extension::ENABLED, false);
+    service->extension_prefs()->OnExtensionInstalled(extension);
     service->SetIsIncognitoEnabled(extension, true);
   }
 
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 93f29c4..0f9661c 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -444,14 +444,13 @@ void ExtensionPrefs::SetToolbarOrder(
   prefs_->ScheduleSavePersistentPrefs();
 }
 
-void ExtensionPrefs::OnExtensionInstalled(
-    Extension* extension, Extension::State initial_state,
-    bool initial_incognito_enabled) {
+void ExtensionPrefs::OnExtensionInstalled(Extension* extension) {
   const std::string& id = extension->id();
-  UpdateExtensionPref(id, kPrefState,
-                      Value::CreateIntegerValue(initial_state));
-  UpdateExtensionPref(id, kPrefIncognitoEnabled,
-                      Value::CreateBooleanValue(initial_incognito_enabled));
+  // Make sure we don't enable a disabled extension.
+  if (GetExtensionState(extension->id()) != Extension::DISABLED) {
+    UpdateExtensionPref(id, kPrefState,
+                        Value::CreateIntegerValue(Extension::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 f7733c3..77487e9 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -47,9 +47,7 @@ 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,
-                            Extension::State initial_state,
-                            bool initial_incognito_enabled);
+  void OnExtensionInstalled(Extension* extension);
 
   // 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 26df5b1..5f45ad5 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -345,26 +345,3 @@ 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 95177c6..9c428e3 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -169,12 +169,7 @@ void ManifestFetchesBuilder::AddExtension(const Extension& extension) {
 void ManifestFetchesBuilder::AddPendingExtension(
     const std::string& id,
     const PendingExtensionInfo& info) {
-  // 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,
+  AddExtensionData(Extension::INTERNAL, id, info.version,
                    false, info.is_theme, info.update_url);
 }
 
@@ -721,6 +716,12 @@ 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;
@@ -744,24 +745,20 @@ std::vector<int> ExtensionUpdater::DetermineUpdates(
     if (!fetch_data.Includes(update->extension_id))
       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;
+    std::string version;
+    if (!GetExistingVersion(update->extension_id, &version))
+      continue;
 
-      scoped_ptr<Version> existing_version(
-          Version::GetVersionFromString(version));
-      scoped_ptr<Version> update_version(
-          Version::GetVersionFromString(update->version));
+    // 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));
 
-      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 5266888..78fbbf3 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -115,12 +115,13 @@ 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;
-    const Extension::State kInitialState = Extension::ENABLED;
-    const bool kInitialIncognitoEnabled = false;
+    scoped_ptr<Version> version(
+        Version::GetVersionFromString(StringPrintf("%d.0.0.0", i)));
+    ASSERT_TRUE(version.get());
     std::string id = GenerateId(StringPrintf("extension%i", i));
     (*pending_extensions)[id] =
-        PendingExtensionInfo(update_url, is_theme, kInstallSilently,
-                             kInitialState, kInitialIncognitoEnabled);
+        PendingExtensionInfo(update_url, *version,
+                             is_theme, kInstallSilently);
   }
 }
 
@@ -325,7 +326,7 @@ class ExtensionUpdaterTest : public testing::Test {
     ExtractParameters(decoded, &params);
     if (pending) {
       EXPECT_EQ(pending_extensions.begin()->first, params["id"]);
-      EXPECT_EQ("0.0.0.0", params["v"]);
+      EXPECT_EQ("1.0.0.0", params["v"]);
     } else {
       EXPECT_EQ(extensions[0]->id(), params["id"]);
       EXPECT_EQ(extensions[0]->VersionString(), params["v"]);
@@ -445,15 +446,16 @@ 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, "1.0.0.0",
+      fetch_data.AddExtension(it->first,
+                              it->second.version.GetString(),
                               ManifestFetchData::kNeverPinged);
       AddParseResult(it->first,
                      "1.1", "http://localhost/e1_1.1.crx", &updates);
     }
     std::vector<int> updateable =
         updater->DetermineUpdates(fetch_data, updates);
-    // All the apps should be updateable.
-    EXPECT_EQ(3u, updateable.size());
+    // Only the first one is updateable.
+    EXPECT_EQ(1u, updateable.size());
     for (std::vector<int>::size_type i = 0; i < updateable.size(); ++i) {
       EXPECT_EQ(static_cast<int>(i), updateable[i]);
     }
@@ -550,12 +552,10 @@ 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, kIsTheme, kInstallSilently,
-                               kInitialState, kInitialIncognitoEnabled);
+          PendingExtensionInfo(test_url, *version,
+                               kIsTheme, kInstallSilently);
       service.set_pending_extensions(pending_extensions);
     }
 
@@ -880,15 +880,18 @@ 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"),
-                                              false, false, true, false));
+                                              *version, false, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // Extensions with empty IDs should be rejected.
   builder.AddPendingExtension(
-      "", PendingExtensionInfo(GURL(), false, false, true, false));
+      "", PendingExtensionInfo(GURL(), *version, false, false));
   EXPECT_TRUE(builder.GetFetches().empty());
 
   // TODO(akalin): Test that extensions with empty update URLs
@@ -897,8 +900,7 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
   // Extensions with empty update URLs should have a default one
   // filled in.
   builder.AddPendingExtension(
-      GenerateId("foo"), PendingExtensionInfo(GURL(),
-                                              false, false, true, false));
+      GenerateId("foo"), PendingExtensionInfo(GURL(), *version, false, 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 5c4531d..a416f17 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -73,22 +73,19 @@ static bool ShouldReloadExtensionManifest(const ExtensionInfo& info) {
 }  // namespace
 
 PendingExtensionInfo::PendingExtensionInfo(const GURL& update_url,
+                                           const Version& version,
                                            bool is_theme,
-                                           bool install_silently,
-                                           bool enable_on_install,
-                                           bool enable_incognito_on_install)
+                                           bool install_silently)
     : update_url(update_url),
+      version(version),
       is_theme(is_theme),
-      install_silently(install_silently),
-      enable_on_install(enable_on_install),
-      enable_incognito_on_install(enable_incognito_on_install) {}
+      install_silently(install_silently) {}
 
 PendingExtensionInfo::PendingExtensionInfo()
     : update_url(),
+      version(),
       is_theme(false),
-      install_silently(false),
-      enable_on_install(false),
-      enable_incognito_on_install(false) {}
+      install_silently(false) {}
 
 // ExtensionsService.
 
@@ -274,25 +271,21 @@ void ExtensionsService::UpdateExtension(const std::string& id,
 
 void ExtensionsService::AddPendingExtension(
     const std::string& id, const GURL& update_url,
-    bool is_theme, bool install_silently,
-    bool enable_on_install, bool enable_incognito_on_install) {
+    const Version& version, bool is_theme, bool install_silently) {
   if (GetExtensionByIdInternal(id, true, true)) {
     LOG(DFATAL) << "Trying to add pending extension " << id
                 << " which already exists";
     return;
   }
-  AddPendingExtensionInternal(
-      id, update_url, is_theme, install_silently,
-      enable_on_install, enable_incognito_on_install);
+  AddPendingExtensionInternal(id, update_url, version,
+                              is_theme, install_silently);
 }
 
 void ExtensionsService::AddPendingExtensionInternal(
     const std::string& id, const GURL& update_url,
-    bool is_theme, bool install_silently,
-    bool enable_on_install, bool enable_incognito_on_install) {
+    const Version& version, bool is_theme, bool install_silently) {
   pending_extensions_[id] =
-      PendingExtensionInfo(update_url, is_theme, install_silently,
-                           enable_on_install, enable_incognito_on_install);
+      PendingExtensionInfo(update_url, version, is_theme, install_silently);
 }
 
 void ExtensionsService::ReloadExtension(const std::string& extension_id) {
@@ -912,52 +905,23 @@ 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()) {
-    // 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;
+  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;
   }
 
-  extension_prefs_->OnExtensionInstalled(
-      extension, initial_state, initial_enable_incognito);
+  extension_prefs_->OnExtensionInstalled(extension);
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
   // to apply it.
@@ -973,11 +937,16 @@ 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 f49a440..f019630 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -48,18 +48,16 @@ class Version;
 // default one is assumed.
 struct PendingExtensionInfo {
   PendingExtensionInfo(const GURL& update_url,
+                       const Version& version,
                        bool is_theme,
-                       bool install_silently,
-                       bool enable_on_install,
-                       bool enable_incognito_on_install);
+                       bool install_silently);
 
   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
@@ -208,12 +206,11 @@ class ExtensionsService
   // It is an error to call this with an already-installed extension
   // (even a disabled one).
   //
-  // TODO(akalin): Replace |install_silently| with a list of
-  // pre-enabled permissions.
+  // TODO(akalin): Make sure that all the places that check for
+  // existing versions also consult the pending extension info.
   void AddPendingExtension(
       const std::string& id, const GURL& update_url,
-      bool is_theme, bool install_silently,
-      bool enable_on_install, bool enable_incognito_on_install);
+      const Version& version, bool is_theme, bool install_silently);
 
   // Reloads the specified extension.
   void ReloadExtension(const std::string& extension_id);
@@ -368,8 +365,7 @@ class ExtensionsService
   // id is not already installed.
   void AddPendingExtensionInternal(
       const std::string& id, const GURL& update_url,
-      bool is_theme, bool install_silently,
-      bool enable_on_install, bool enable_incognito_on_install);
+      const Version& version, bool is_theme, bool install_silently);
 
   // 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 6ce579c..cc50bab 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -429,16 +429,9 @@ class ExtensionsServiceTest
     ExtensionErrorReporter::GetInstance()->ClearErrors();
   }
 
-  enum UpdateState {
-    FAILED_SILENTLY,
-    FAILED,
-    UPDATED,
-    INSTALLED,
-    ENABLED
-  };
-
   void UpdateExtension(const std::string& id, const FilePath& in_path,
-                       UpdateState expected_state) {
+                       bool should_succeed, bool should_install,
+                       bool expect_report_on_failure) {
     ASSERT_TRUE(file_util::PathExists(in_path));
 
     // We need to copy this to a temporary location because Update() will delete
@@ -447,39 +440,17 @@ 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();
-    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);
+
+    if (should_succeed) {
+      EXPECT_EQ(0u, errors.size()) << path.value();
+      EXPECT_EQ(should_install ? 1u : 0u, service_->extensions()->size());
     } else {
-      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);
+      if (expect_report_on_failure) {
+        EXPECT_EQ(1u, errors.size()) << path.value();
+      }
     }
 
     // Update() should delete the temporary input file.
@@ -1171,7 +1142,7 @@ TEST_F(ExtensionsServiceTest, UpdateExtension) {
   ASSERT_EQ(good_crx, good->id());
 
   path = extensions_path.AppendASCII("good2.crx");
-  UpdateExtension(good_crx, path, ENABLED);
+  UpdateExtension(good_crx, path, true, true, true);
   ASSERT_EQ("1.0.0.1", loaded_[0]->version()->GetString());
 }
 
@@ -1183,7 +1154,7 @@ TEST_F(ExtensionsServiceTest, UpdateNotInstalledExtension) {
   extensions_path = extensions_path.AppendASCII("extensions");
 
   FilePath path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(good_crx, path, UPDATED);
+  UpdateExtension(good_crx, path, true, false, true);
   loop_.RunAllPending();
 
   ASSERT_EQ(0u, service_->extensions()->size());
@@ -1207,7 +1178,7 @@ TEST_F(ExtensionsServiceTest, UpdateWillNotDowngrade) {
 
   // Change path from good2.crx -> good.crx
   path = extensions_path.AppendASCII("good.crx");
-  UpdateExtension(good_crx, path, FAILED);
+  UpdateExtension(good_crx, path, false, false, true);
   ASSERT_EQ("1.0.0.1", service_->extensions()->at(0)->VersionString());
 }
 
@@ -1223,7 +1194,7 @@ TEST_F(ExtensionsServiceTest, UpdateToSameVersionIsNoop) {
   InstallExtension(path, true);
   Extension* good = service_->extensions()->at(0);
   ASSERT_EQ(good_crx, good->id());
-  UpdateExtension(good_crx, path, FAILED_SILENTLY);
+  UpdateExtension(good_crx, path, false, false, false);
 }
 
 // Test adding a pending extension.
@@ -1232,20 +1203,21 @@ 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, kFakeIsTheme, kFakeInstallSilently,
-      kFakeInitialState, kFakeInitialIncognitoEnabled);
+  service_->AddPendingExtension(kFakeId, kFakeUpdateURL,
+                                *fake_version, kFakeIsTheme,
+                                kFakeInstallSilently);
   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 {
@@ -1253,36 +1225,27 @@ const char kGoodId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kGoodUpdateURL[] = "http://good.update/url";
 const bool kGoodIsTheme = false;
 const bool kGoodInstallSilently = true;
-const Extension::State kGoodInitialState = Extension::DISABLED;
-const bool kGoodInitialIncognitoEnabled = true;
+const char kGoodVersion[] = "1.2.3.4";
 }  // namespace
 
 // Test updating a pending extension.
 TEST_F(ExtensionsServiceTest, UpdatePendingExtension) {
   InitializeEmptyExtensionsService();
-  service_->AddPendingExtension(
-      kGoodId, GURL(kGoodUpdateURL), kGoodIsTheme,
-      kGoodInstallSilently, kGoodInitialState,
-      kGoodInitialIncognitoEnabled);
+  scoped_ptr<Version> good_version(
+      Version::GetVersionFromString(kGoodVersion));
+  ASSERT_TRUE(good_version.get());
+  service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL),
+                                *good_version, kGoodIsTheme,
+                                kGoodInstallSilently);
   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, INSTALLED);
+  UpdateExtension(kGoodId, path, true, true, false);
 
   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
@@ -1292,18 +1255,20 @@ 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), !kGoodIsTheme,
-      kGoodInstallSilently, kGoodInitialState,
-      kGoodInitialIncognitoEnabled);
+  service_->AddPendingExtension(kGoodId, GURL(kGoodUpdateURL),
+                                *good_version, !kGoodIsTheme,
+                                kGoodInstallSilently);
   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, UPDATED);
+  UpdateExtension(kGoodId, path, true, false, false);
 
   // TODO(akalin): Figure out how to check that the extensions
   // directory is cleaned up properly in OnExtensionInstalled().
@@ -1319,7 +1284,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, UPDATED);
+  UpdateExtension(kGoodId, path, true, false, false);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
 }
@@ -1340,11 +1305,10 @@ TEST_F(ExtensionsServiceTest, UpdatePendingExtensionAlreadyInstalled) {
   // Use AddPendingExtensionInternal() as AddPendingExtension() would
   // balk.
   service_->AddPendingExtensionInternal(
-      good->id(), good->update_url(), good->is_theme(),
-      kGoodInstallSilently, kGoodInitialState,
-      kGoodInitialIncognitoEnabled);
+      good->id(), good->update_url(), *good->version(),
+      good->is_theme(), kGoodInstallSilently);
 
-  UpdateExtension(good->id(), path, INSTALLED);
+  UpdateExtension(good->id(), path, true, true, false);
 
   EXPECT_FALSE(ContainsKey(service_->pending_extensions(), kGoodId));
 }
@@ -1390,7 +1354,7 @@ TEST_F(ExtensionsServiceTest, UnloadBlacklistedExtension) {
   InstallExtension(path, true);
   Extension* good = service_->extensions()->at(0);
   EXPECT_EQ(good_crx, good->id());
-  UpdateExtension(good_crx, path, FAILED_SILENTLY);
+  UpdateExtension(good_crx, path, false, false, false);
 
   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 97c47e8..5a0c9f8 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -69,9 +69,7 @@ Extension* TestExtensionPrefs::AddExtensionWithManifest(
   EXPECT_TRUE(extension->InitFromValue(manifest, false, &errors));
   extension->set_location(Extension::INTERNAL);
   EXPECT_TRUE(Extension::IdIsValid(extension->id()));
-  const bool kInitialIncognitoEnabled = false;
-  prefs_->OnExtensionInstalled(extension, Extension::ENABLED,
-                               kInitialIncognitoEnabled);
+  prefs_->OnExtensionInstalled(extension);
   return extension;
 }
 
diff --git a/chrome/browser/sync/glue/extension_model_associator.cc b/chrome/browser/sync/glue/extension_model_associator.cc
index 8a33951..d8200b2 100644
--- a/chrome/browser/sync/glue/extension_model_associator.cc
+++ b/chrome/browser/sync/glue/extension_model_associator.cc
@@ -385,11 +385,23 @@ void ExtensionModelAssociator::TryUpdateClient(
     }
   } else {
     GURL update_url(specifics.update_url());
+    // TODO(akalin): The version number should be used only to
+    // determine auto-updating permissions, not to send to the
+    // auto-update server.
+    scoped_ptr<Version> version(
+        Version::GetVersionFromString("0.0.0.0"));
+    CHECK(version.get());
     // TODO(akalin): Replace silent update with a list of enabled
     // permissions.
+    //
+    // TODO(akalin): Pass through the enabled/incognito_enabled state.
+    // We can't do it on OnClientUpdate() because we'd run into
+    // problems with triggering notifications while we're in a
+    // notification handler.  The bug that this causes is that syncing
+    // a fresh client (i.e., no extensions) re-enables disabled
+    // extensions.
     extensions_service->AddPendingExtension(
-        id, update_url, false, true,
-        specifics.enabled(), specifics.incognito_enabled());
+        id, update_url, *version, false, true);
   }
   DCHECK(!extension_data->NeedsUpdate(ExtensionData::SERVER));
 }
diff --git a/chrome/browser/sync/glue/theme_util.cc b/chrome/browser/sync/glue/theme_util.cc
index 1bd078e..4ea65a4 100644
--- a/chrome/browser/sync/glue/theme_util.cc
+++ b/chrome/browser/sync/glue/theme_util.cc
@@ -127,16 +127,15 @@ void SetCurrentThemeFromThemeSpecifics(
       // No extension with this id exists -- we must install it; we do
       // so by adding it as a pending extension and then triggering an
       // auto-update cycle.
+      scoped_ptr<Version> version(Version::GetVersionFromString("0.0.0.0"));
+      DCHECK(version.get());
       const bool kIsTheme = true;
       // Themes don't need to install silently as they just pop up an
       // informational dialog after installation instead of a
       // confirmation dialog.
       const bool kInstallSilently = false;
-      const bool kEnableOnInstall = true;
-      const bool kEnableIncognitoOnInstall = false;
-      extensions_service->AddPendingExtension(
-          id, update_url, kIsTheme, kInstallSilently,
-          kEnableOnInstall, kEnableIncognitoOnInstall);
+      extensions_service->AddPendingExtension(id, update_url, *version,
+                                              kIsTheme, kInstallSilently);
       ExtensionUpdater* extension_updater = extensions_service->updater();
       // Auto-updates should now be on always (see the construction of
       // the ExtensionsService in ProfileImpl::InitExtensions()).