Generate IDs for --load-extension by hashing the path instead

of incrementing a static counter.

BUG=21281
TEST=Load an unpacked extension from chrome://extensions/ that
does not have an ID in its manifest. The resulting ID should
be random-looking, not something like "aaaaaa".

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

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

3 files changed, 35 insertions, 45 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 2ac504e..f00db65 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -64,9 +64,6 @@ static bool IsAPIPermission(const std::string& str) {
 
 }  // namespace
 
-// static
-int Extension::id_counter_ = 0;
-
 const char Extension::kManifestFilename[] = "manifest.json";
 const char Extension::kLocaleFolder[] = "_locales";
 const char Extension::kMessagesFilename[] = "messages.json";
@@ -176,8 +173,7 @@ Extension::Location Extension::ExternalExtensionInstallType(
   return Extension::EXTERNAL_PREF;
 }
 
-bool Extension::GenerateIdFromPublicKey(const std::string& input,
-                                        std::string* output) {
+bool Extension::GenerateId(const std::string& input, std::string* output) {
   CHECK(output);
   if (input.length() == 0)
     return false;
@@ -607,7 +603,7 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     std::string public_key_bytes;
     if (!source.GetString(keys::kPublicKey, &public_key_) ||
       !ParsePEMKeyBytes(public_key_, &public_key_bytes) ||
-      !GenerateIdFromPublicKey(public_key_bytes, &id_)) {
+      !GenerateId(public_key_bytes, &id_)) {
         *error = errors::kInvalidKey;
         return false;
     }
@@ -615,14 +611,13 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
     *error = errors::kInvalidKey;
     return false;
   } else {
-    // Generate a random ID
-    id_ = StringPrintf("%x", NextGeneratedId());
-
-    // pad the string out to kIdSize*2 chars with zeroes.
-    id_.insert(0, Extension::kIdSize*2 - id_.length(), '0');
-
-    // Convert to our mp-decimal.
-    ConvertHexadecimalToIDAlphabet(&id_);
+    // 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.
+    if (!GenerateId(WideToUTF8(path_.ToWStringHack()), &id_)) {
+      NOTREACHED() << "Could not create ID from path.";
+      return false;
+    }
   }
 
   // Make a copy of the manifest so we can store it in prefs.
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index d77e5e27..c01d948 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -106,17 +106,9 @@ class Extension {
   // The mimetype used for extensions.
   static const char kMimeType[];
 
-  Extension()
-      : location_(INVALID), is_theme_(false),
-        background_page_ready_(false) {}
   explicit Extension(const FilePath& path);
   virtual ~Extension();
 
-  // Resets the id counter. This is only useful for unit tests.
-  static void ResetGeneratedIdCounter() {
-    id_counter_ = 0;
-  }
-
   // Checks to see if the extension has a valid ID.
   static bool IdIsValid(const std::string& id);
 
@@ -160,10 +152,9 @@ class Extension {
   // Does a simple base64 encoding of |input| into |output|.
   static bool ProducePEM(const std::string& input, std::string* output);
 
-  // Note: The result is coverted to lower-case because the browser enforces
-  // hosts to be lower-case in omni-bar.
-  static bool GenerateIdFromPublicKey(const std::string& input,
-                                      std::string* output);
+  // Generates an extension ID from arbitrary input. The same input string will
+  // always generate the same output ID.
+  static bool GenerateId(const std::string& input, std::string* output);
 
   // Expects base64 encoded |input| and formats into |output| including
   // the appropriate header & footer.
@@ -281,14 +272,6 @@ class Extension {
   void SetBackgroundPageReady();
 
  private:
-  // Counter used to assign ids to extensions that are loaded using
-  // --load-extension.
-  static int id_counter_;
-
-  // Returns the next counter id. Intentionally post-incrementing so that first
-  // value is 0.
-  static int NextGeneratedId() { return id_counter_++; }
-
   // Helper method that loads a UserScript object from a
   // dictionary in the content_script list of the manifest.
   bool LoadUserScriptHelper(const DictionaryValue* content_script,
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index fc024ab..466f27a 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -227,8 +227,6 @@ TEST(ExtensionTest, InitFromValueValid) {
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-  Extension::ResetGeneratedIdCounter();
-
   Extension extension(path);
   std::string error;
   DictionaryValue input_value;
@@ -239,11 +237,10 @@ TEST(ExtensionTest, InitFromValueValid) {
 
   EXPECT_TRUE(extension.InitFromValue(input_value, false, &error));
   EXPECT_EQ("", error);
-  EXPECT_EQ("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", extension.id());
+  EXPECT_TRUE(Extension::IdIsValid(extension.id()));
   EXPECT_EQ("1.0.0.0", extension.VersionString());
   EXPECT_EQ("my extension", extension.name());
-  EXPECT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/",
-            extension.url().spec());
+  EXPECT_EQ(extension.id(), extension.url().host());
   EXPECT_EQ(path.value(), extension.path().value());
 }
 
@@ -277,7 +274,12 @@ TEST(ExtensionTest, GetResourceURLAndPath) {
 }
 
 TEST(ExtensionTest, LoadPageActionHelper) {
-  Extension extension;
+#if defined(OS_WIN)
+    FilePath path(StringPrintf(L"c:\\extension"));
+#else
+    FilePath path(StringPrintf("/extension"));
+#endif
+  Extension extension(path);
   std::string error_msg;
   scoped_ptr<ExtensionAction> action;
   DictionaryValue input;
@@ -424,7 +426,7 @@ TEST(ExtensionTest, IdIsValid) {
   EXPECT_FALSE(Extension::IdIsValid("abcdefghijklmnopabcdefghijklmno0"));
 }
 
-TEST(ExtensionTest, GenerateIDFromPublicKey) {
+TEST(ExtensionTest, GenerateID) {
   const uint8 public_key_info[] = {
     0x30, 0x81, 0x9f, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7,
     0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x81, 0x8d, 0x00, 0x30, 0x81,
@@ -444,7 +446,7 @@ TEST(ExtensionTest, GenerateIDFromPublicKey) {
 
   std::string extension_id;
   EXPECT_TRUE(
-      Extension::GenerateIdFromPublicKey(
+      Extension::GenerateId(
           std::string(reinterpret_cast<const char*>(&public_key_info[0]),
                       arraysize(public_key_info)),
           &extension_id));
@@ -463,7 +465,12 @@ TEST(ExtensionTest, UpdateUrls) {
     EXPECT_TRUE(url.is_valid());
 
     DictionaryValue input_value;
-    Extension extension;
+#if defined(OS_WIN)
+    FilePath path(StringPrintf(L"c:\\extension%i", i));
+#else
+    FilePath path(StringPrintf("/extension%i", i));
+#endif
+    Extension extension(path);
     std::string error;
 
     input_value.SetString(keys::kVersion, "1.0");
@@ -480,7 +487,12 @@ TEST(ExtensionTest, UpdateUrls) {
   valid.push_back("http://test.com/update#whatever");
   for (size_t i = 0; i < invalid.size(); i++) {
     DictionaryValue input_value;
-    Extension extension;
+#if defined(OS_WIN)
+    FilePath path(StringPrintf(L"c:\\extension%i", i));
+#else
+    FilePath path(StringPrintf("/extension%i", i));
+#endif
+    Extension extension(path);
     std::string error;
     input_value.SetString(keys::kVersion, "1.0");
     input_value.SetString(keys::kName, "Test");
@@ -529,7 +541,7 @@ static Extension* LoadManifest(const std::string& dir,
     return NULL;
 
   std::string error;
-  scoped_ptr<Extension> extension(new Extension);
+  scoped_ptr<Extension> extension(new Extension(path.DirName()));
   extension->InitFromValue(*static_cast<DictionaryValue*>(result.get()),
                            false, &error);