Get rid of the extension's "Current Version" file.

The entire manifest.json value is now stored in the prefs file. This will
allow for quick extension checks on startup.

BUG=18293
TEST=Make sure installing/upgrading/uninstalling extensions works as expected.

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

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

15 files changed, 254 insertions, 288 deletions

diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 352cd8c..677aada 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -171,6 +171,9 @@ void CrxInstaller::ConfirmInstall() {
     return;
   }
 
+  current_version_ =
+      frontend_->extension_prefs()->GetVersionString(extension_->id());
+
   if (client_.get()) {
     AddRef();  // balanced in ContinueInstall() and AbortInstall().
     client_->ConfirmInstall(this, extension_.get(), install_icon_.get());
@@ -199,17 +202,10 @@ void CrxInstaller::CompleteInstall() {
   DCHECK(MessageLoop::current() == file_loop_);
 
   FilePath version_dir;
-  Extension::InstallType install_type = Extension::INSTALL_ERROR;
-  std::string error_msg;
-  if (!extension_file_util::InstallExtension(unpacked_extension_root_,
-                                             install_directory_,
-                                             extension_->id(),
-                                             extension_->VersionString(),
-                                             &version_dir,
-                                             &install_type, &error_msg)) {
-    ReportFailureFromFileThread(error_msg);
-    return;
-  }
+  Extension::InstallType install_type =
+      extension_file_util::CompareToInstalledVersion(
+          install_directory_, extension_->id(), current_version_,
+          extension_->VersionString(), &version_dir);
 
   if (install_type == Extension::DOWNGRADE) {
     ReportFailureFromFileThread("Attempted to downgrade extension.");
@@ -222,6 +218,13 @@ void CrxInstaller::CompleteInstall() {
     return;
   }
 
+  std::string error_msg;
+  if (!extension_file_util::InstallExtension(unpacked_extension_root_,
+                                             version_dir, &error_msg)) {
+    ReportFailureFromFileThread(error_msg);
+    return;
+  }
+
   // This is lame, but we must reload the extension because absolute paths
   // inside the content scripts are established inside InitFromValue() and we
   // just moved the extension.
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index d7ae504..c1a4d55 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -138,6 +138,10 @@ class CrxInstaller :
   // ExtensionsService on success, or delete it on failure.
   scoped_ptr<Extension> extension_;
 
+  // If non-empty, contains the current version of the extension we're
+  // installing (for upgrades).
+  std::string current_version_;
+
   // The icon we will display in the installation UI, if any.
   scoped_ptr<SkBitmap> install_icon_;
 
diff --git a/chrome/browser/extensions/extension_file_util.cc b/chrome/browser/extensions/extension_file_util.cc
index bf6cd9c..9c15da3 100644
--- a/chrome/browser/extensions/extension_file_util.cc
+++ b/chrome/browser/extensions/extension_file_util.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
+#include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_l10n_util.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -17,6 +18,8 @@
 namespace extension_file_util {
 
 const char kInstallDirectoryName[] = "Extensions";
+// TODO(mpcomplete): obsolete. remove after migration period.
+// http://code.google.com/p/chromium/issues/detail?id=19733
 const char kCurrentVersionFileName[] = "Current Version";
 
 bool MoveDirSafely(const FilePath& source_dir, const FilePath& dest_dir) {
@@ -37,76 +40,34 @@ bool MoveDirSafely(const FilePath& source_dir, const FilePath& dest_dir) {
   return true;
 }
 
-bool SetCurrentVersion(const FilePath& dest_dir, const std::string& version,
-                       std::string* error) {
-  // Write out the new CurrentVersion file.
-  // <profile>/Extension/<name>/CurrentVersion
-  FilePath current_version = dest_dir.AppendASCII(kCurrentVersionFileName);
-  FilePath current_version_old =
-    current_version.InsertBeforeExtension(FILE_PATH_LITERAL("_old"));
-  if (file_util::PathExists(current_version_old)) {
-    if (!file_util::Delete(current_version_old, false)) {
-      *error = "Couldn't remove CurrentVersion_old file.";
-      return false;
-    }
-  }
-
-  if (file_util::PathExists(current_version)) {
-    if (!file_util::Move(current_version, current_version_old)) {
-      *error = "Couldn't move CurrentVersion file.";
-      return false;
-    }
-  }
-  net::FileStream stream;
-  int flags = base::PLATFORM_FILE_CREATE_ALWAYS | base::PLATFORM_FILE_WRITE;
-  if (stream.Open(current_version, flags) != 0)
-    return false;
-  if (stream.Write(version.c_str(), version.size(), NULL) < 0) {
-    // Restore the old CurrentVersion.
-    if (file_util::PathExists(current_version_old)) {
-      if (!file_util::Move(current_version_old, current_version)) {
-        LOG(WARNING) << "couldn't restore " << current_version_old.value() <<
-            " to " << current_version.value();
-
-        // TODO(erikkay): This is an ugly state to be in.  Try harder?
-      }
-    }
-    *error = "Couldn't create CurrentVersion file.";
-    return false;
-  }
-  return true;
-}
-
-bool ReadCurrentVersion(const FilePath& dir, std::string* version_string) {
-  FilePath current_version = dir.AppendASCII(kCurrentVersionFileName);
-  if (file_util::PathExists(current_version)) {
-    if (file_util::ReadFileToString(current_version, version_string)) {
-      TrimWhitespaceASCII(*version_string, TRIM_ALL, version_string);
-      return true;
-    }
-  }
-  return false;
-}
-
 Extension::InstallType CompareToInstalledVersion(
-    const FilePath& install_directory, const std::string& id,
-    const std::string& new_version_str, std::string *current_version_str) {
-  CHECK(current_version_str);
-  FilePath dir(install_directory.AppendASCII(id.c_str()));
-  if (!ReadCurrentVersion(dir, current_version_str))
+    const FilePath& extensions_dir,
+    const std::string& extension_id,
+    const std::string& current_version_str,
+    const std::string& new_version_str,
+    FilePath* version_dir) {
+  FilePath dest_dir = extensions_dir.AppendASCII(extension_id);
+  FilePath current_version_dir = dest_dir.AppendASCII(current_version_str);
+  *version_dir = dest_dir.AppendASCII(new_version_str);
+
+  if (current_version_str.empty())
     return Extension::NEW_INSTALL;
 
   scoped_ptr<Version> current_version(
-    Version::GetVersionFromString(*current_version_str));
+    Version::GetVersionFromString(current_version_str));
   scoped_ptr<Version> new_version(
     Version::GetVersionFromString(new_version_str));
   int comp = new_version->CompareTo(*current_version);
   if (comp > 0)
     return Extension::UPGRADE;
-  else if (comp == 0)
-    return Extension::REINSTALL;
-  else
+  if (comp < 0)
     return Extension::DOWNGRADE;
+
+  // Same version. Treat corrupted existing installation as new install case.
+  if (!SanityCheckExtension(current_version_dir))
+    return Extension::NEW_INSTALL;
+
+  return Extension::REINSTALL;
 }
 
 bool SanityCheckExtension(const FilePath& dir) {
@@ -118,44 +79,17 @@ bool SanityCheckExtension(const FilePath& dir) {
 }
 
 bool InstallExtension(const FilePath& src_dir,
-                      const FilePath& extensions_dir,
-                      const std::string& extension_id,
-                      const std::string& extension_version,
-                      FilePath* version_dir,
-                      Extension::InstallType* install_type,
+                      const FilePath& version_dir,
                       std::string* error) {
-  FilePath dest_dir = extensions_dir.AppendASCII(extension_id);
-  *version_dir = dest_dir.AppendASCII(extension_version);
-
-  std::string current_version;
-  *install_type = CompareToInstalledVersion(
-      extensions_dir, extension_id, extension_version, &current_version);
-
-  // Do not allow downgrade.
-  if (*install_type == Extension::DOWNGRADE)
-    return true;
-
-  if (*install_type == Extension::REINSTALL) {
-    if (!SanityCheckExtension(*version_dir)) {
-      // Treat corrupted existing installation as new install case.
-      *install_type = Extension::NEW_INSTALL;
-    } else {
-      return true;
-    }
-  }
-
   // If anything fails after this, we want to delete the extension dir.
   ScopedTempDir scoped_version_dir;
-  scoped_version_dir.Set(*version_dir);
+  scoped_version_dir.Set(version_dir);
 
-  if (!MoveDirSafely(src_dir, *version_dir)) {
+  if (!MoveDirSafely(src_dir, version_dir)) {
     *error = "Could not move extension directory into profile.";
     return false;
   }
 
-  if (!SetCurrentVersion(dest_dir, extension_version, error))
-    return false;
-
   scoped_version_dir.Take();
   return true;
 }
@@ -184,13 +118,20 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
                                 require_key, error))
     return NULL;
 
+  if (!ValidateExtension(extension.get(), error))
+    return NULL;
+
+  return extension.release();
+}
+
+bool ValidateExtension(Extension* extension, std::string* error) {
   // Validate icons exist.
   for (std::map<int, std::string>::const_iterator iter =
        extension->icons().begin(); iter != extension->icons().end(); ++iter) {
     if (!file_util::PathExists(extension->GetResourcePath(iter->second))) {
       *error = StringPrintf("Could not load extension icon '%s'.",
                             iter->second.c_str());
-      return NULL;
+      return false;
     }
   }
 
@@ -207,14 +148,14 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
             *error = StringPrintf(
                 "Could not load '%s' for theme.",
                 WideToUTF8(image_path.ToWStringHack()).c_str());
-            return NULL;
+            return false;
           }
         }
       }
     }
 
     // Themes cannot contain other extension types.
-    return extension.release();
+    return true;
   }
 
   // Validate that claimed script resources actually exist.
@@ -226,7 +167,7 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
       if (!file_util::PathExists(path)) {
         *error = StringPrintf("Could not load '%s' for content script.",
                               WideToUTF8(path.ToWStringHack()).c_str());
-        return NULL;
+        return false;
       }
     }
 
@@ -235,7 +176,7 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
       if (!file_util::PathExists(path)) {
         *error = StringPrintf("Could not load '%s' for content script.",
                               WideToUTF8(path.ToWStringHack()).c_str());
-        return NULL;
+        return false;
       }
     }
   }
@@ -246,7 +187,7 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
     if (!file_util::PathExists(plugin.path)) {
       *error = StringPrintf("Could not load '%s' for plugin.",
                             WideToUTF8(plugin.path.ToWStringHack()).c_str());
-      return NULL;
+      return false;
     }
   }
 
@@ -257,7 +198,7 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
     if (!file_util::PathExists(blacklist.path)) {
       *error = StringPrintf("Could not load '%s' for privacy blacklist.",
                             WideToUTF8(blacklist.path.ToWStringHack()).c_str());
-      return NULL;
+      return false;
     }
   }
 
@@ -272,33 +213,34 @@ Extension* LoadExtension(const FilePath& extension_path, bool require_key,
       if (!file_util::PathExists(extension->GetResourcePath(*iter))) {
         *error = StringPrintf("Could not load icon '%s' for page action.",
                               iter->c_str());
-        return NULL;
+        return false;
       }
     }
   }
 
   // Load locale information if available.
-  FilePath locale_path = extension_path.AppendASCII(Extension::kLocaleFolder);
+  FilePath locale_path =
+      extension->path().AppendASCII(Extension::kLocaleFolder);
   if (file_util::PathExists(locale_path)) {
     if (!extension_l10n_util::AddValidLocales(locale_path,
-                                              extension.get(),
+                                              extension,
                                               error)) {
-      return NULL;
+      return false;
     }
 
-    if (!extension_l10n_util::ValidateDefaultLocale(extension.get())) {
+    if (!extension_l10n_util::ValidateDefaultLocale(extension)) {
       *error = extension_manifest_errors::kLocalesNoDefaultLocaleSpecified;
-      return NULL;
+      return false;
     }
   }
 
   // Check children of extension root to see if any of them start with _ and is
   // not on the reserved list.
-  if (!CheckForIllegalFilenames(extension_path, error)) {
-    return NULL;
+  if (!CheckForIllegalFilenames(extension->path(), error)) {
+    return false;
   }
 
-  return extension.release();
+  return true;
 }
 
 void UninstallExtension(const std::string& id, const FilePath& extensions_dir) {
@@ -314,8 +256,7 @@ void UninstallExtension(const std::string& id, const FilePath& extensions_dir) {
   FilePath current_version_file = extension_root.AppendASCII(
       kCurrentVersionFileName);
   if (!file_util::PathExists(current_version_file)) {
-    LOG(WARNING) << "Extension " << id
-                 << " does not have a Current Version file.";
+    // This is OK, since we're phasing out the current version file.
   } else {
     if (!file_util::Delete(current_version_file, false)) {
       LOG(WARNING) << "Could not delete Current Version file for extension "
@@ -332,7 +273,8 @@ void UninstallExtension(const std::string& id, const FilePath& extensions_dir) {
     LOG(WARNING) << "Could not delete directory for extension " << id;
 }
 
-void GarbageCollectExtensions(const FilePath& install_directory) {
+void GarbageCollectExtensions(const FilePath& install_directory,
+                              const std::set<std::string>& installed_ids) {
   // Nothing to clean up if it doesn't exist.
   if (!file_util::DirectoryExists(install_directory))
     return;
@@ -347,13 +289,11 @@ void GarbageCollectExtensions(const FilePath& install_directory) {
     std::string extension_id = WideToASCII(
         extension_path.BaseName().ToWStringHack());
 
-    // If there is no Current Version file, just delete the directory and move
-    // on. This can legitimately happen when an uninstall does not complete, for
-    // example, when a plugin is in use at uninstall time.
-    FilePath current_version_path = extension_path.AppendASCII(
-        kCurrentVersionFileName);
-    if (!file_util::PathExists(current_version_path)) {
-      LOG(INFO) << "Deleting incomplete install for directory "
+    // If there is no entry in the prefs file, just delete the directory and
+    // move on. This can legitimately happen when an uninstall does not
+    // complete, for example, when a plugin is in use at uninstall time.
+    if (installed_ids.count(extension_id) == 0) {
+      LOG(INFO) << "Deleting unreferenced install for directory "
                 << WideToASCII(extension_path.ToWStringHack()) << ".";
       file_util::Delete(extension_path, true);  // Recursive.
       continue;
@@ -409,4 +349,4 @@ bool CheckForIllegalFilenames(const FilePath& extension_path,
   return true;
 }
 
-}  // extensionfile_util
+}  // extension_file_util
diff --git a/chrome/browser/extensions/extension_file_util.h b/chrome/browser/extensions/extension_file_util.h
index c2eb5c9..9c13ea2 100644
--- a/chrome/browser/extensions/extension_file_util.h
+++ b/chrome/browser/extensions/extension_file_util.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_FILE_UTIL_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_FILE_UTIL_H_
 
+#include <set>
 #include <string>
 
 #include "base/file_path.h"
@@ -33,24 +34,26 @@ bool SetCurrentVersion(const FilePath& dest_dir, const std::string& version,
 // Reads the Current Version file.
 bool ReadCurrentVersion(const FilePath& dir, std::string* version_string);
 
-// Determine what type of install (new, upgrade, overinstall, downgrade) a given
-// id/version combination is. Also returns the current version, if any, in
-// current_version_str.
+// Determine what type of install it is (new, upgrade, overinstall, downgrade)
+// given the current version and a newly installing version. |extensions_dir| is
+// the root directory containing all extensions in the user profile.
+// |extension_id| and |current_version_str| are the id and version of the
+// extension contained in |src_dir|, if any.
+//
+// Returns the full path to the destination version directory and the type of
+// install that was attempted.
 Extension::InstallType CompareToInstalledVersion(
-    const FilePath& install_directory, const std::string& id,
-    const std::string& new_version_str, std::string *current_version_str);
+    const FilePath& extensions_dir,
+    const std::string& extension_id,
+    const std::string& current_version_str,
+    const std::string& new_version_str,
+    FilePath* version_dir);
 
 // Sanity check that the directory has the minimum files to be a working
 // extension.
 bool SanityCheckExtension(const FilePath& extension_root);
 
-// Installs an extension unpacked in |src_dir|. |extensions_dir| is the root
-// directory containing all extensions in the user profile. |extension_id| and
-// |extension_version| are the id and version of the extension contained in
-// |src_dir|.
-//
-// Returns the full path to the destination version directory and the type of
-// install that was attempted.
+// Installs an extension unpacked in |src_dir|.
 //
 // On failure, also returns an error message.
 //
@@ -58,25 +61,26 @@ bool SanityCheckExtension(const FilePath& extension_root);
 // overinstall of previous verisons of the extension. In that case, the function
 // returns true and install_type is populated.
 bool InstallExtension(const FilePath& src_dir,
-                      const FilePath& extensions_dir,
-                      const std::string& extension_id,
-                      const std::string& extension_version,
-                      FilePath* version_dir,
-                      Extension::InstallType* install_type,
+                      const FilePath& version_dir,
                       std::string* error);
 
-// Load an extension from the specified directory. Returns NULL on failure, with
-// a description of the error in |error|.
+// Loads and validates an extension from the specified directory. Returns NULL
+// on failure, with a description of the error in |error|.
 Extension* LoadExtension(const FilePath& extension_root, bool require_key,
                          std::string* error);
 
+// Returns true if the given extension object is valid and consistent.
+// Otherwise, a description of the error is returned in |error|.
+bool ValidateExtension(Extension* extension, std::string* error);
+
 // Uninstalls the extension |id| from the install directory |extensions_dir|.
 void UninstallExtension(const std::string& id, const FilePath& extensions_dir);
 
 // Clean up directories that aren't valid extensions from the install directory.
 // TODO(aa): Also consider passing in a list of known current extensions and
 // removing others?
-void GarbageCollectExtensions(const FilePath& extensions_dir);
+void GarbageCollectExtensions(const FilePath& extensions_dir,
+                              const std::set<std::string>& installed_ids);
 
 // We need to reserve the namespace of entries that start with "_" for future
 // use by Chrome.
diff --git a/chrome/browser/extensions/extension_file_util_unittest.cc b/chrome/browser/extensions/extension_file_util_unittest.cc
index 73dfb46..9f99606 100644
--- a/chrome/browser/extensions/extension_file_util_unittest.cc
+++ b/chrome/browser/extensions/extension_file_util_unittest.cc
@@ -61,110 +61,54 @@ TEST(ExtensionFileUtil, MoveDirSafely) {
   ASSERT_FALSE(file_util::PathExists(src_path));
 }
 
-TEST(ExtensionFileUtil, SetCurrentVersion) {
-  // Create an empty test directory.
-  ScopedTempDir temp;
-  ASSERT_TRUE(temp.CreateUniqueTempDir());
-
-  // Set its version.
-  std::string error;
-  std::string version = "1.0";
-  ASSERT_TRUE(extension_file_util::SetCurrentVersion(temp.path(), version,
-                                                     &error));
-
-  // Test the result.
-  std::string version_out;
-  ASSERT_TRUE(file_util::ReadFileToString(
-      temp.path().AppendASCII(extension_file_util::kCurrentVersionFileName),
-      &version_out));
-  ASSERT_EQ(version, version_out);
-
-  // Try again, overwriting the old value.
-  version = "2.0";
-  version_out.clear();
-  ASSERT_TRUE(extension_file_util::SetCurrentVersion(temp.path(), version,
-                                                     &error));
-  ASSERT_TRUE(file_util::ReadFileToString(
-      temp.path().AppendASCII(extension_file_util::kCurrentVersionFileName),
-      &version_out));
-  ASSERT_EQ(version, version_out);
-}
-
-TEST(ExtensionFileUtil, ReadCurrentVersion) {
-  // Read the version from a valid extension.
-  FilePath extension_path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extension_path));
-  extension_path = extension_path.AppendASCII("extensions")
-      .AppendASCII("good")
-      .AppendASCII("Extensions")
-      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj");
-
-  std::string version;
-  ASSERT_TRUE(extension_file_util::ReadCurrentVersion(extension_path,
-                                                      &version));
-  ASSERT_EQ("1.0.0.0", version);
-
-  // Create an invalid extension and read its current version.
-  ScopedTempDir temp;
-  ASSERT_TRUE(temp.CreateUniqueTempDir());
-  ASSERT_FALSE(extension_file_util::ReadCurrentVersion(temp.path(), &version));
-}
-
 TEST(ExtensionFileUtil, CompareToInstalledVersion) {
   // Compare to an existing extension.
-  FilePath install_directory;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_directory));
-  install_directory = install_directory.AppendASCII("extensions")
+  FilePath install_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &install_dir));
+  install_dir = install_dir.AppendASCII("extensions")
       .AppendASCII("good")
       .AppendASCII("Extensions");
 
-  std::string id = "behllobkkfkfnphdnhnkndlbkcpglgmj";
-  std::string version = "1.0.0.0";
-  std::string version_out;
+  const std::string kId = "behllobkkfkfnphdnhnkndlbkcpglgmj";
+  const std::string kCurrentVersion = "1.0.0.0";
+
+  FilePath version_dir;
 
-  ASSERT_EQ(Extension::UPGRADE, extension_file_util::CompareToInstalledVersion(
-    install_directory, id, "1.0.0.1", &version_out));
-  ASSERT_EQ(version, version_out);
+  ASSERT_EQ(Extension::UPGRADE,
+            extension_file_util::CompareToInstalledVersion(
+                install_dir, kId, kCurrentVersion, "1.0.0.1", &version_dir));
 
-  version_out.clear();
   ASSERT_EQ(Extension::REINSTALL,
             extension_file_util::CompareToInstalledVersion(
-                install_directory, id, "1.0.0.0", &version_out));
-  ASSERT_EQ(version, version_out);
+                install_dir, kId, kCurrentVersion, "1.0.0.0", &version_dir));
 
-  version_out.clear();
   ASSERT_EQ(Extension::REINSTALL,
             extension_file_util::CompareToInstalledVersion(
-                install_directory, id, "1.0.0", &version_out));
-  ASSERT_EQ(version, version_out);
+                install_dir, kId, kCurrentVersion, "1.0.0", &version_dir));
 
-  version_out.clear();
   ASSERT_EQ(Extension::DOWNGRADE,
             extension_file_util::CompareToInstalledVersion(
-                install_directory, id, "0.0.1.0", &version_out));
-  ASSERT_EQ(version, version_out);
+                install_dir, kId, kCurrentVersion, "0.0.1.0", &version_dir));
 
-  // Compare to an extension that is missing its Current Version file.
+  // Compare to an extension that is missing its manifest file.
   ScopedTempDir temp;
   ASSERT_TRUE(temp.CreateUniqueTempDir());
-  FilePath src = install_directory.AppendASCII(id).AppendASCII(version);
-  FilePath dest = temp.path().AppendASCII(id).AppendASCII(version);
+  FilePath src = install_dir.AppendASCII(kId).AppendASCII(kCurrentVersion);
+  FilePath dest = temp.path().AppendASCII(kId).AppendASCII(kCurrentVersion);
   ASSERT_TRUE(file_util::CreateDirectory(dest.DirName()));
   ASSERT_TRUE(file_util::CopyDirectory(src, dest, true));
+  ASSERT_TRUE(file_util::Delete(dest.AppendASCII("manifest.json"), false));
 
-  version_out.clear();
   ASSERT_EQ(Extension::NEW_INSTALL,
             extension_file_util::CompareToInstalledVersion(
-                temp.path(), id, "0.0.1.0", &version_out));
-  ASSERT_EQ("", version_out);
+                temp.path(), kId, kCurrentVersion, "1.0.0", &version_dir));
 
-  // Compare to a completely non-existent extension.
-  version_out.clear();
+  // Compare to a non-existent extension.
+  const std::string kMissingVersion = "";
+  const std::string kBadId = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
   ASSERT_EQ(Extension::NEW_INSTALL,
             extension_file_util::CompareToInstalledVersion(
-                temp.path(), "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", "0.0.1.0",
-                &version_out));
-  ASSERT_EQ("", version_out);
+                temp.path(), kBadId, kMissingVersion, "1.0.0", &version_dir));
 }
 
 // Creates minimal manifest, with or without default_locale section.
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 84ee99d..4583944 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -24,6 +24,12 @@ const wchar_t kPrefState[] = L"state";
 // The path to the current version's manifest file.
 const wchar_t kPrefPath[] = L"path";
 
+// The dictionary containing the extension's manifest.
+const wchar_t kPrefManifest[] = L"manifest";
+
+// The version number.
+const wchar_t kPrefVersion[] = L"manifest.version";
+
 // Indicates if an extension is blacklisted:
 const wchar_t kPrefBlacklist[] = L"blacklist";
 
@@ -43,6 +49,7 @@ InstalledExtensions::~InstalledExtensions() {
 
 void InstalledExtensions::VisitInstalledExtensions(
     InstalledExtensions::Callback *callback) {
+  scoped_ptr<InstalledExtensions::Callback> cleanup(callback);
   DictionaryValue::key_iterator extension_id = extension_data_->begin_keys();
   for (; extension_id != extension_data_->end_keys(); ++extension_id) {
     DictionaryValue* ext;
@@ -74,9 +81,16 @@ void InstalledExtensions::VisitInstalledExtensions(
       NOTREACHED();
       continue;
     }
+    DictionaryValue* manifest = NULL;
+    if (!ext->GetDictionary(kPrefManifest, &manifest)) {
+      LOG(WARNING) << "Missing manifest for extension " << *extension_id;
+      // Just a warning for now.
+    }
+
     Extension::Location location =
         static_cast<Extension::Location>(location_value);
-    callback->Run(WideToASCII(*extension_id), FilePath(path), location);
+    callback->Run(manifest, WideToASCII(*extension_id), FilePath(path),
+                  location);
   }
 }
 
@@ -300,13 +314,18 @@ void ExtensionPrefs::SetShelfToolstripOrder(const URLList& urls) {
 
 void ExtensionPrefs::OnExtensionInstalled(Extension* extension) {
   const std::string& id = extension->id();
-  UpdateExtensionPref(id, kPrefState,
-                      Value::CreateIntegerValue(Extension::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_,
       extension->path(), NULL);
   UpdateExtensionPref(id, kPrefPath, Value::CreateStringValue(path));
+  UpdateExtensionPref(id, kPrefManifest,
+                      extension->manifest_value()->DeepCopy());
   prefs_->SavePersistentPrefs();
 }
 
@@ -351,6 +370,25 @@ void ExtensionPrefs::SetExtensionState(Extension* extension,
   prefs_->SavePersistentPrefs();
 }
 
+std::string ExtensionPrefs::GetVersionString(const std::string& extension_id) {
+  DictionaryValue* extension = GetExtensionPref(extension_id);
+  if (!extension)
+    return std::string();
+
+  std::string version;
+  if (!extension->GetString(kPrefVersion, &version)) {
+    LOG(ERROR) << "Bad or missing pref 'version' for extension '"
+               << extension_id << "'";
+  }
+
+  return version;
+}
+
+void ExtensionPrefs::MigrateToPrefs(Extension* extension) {
+  UpdateExtensionPref(extension->id(), kPrefManifest,
+                      extension->manifest_value()->DeepCopy());
+}
+
 bool ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id,
                                          const std::wstring& key,
                                          Value* data_value) {
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 291556c..59adfd8 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -49,6 +49,13 @@ class ExtensionPrefs {
   // Called to change the extension's state when it is enabled/disabled.
   void SetExtensionState(Extension* extension, Extension::State);
 
+  // Returns the version string for the currently installed extension, or
+  // the empty string if not found.
+  std::string GetVersionString(const std::string& extension_id);
+
+  // Ensure old extensions have fully up-to-date prefs values.
+  void MigrateToPrefs(Extension* extension);
+
   // Returns base extensions install directory.
   const FilePath& install_directory() const { return install_directory_; }
 
@@ -106,19 +113,16 @@ class InstalledExtensions {
   explicit InstalledExtensions(ExtensionPrefs* prefs);
   ~InstalledExtensions();
 
-  typedef Callback3<const std::string&,
+  typedef Callback4<DictionaryValue*,
+                    const std::string&,
                     const FilePath&,
                     Extension::Location>::Type Callback;
 
   // Runs |callback| for each installed extension with the path to the
   // version directory and the location. Blacklisted extensions won't trigger
-  // the callback.
+  // the callback. Ownership of |callback| is transferred to callee.
   void VisitInstalledExtensions(Callback *callback);
 
-  // Same as above, but only for the given extension.
-  void VisitInstalledExtension(const std::string& extension_id,
-                               Callback *callback);
-
  private:
   // A copy of the extensions pref dictionary so that this can be passed
   // around without a dependency on prefs.
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index f571c08..6dcc015 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -28,6 +28,31 @@
 #include "chrome/browser/extensions/external_registry_extension_provider_win.h"
 #endif
 
+namespace {
+
+// Helper class to collect the IDs of every extension listed in the prefs.
+class InstalledExtensionSet {
+ public:
+  InstalledExtensionSet(InstalledExtensions* installed) {
+    scoped_ptr<InstalledExtensions> cleanup(installed);
+    installed->VisitInstalledExtensions(
+        NewCallback(this, &InstalledExtensionSet::ExtensionVisited));
+  }
+
+  const std::set<std::string>& extensions() { return extensions_; }
+
+ private:
+  void ExtensionVisited(
+      DictionaryValue* manifest, const std::string& id,
+      const FilePath& path, Extension::Location location) {
+    extensions_.insert(id);
+  }
+
+  std::set<std::string> extensions_;
+};
+
+} // namespace
+
 // ExtensionsService.
 
 const char* ExtensionsService::kInstallDirectoryName = "Extensions";
@@ -283,8 +308,11 @@ void ExtensionsService::ReloadExtensions() {
 }
 
 void ExtensionsService::GarbageCollectExtensions() {
+  InstalledExtensionSet installed(
+      new InstalledExtensions(extension_prefs_.get()));
   backend_loop_->PostTask(FROM_HERE, NewRunnableFunction(
-      &extension_file_util::GarbageCollectExtensions, install_directory_));
+      &extension_file_util::GarbageCollectExtensions, install_directory_,
+      installed.extensions()));
 }
 
 void ExtensionsService::OnLoadedInstalledExtensions() {
@@ -347,6 +375,8 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
 
       switch (extension_prefs_->GetExtensionState(extension->id())) {
         case Extension::ENABLED:
+          if (extension->location() != Extension::LOAD)
+            extension_prefs_->MigrateToPrefs(extension.get());
           enabled_extensions.push_back(extension.get());
           extensions_.push_back(extension.release());
           break;
@@ -378,11 +408,7 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
 }
 
 void ExtensionsService::OnExtensionInstalled(Extension* extension) {
-  // Make sure we don't enable a disabled extension.
-  if (extension_prefs_->GetExtensionState(extension->id()) !=
-      Extension::DISABLED) {
-    extension_prefs_->OnExtensionInstalled(extension);
-  }
+  extension_prefs_->OnExtensionInstalled(extension);
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
   // to apply it.
@@ -516,9 +542,8 @@ void ExtensionsServiceBackend::LoadInstalledExtensions(
   alert_on_error_ = false;
 
   // Call LoadInstalledExtension for each extension |installed| knows about.
-  scoped_ptr<InstalledExtensions::Callback> callback(
+  installed->VisitInstalledExtensions(
       NewCallback(this, &ExtensionsServiceBackend::LoadInstalledExtension));
-  installed->VisitInstalledExtensions(callback.get());
 
   frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
       frontend_, &ExtensionsService::OnLoadedInstalledExtensions));
@@ -555,7 +580,8 @@ void ExtensionsServiceBackend::LoadSingleExtension(
 }
 
 void ExtensionsServiceBackend::LoadInstalledExtension(
-    const std::string& id, const FilePath& path, Extension::Location location) {
+    DictionaryValue* manifest, const std::string& id,
+    const FilePath& path, Extension::Location location) {
   if (CheckExternalUninstall(id, location)) {
     frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
         frontend_,
@@ -567,10 +593,20 @@ void ExtensionsServiceBackend::LoadInstalledExtension(
   }
 
   std::string error;
-  Extension* extension = extension_file_util::LoadExtension(
-      path,
-      true,  // Require id
-      &error);
+  Extension* extension = NULL;
+  if (manifest) {
+    scoped_ptr<Extension> tmp(new Extension(path));
+    if (tmp->InitFromValue(*manifest, true, &error) &&
+        extension_file_util::ValidateExtension(tmp.get(), &error)) {
+      extension = tmp.release();
+    }
+  } else {
+    // TODO(mpcomplete): obsolete. remove after migration period.
+    // http://code.google.com/p/chromium/issues/detail?id=19733
+    extension = extension_file_util::LoadExtension(path,
+                                                   true,  // Require id
+                                                   &error);
+  }
 
   if (!extension) {
     ReportExtensionLoadError(path, error);
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index fafe97e..6a56749 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -303,7 +303,9 @@ class ExtensionsServiceBackend
                                         Extension::Location location);
  private:
   // Loads a single installed extension.
-  void LoadInstalledExtension(const std::string& id, const FilePath& path,
+  void LoadInstalledExtension(DictionaryValue* manifest,
+                              const std::string& id,
+                              const FilePath& path,
                               Extension::Location location);
 
   // Finish installing the extension in |crx_path| after it has been unpacked to
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 3f464d5..d19a3d3 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -636,25 +636,21 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectoryFail) {
 // Test that partially deleted extensions are cleaned up during startup
 // Test loading bad extensions from the profile directory.
 TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
-  InitializeEmptyExtensionsService();
-
-  FilePath source_path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
-  source_path = source_path.AppendASCII("extensions")
-                           .AppendASCII("good")
-                           .AppendASCII("Extensions");
-
-  file_util::Delete(extensions_install_dir_, true);
+  FilePath source_install_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_install_dir));
+  source_install_dir = source_install_dir
+      .AppendASCII("extensions")
+      .AppendASCII("good")
+      .AppendASCII("Extensions");
+  FilePath pref_path = source_install_dir
+      .DirName()
+      .AppendASCII("Preferences");
 
-  // Recursive.
-  file_util::CopyDirectory(source_path, extensions_install_dir_, true);
+  InitializeInstalledExtensionsService(pref_path, source_install_dir);
 
-  // Simulate that one of them got partially deleted by deling the
-  // Current Version file.
-  FilePath vers = extensions_install_dir_
-      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")
-      .AppendASCII(ExtensionsService::kCurrentVersionFileName);
-  ASSERT_TRUE(file_util::Delete(vers, false));  // not recursive
+  // Simulate that one of them got partially deleted by clearing its pref.
+  prefs_->GetMutableDictionary(L"extensions.settings")->
+      Remove(L"behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
 
   service_->Init();
   loop_.RunAllPending();
@@ -669,8 +665,9 @@ TEST_F(ExtensionsServiceTest, CleanupOnStartup) {
   EXPECT_EQ(2u, count);
 
   // And extension1 dir should now be toast.
-  vers = vers.DirName();
-  ASSERT_FALSE(file_util::PathExists(vers));
+  FilePath extension_dir = extensions_install_dir_
+      .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj");
+  ASSERT_FALSE(file_util::PathExists(extension_dir));
 }
 
 // Test installing extensions. This test tries to install few extensions using
@@ -1148,19 +1145,6 @@ TEST_F(ExtensionsServiceTest, UninstallExtension) {
 
   // The directory should be gone.
   EXPECT_FALSE(file_util::PathExists(extension_path));
-
-  // Try uinstalling one that doesn't have a Current Version file for some
-  // reason.
-  unloaded_id_.clear();
-  InstallExtension(path, true);
-  FilePath current_version_file =
-      extension_path.AppendASCII(ExtensionsService::kCurrentVersionFileName);
-  EXPECT_TRUE(file_util::Delete(current_version_file, true));
-  service_->UninstallExtension(extension_id, false);
-  loop_.RunAllPending();
-  EXPECT_FALSE(file_util::PathExists(extension_path));
-
-  ValidatePrefKeyCount(0);
 }
 
 // Tests loading single extensions (like --load-extension)
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 5ceee67..1eb43e3 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -459,7 +459,7 @@ Extension::Extension(const FilePath& path)
 // util class in base:
 // http://code.google.com/p/chromium/issues/detail?id=13572
 bool Extension::ParsePEMKeyBytes(const std::string& input,
-                                        std::string* output) {
+                                 std::string* output) {
   DCHECK(output);
   if (!output)
     return false;
@@ -498,8 +498,8 @@ bool Extension::ProducePEM(const std::string& input,
 }
 
 bool Extension::FormatPEMForFileOutput(const std::string input,
-                                              std::string* output,
-                                              bool is_public) {
+                                       std::string* output,
+                                       bool is_public) {
   CHECK(output);
   if (input.length() == 0)
     return false;
@@ -550,6 +550,9 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     ConvertHexadecimalToIDAlphabet(&id_);
   }
 
+  // Make a copy of the manifest so we can store it in prefs.
+  manifest_value_.reset(static_cast<DictionaryValue*>(source.DeepCopy()));
+
   // Initialize the URL.
   extension_url_ = GURL(std::string(chrome::kExtensionScheme) +
                         chrome::kStandardSchemeSeparator + id_ + "/");
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index f3acd1c..34685fc 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -237,6 +237,10 @@ class Extension {
   // an empty FilePath if the extension does not have that icon.
   FilePath GetIconPath(Icons icon);
 
+  const DictionaryValue* manifest_value() const {
+    return manifest_value_.get();
+  }
+
   // Returns a list of all locales supported by the extension.
   const std::set<std::string>& supported_locales() const {
     return supported_locales_;
@@ -363,7 +367,10 @@ class Extension {
   // URL for fetching an update manifest
   GURL update_url_;
 
-  // List of all locales extension supports.
+  // A copy of the manifest that this extension was created from.
+  scoped_ptr<DictionaryValue> manifest_value_;
+
+    // List of all locales extension supports.
   std::set<std::string> supported_locales_;
 
   // Default locale, used for fallback.
diff --git a/chrome/test/data/extensions/good/Extensions/behllobkkfkfnphdnhnkndlbkcpglgmj/Current Version b/chrome/test/data/extensions/good/Extensions/behllobkkfkfnphdnhnkndlbkcpglgmj/Current Version
deleted file mode 100644
index 1921233..0000000
--- a/chrome/test/data/extensions/good/Extensions/behllobkkfkfnphdnhnkndlbkcpglgmj/Current Version
+++ /dev/null
@@ -1 +0,0 @@
-1.0.0.0
diff --git a/chrome/test/data/extensions/good/Extensions/bjafgdebaacbbbecmhlhpofkepfkgcpa/Current Version b/chrome/test/data/extensions/good/Extensions/bjafgdebaacbbbecmhlhpofkepfkgcpa/Current Version
deleted file mode 100644
index d3827e7..0000000
--- a/chrome/test/data/extensions/good/Extensions/bjafgdebaacbbbecmhlhpofkepfkgcpa/Current Version
+++ /dev/null
@@ -1 +0,0 @@
-1.0
diff --git a/chrome/test/data/extensions/good/Extensions/hpiknbiabeeppbpihjehijgoemciehgk/Current Version b/chrome/test/data/extensions/good/Extensions/hpiknbiabeeppbpihjehijgoemciehgk/Current Version
deleted file mode 100644
index 0cfbf08..0000000
--- a/chrome/test/data/extensions/good/Extensions/hpiknbiabeeppbpihjehijgoemciehgk/Current Version
+++ /dev/null
@@ -1 +0,0 @@
-2