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

7 files changed, 32 insertions, 43 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index c7e37fa..b7acc21 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -199,6 +199,7 @@ void ExtensionBrowserTest::Observe(NotificationType type,
                                    const NotificationDetails& details) {
   switch (type.value) {
     case NotificationType::EXTENSION_LOADED:
+      last_loaded_extension_id_ = Details<Extension>(details).ptr()->id();
       std::cout << "Got EXTENSION_LOADED notification.\n";
       MessageLoopForUI::current()->Quit();
       break;
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index 943bdd4..5e2b882 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -56,6 +56,7 @@ class ExtensionBrowserTest
   bool loaded_;
   bool installed_;
   FilePath test_data_dir_;
+  std::string last_loaded_extension_id_;
 
  private:
   bool InstallOrUpdateExtension(const std::string& id, const FilePath& path,
diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc
index bdda3dd..bab50d1 100644
--- a/chrome/browser/extensions/extension_browsertests_misc.cc
+++ b/chrome/browser/extensions/extension_browsertests_misc.cc
@@ -27,9 +27,6 @@
 #include "chrome/test/ui_test_utils.h"
 #include "net/base/net_util.h"
 
-// ID assigned to the first unpacked extension loaded by LoadExtension().
-#define kDefaultExtensionID "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-
 const std::wstring kSubscribePage =
     L"files/extensions/samples/subscribe_page_action/subscribe.html";
 const std::wstring kValidFeed0 = L"files/feeds/feed_script.xml";
@@ -250,7 +247,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, UnloadPageAction) {
   ui_test_utils::NavigateToURL(browser(), net::FilePathToFileURL(feed));
   ASSERT_TRUE(WaitForPageActionCountChangeTo(1));
 
-  UnloadExtension(kDefaultExtensionID);
+  UnloadExtension(last_loaded_extension_id_);
 
   // Make sure the page action goes away when it's unloaded.
   ASSERT_TRUE(WaitForPageActionCountChangeTo(0));
@@ -622,7 +619,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenExtension) {
 
   TabContents* newtab = WindowOpenHelper(
       browser(),
-      GURL("chrome-extension://" kDefaultExtensionID "/test.html"),
+      GURL(std::string("chrome-extension://") + last_loaded_extension_id_ +
+           "/test.html"),
       "newtab.html");
 
   bool result = false;
@@ -639,7 +637,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenInvalidExtension) {
 
   WindowOpenHelper(
       browser(),
-      GURL("chrome-extension://" kDefaultExtensionID "/test.html"),
+      GURL(std::string("chrome-extension://") + last_loaded_extension_id_ +
+           "/test.html"),
       "chrome-extension://thisissurelynotavalidextensionid/newtab.html");
 
   // If we got to this point, we didn't crash, so we're good.
@@ -655,7 +654,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WindowOpenNoPrivileges) {
   TabContents* newtab = WindowOpenHelper(
       browser(),
       GURL("about:blank"),
-      "chrome-extension://" kDefaultExtensionID "/newtab.html");
+      std::string("chrome-extension://") + last_loaded_extension_id_ +
+          "/newtab.html");
 
   // Extension API should fail.
   bool result = false;
diff --git a/chrome/browser/extensions/extension_uitest.cc b/chrome/browser/extensions/extension_uitest.cc
index 1f9238a..8518dad 100644
--- a/chrome/browser/extensions/extension_uitest.cc
+++ b/chrome/browser/extensions/extension_uitest.cc
@@ -400,7 +400,7 @@ void BrowserEventAutomationProxy::HandleMessageFromChrome() {
     // TEST_F(BrowserEventExtensionTest, RunTest) to understand where the
     // extension Id comes from.
     tab_->HandleMessageFromExternalHost(
-        "{\"rqid\":0, \"extid\": \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\","
+        "{\"rqid\":0, \"extid\": \"ofoknjclcmghjfmbncljcnpjmfmldhno\","
         " \"connid\": 1}",
         keys::kAutomationOrigin,
         keys::kAutomationPortRequestTarget);
@@ -475,15 +475,8 @@ TEST_F(BrowserEventExtensionTest, RunTest) {
   // This test loads an HTML file that tries to add listeners to a bunch of
   // chrome.* events and upon adding a listener it posts the name of the event
   // to the automation layer, which we'll count to make sure the events work.
-  //
-  // The extension for this test does not specify a "key" property in its
-  // manifest file.  Therefore, the extension system will automatically assign
-  // it an Id.  To make this test consistent and non-flaky, the genetated Id
-  // counter is reset before the test so that we can hardcode the first Id
-  // that will be generated.
-  Extension::ResetGeneratedIdCounter();
   TestWithURL(GURL(
-      "chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/test.html"));
+      "chrome-extension://ofoknjclcmghjfmbncljcnpjmfmldhno/test.html"));
   BrowserEventAutomationProxy* proxy =
       static_cast<BrowserEventAutomationProxy*>(automation());
 
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index e9892a2..69f2460 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -79,8 +79,6 @@ class ScopedTempPrefService {
   scoped_ptr<PrefService> prefs_;
 };
 
-const char* kIdPrefix = "000000000000000000000000000000000000000";
-
 // Creates test extensions and inserts them into list. The name and
 // version are all based on their index. If |update_url| is non-null, it
 // will be used as the update_url for each extension.
@@ -88,7 +86,12 @@ void CreateTestExtensions(int count, ExtensionList *list,
                           const std::string* update_url) {
   for (int i = 1; i <= count; i++) {
     DictionaryValue input;
-    Extension* e = new Extension();
+#if defined(OS_WIN)
+    FilePath path(StringPrintf(L"c:\\extension%i", i));
+#else
+    FilePath path(StringPrintf("/extension%i", i));
+#endif
+    Extension* e = new Extension(path);
     input.SetString(extension_manifest_keys::kVersion,
                     StringPrintf("%d.0.0.0", i));
     input.SetString(extension_manifest_keys::kName,
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 04ff3d7..fa257e0 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -1197,7 +1197,6 @@ TEST_F(ExtensionsServiceTest, LoadExtension) {
 // --load-extension.
 TEST_F(ExtensionsServiceTest, GenerateID) {
   InitializeEmptyExtensionsService();
-  Extension::ResetGeneratedIdCounter();
 
   FilePath extensions_path;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &extensions_path));
@@ -1208,25 +1207,19 @@ TEST_F(ExtensionsServiceTest, GenerateID) {
   loop_.RunAllPending();
   EXPECT_EQ(0u, GetErrors().size());
   ASSERT_EQ(1u, loaded_.size());
-  std::string id1 = loaded_[0]->id();
-  EXPECT_EQ(all_zero, id1);
-  EXPECT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/",
-            loaded_[0]->url().spec());
+  ASSERT_TRUE(Extension::IdIsValid(loaded_[0]->id()));
   EXPECT_EQ(loaded_[0]->location(), Extension::LOAD);
 
   // --load-extension doesn't add entries to prefs
   ValidatePrefKeyCount(0);
 
+  std::string previous_id = loaded_[0]->id();
+
+  // If we reload the same path, we should get the same extension ID.
   service_->LoadExtension(no_id_ext);
   loop_.RunAllPending();
-  std::string id2 = loaded_[1]->id();
-  EXPECT_EQ(zero_n_one, id2);
-  EXPECT_EQ("chrome-extension://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab/",
-            loaded_[1]->url().spec());
-  EXPECT_EQ(loaded_[1]->location(), Extension::LOAD);
-
-  // --load-extension doesn't add entries to prefs
-  ValidatePrefKeyCount(0);
+  ASSERT_EQ(1u, loaded_.size());
+  ASSERT_EQ(previous_id, loaded_[0]->id());
 }
 
 // Tests the external installation feature
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 9d043b7..8756450 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -127,18 +127,13 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
     return;
   }
 
-  // Delete any images that may be used by the browser.  We're going to write
-  // out our own versions of the parsed images, and we want to make sure the
-  // originals are gone for good.
-  extension_.reset(new Extension);
-  std::string manifest_error;
-
-  // Update the path to refer to the temporary location. We do this because
-  // clients may want to use resources inside the extension before it is
-  // installed and they need the correct path. For example, the install UI shows
-  // one of the icons from the extension.
-  extension_->set_path(extension_root_);
+  // Create an extension object that refers to the temporary location the
+  // extension was unpacked to. We use this until the extension is finally
+  // installed. For example, the install UI shows images from inside the
+  // extension.
+  extension_.reset(new Extension(extension_root_));
 
+  std::string manifest_error;
   if (!extension_->InitFromValue(*final_manifest, true,  // require id
                                  &manifest_error)) {
     ReportFailure(std::string("Manifest is invalid: ") +
@@ -146,6 +141,9 @@ void SandboxedExtensionUnpacker::OnUnpackExtensionSucceeded(
     return;
   }
 
+  // Delete any images that may be used by the browser.  We're going to write
+  // out our own versions of the parsed images, and we want to make sure the
+  // originals are gone for good.
   std::set<FilePath> image_paths = extension_->GetBrowserImages();
   if (image_paths.size() != images.size()) {
     ReportFailure("Decoded images don't match what's in the manifest.");