Use less temporary extension IDs during pack and unpack.

This fixes issues related to permission whitelists, which are based on the extension ID. 

BUG=113696
TEST=ExtensionServiceTest.InstallWhitelistedExtension


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

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

9 files changed, 58 insertions, 36 deletions

diff --git a/chrome/common/chrome_utility_messages.h b/chrome/common/chrome_utility_messages.h
index 8a409a9..0f58ce5 100644
--- a/chrome/common/chrome_utility_messages.h
+++ b/chrome/common/chrome_utility_messages.h
@@ -54,8 +54,9 @@ IPC_STRUCT_TRAITS_END()
 // These are messages from the browser to the utility process.
 // Tell the utility process to unpack the given extension file in its
 // directory and verify that it is valid.
-IPC_MESSAGE_CONTROL3(ChromeUtilityMsg_UnpackExtension,
+IPC_MESSAGE_CONTROL4(ChromeUtilityMsg_UnpackExtension,
                      FilePath /* extension_filename */,
+                     std::string /* extension_id */,
                      int /* Extension::Location */,
                      int /* InitFromValue flags */)
 
diff --git a/chrome/common/extensions/api/extension_api_unittest.cc b/chrome/common/extensions/api/extension_api_unittest.cc
index be481d1..66c76f5 100644
--- a/chrome/common/extensions/api/extension_api_unittest.cc
+++ b/chrome/common/extensions/api/extension_api_unittest.cc
@@ -46,7 +46,7 @@ TEST(ExtensionAPI, Depends) {
   }
 
   std::string error;
-  scoped_refptr<Extension> extension(Extension::CreateWithId(
+  scoped_refptr<Extension> extension(Extension::Create(
       FilePath(), Extension::LOAD, manifest, Extension::NO_FLAGS, "x", &error));
   CHECK(extension.get());
   CHECK(error.empty());
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index a31d0b7..a9bb976 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -229,10 +229,24 @@ scoped_refptr<Extension> Extension::Create(const FilePath& path,
                                            const DictionaryValue& value,
                                            int flags,
                                            std::string* utf8_error) {
+  return Extension::Create(path,
+                           location,
+                           value,
+                           flags,
+                           std::string(),  // ID is ignored if empty.
+                           utf8_error);
+}
+
+scoped_refptr<Extension> Extension::Create(const FilePath& path,
+                                           Location location,
+                                           const DictionaryValue& value,
+                                           int flags,
+                                           const std::string& explicit_id,
+                                           std::string* utf8_error) {
   DCHECK(utf8_error);
   string16 error;
   scoped_refptr<Extension> extension = new Extension(path, location);
-
+  extension->id_ = explicit_id;
   if (!extension->InitFromValue(new extensions::Manifest(value.DeepCopy()),
                                 flags, &error)) {
     *utf8_error = UTF16ToUTF8(error);
@@ -241,19 +255,6 @@ scoped_refptr<Extension> Extension::Create(const FilePath& path,
   return extension;
 }
 
-scoped_refptr<Extension> Extension::CreateWithId(const FilePath& path,
-                                                 Location location,
-                                                 const DictionaryValue& value,
-                                                 int flags,
-                                                 const std::string& explicit_id,
-                                                 std::string* error) {
-  scoped_refptr<Extension> extension = Create(
-      path, location, value, flags, error);
-  if (extension.get())
-    extension->id_ = explicit_id;
-  return extension;
-}
-
 // static
 Extension::Location Extension::GetHigherPriorityLocation(
     Extension::Location loc1, Extension::Location loc2) {
@@ -1537,7 +1538,7 @@ bool Extension::InitFromValue(extensions::Manifest* manifest, int flags,
   if (manifest->HasKey(keys::kPublicKey)) {
     std::string public_key_bytes;
     if (!manifest->GetString(keys::kPublicKey,
-                         &public_key_) ||
+                             &public_key_) ||
         !ParsePEMKeyBytes(public_key_,
                           &public_key_bytes) ||
         !GenerateId(public_key_bytes, &id_)) {
@@ -1547,7 +1548,7 @@ bool Extension::InitFromValue(extensions::Manifest* manifest, int flags,
   } else if (flags & REQUIRE_KEY) {
     *error = ASCIIToUTF16(errors::kInvalidKey);
     return false;
-  } else {
+  } else if (id_.empty()) {
     // If there is a path, we generate the ID from it. This is useful for
     // development mode, because it keeps the ID stable across restarts and
     // reloading the extension.
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index f74bae6..b85c7cd 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -211,14 +211,13 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
                                          std::string* error);
 
   // In a few special circumstances, we want to create an Extension and give it
-  // an explicit id. Most consumers should just use the plain Create() method.
-  static scoped_refptr<Extension> CreateWithId(
-      const FilePath& path,
-      Location location,
-      const base::DictionaryValue& value,
-      int flags,
-      const std::string& explicit_id,
-      std::string* error);
+  // an explicit id. Most consumers should just use the other Create() method.
+  static scoped_refptr<Extension> Create(const FilePath& path,
+                                         Location location,
+                                         const base::DictionaryValue& value,
+                                         int flags,
+                                         const std::string& explicit_id,
+                                         std::string* error);
 
   // Given two install sources, return the one which should take priority
   // over the other. If an extension is installed from two sources A and B,
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index ebc0644..b30f65e 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -124,6 +124,15 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_path,
                                        Extension::Location location,
                                        int flags,
                                        std::string* error) {
+  return LoadExtension(
+      extension_path, std::string(), location, flags, error);
+}
+
+scoped_refptr<Extension> LoadExtension(const FilePath& extension_path,
+                                       std::string extension_id,
+                                       Extension::Location location,
+                                       int flags,
+                                       std::string* error) {
   scoped_ptr<DictionaryValue> manifest(LoadManifest(extension_path, error));
   if (!manifest.get())
     return NULL;
@@ -136,6 +145,7 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_path,
       location,
       *manifest,
       flags,
+      extension_id,
       error));
   if (!extension.get())
     return NULL;
diff --git a/chrome/common/extensions/extension_file_util.h b/chrome/common/extensions/extension_file_util.h
index c1241f1..9cffdda 100644
--- a/chrome/common/extensions/extension_file_util.h
+++ b/chrome/common/extensions/extension_file_util.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -46,6 +46,13 @@ scoped_refptr<Extension> LoadExtension(const FilePath& extension_root,
                                        int flags,
                                        std::string* error);
 
+// The same as LoadExtension except use the provided |extension_id|.
+scoped_refptr<Extension> LoadExtension(const FilePath& extension_root,
+                                       std::string extension_id,
+                                       Extension::Location location,
+                                       int flags,
+                                       std::string* error);
+
 // Loads an extension manifest from the specified directory. Returns NULL
 // on failure, with a description of the error in |error|.
 base::DictionaryValue* LoadManifest(const FilePath& extension_root,
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index e07f68c..caa8fac 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -85,9 +85,12 @@ bool PathContainsParentDirectory(const FilePath& path) {
 }  // namespace
 
 ExtensionUnpacker::ExtensionUnpacker(const FilePath& extension_path,
+                                     const std::string& extension_id,
                                      Extension::Location location,
                                      int creation_flags)
-    : extension_path_(extension_path), location_(location),
+    : extension_path_(extension_path),
+      extension_id_(extension_id),
+      location_(location),
       creation_flags_(creation_flags) {
 }
 
@@ -174,16 +177,13 @@ bool ExtensionUnpacker::Run() {
   if (!parsed_manifest_.get())
     return false;  // Error was already reported.
 
-  // NOTE: Since the unpacker doesn't have the extension's public_id, the
-  // InitFromValue is allowed to generate a temporary id for the extension.
-  // ANY CODE THAT FOLLOWS SHOULD NOT DEPEND ON THE CORRECT ID OF THIS
-  // EXTENSION.
   std::string error;
   scoped_refptr<Extension> extension(Extension::Create(
       temp_install_dir_,
       location_,
       *parsed_manifest_,
       creation_flags_,
+      extension_id_,
       &error));
   if (!extension.get()) {
     SetError(error);
diff --git a/chrome/common/extensions/extension_unpacker.h b/chrome/common/extensions/extension_unpacker.h
index d9c56d2..5c6c14c 100644
--- a/chrome/common/extensions/extension_unpacker.h
+++ b/chrome/common/extensions/extension_unpacker.h
@@ -28,9 +28,10 @@ class ExtensionUnpacker {
  public:
   typedef std::vector< Tuple2<SkBitmap, FilePath> > DecodedImages;
 
-  explicit ExtensionUnpacker(const FilePath& extension_path,
-                             Extension::Location location,
-                             int creation_flags);
+  ExtensionUnpacker(const FilePath& extension_path,
+                    const std::string& extension_id,
+                    Extension::Location location,
+                    int creation_flags);
   ~ExtensionUnpacker();
 
   // Install the extension file at |extension_path|.  Returns true on success.
@@ -88,6 +89,9 @@ class ExtensionUnpacker {
   // The extension to unpack.
   FilePath extension_path_;
 
+  // The extension ID if known.
+  std::string extension_id_;
+
   // The location to use for the created extension.
   Extension::Location location_;
 
diff --git a/chrome/common/extensions/extension_unpacker_unittest.cc b/chrome/common/extensions/extension_unpacker_unittest.cc
index 0d67720..a7b4e9f 100644
--- a/chrome/common/extensions/extension_unpacker_unittest.cc
+++ b/chrome/common/extensions/extension_unpacker_unittest.cc
@@ -44,7 +44,7 @@ public:
 
     unpacker_.reset(
         new ExtensionUnpacker(
-            crx_path, Extension::INTERNAL, Extension::NO_FLAGS));
+            crx_path, std::string(), Extension::INTERNAL, Extension::NO_FLAGS));
   }
 
  protected: