Revert 79407 - Move ExtensionService code that manages pending extensions into its own class.

This change should have no impact on chrome's behavior.

Once all code paths that install an extension inform PendingExtensionManger that they are starting an install, we can correctly merge requests to install the same extension from multiple sources.  This will remove two hacky checks to avoid install races we have seen on Chrome Os.  It will also ensure that more hacks are not needed.

BUG=61000
TEST=ExtensionUpdaterTest.*:ExtensionServiceTest.*'

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

TBR=skerner@chromium.org
Review URL: http://codereview.chromium.org/6730058

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

1 files changed, 54 insertions, 64 deletions

diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 4c08abb..bc98520 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -50,8 +50,7 @@ const ManifestFetchData::PingData kNeverPingedData(
 // Base class for further specialized test classes.
 class MockService : public ExtensionUpdateService {
  public:
-  MockService()
-      : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) {}
+  MockService() {}
   virtual ~MockService() {}
 
   virtual const ExtensionList* extensions() const {
@@ -59,20 +58,18 @@ class MockService : public ExtensionUpdateService {
     return NULL;
   }
 
-  virtual PendingExtensionManager* pending_extension_manager() {
-    ADD_FAILURE() << "Subclass should override this if it will "
-                  << "be accessed by a test.";
-    return &pending_extension_manager_;
+  virtual const PendingExtensionMap& pending_extensions() const {
+    ADD_FAILURE();
+    return pending_extensions_;
   }
 
   virtual void UpdateExtension(const std::string& id,
-                               const FilePath& path,
+                               const FilePath& extension_path,
                                const GURL& download_url) {
     FAIL();
   }
 
-  virtual const Extension* GetExtensionById(const std::string& id,
-                                            bool include_disabled) const {
+  virtual const Extension* GetExtensionById(const std::string& id, bool) {
     ADD_FAILURE();
     return NULL;
   }
@@ -92,9 +89,6 @@ class MockService : public ExtensionUpdateService {
   }
 
   virtual ExtensionPrefs* extension_prefs() { return prefs_.prefs(); }
-  virtual const ExtensionPrefs& const_extension_prefs() const {
-    return prefs_.const_prefs();
-  }
 
   virtual Profile* profile() { return &profile_; }
 
@@ -124,7 +118,7 @@ class MockService : public ExtensionUpdateService {
   }
 
  protected:
-  PendingExtensionManager pending_extension_manager_;
+  PendingExtensionMap pending_extensions_;
   TestExtensionPrefs prefs_;
   TestingProfile profile_;
 
@@ -151,11 +145,10 @@ bool ShouldAlwaysInstall(const Extension& extension) {
   return true;
 }
 
-// Loads some pending extension records into a pending extension manager.
-void SetupPendingExtensionManagerForTest(
-    int count,
-    const GURL& update_url,
-    PendingExtensionManager* pending_extension_manager) {
+// Creates test pending extensions and inserts them into list. The
+// name and version are all based on their index.
+void CreateTestPendingExtensions(int count, const GURL& update_url,
+                                 PendingExtensionMap* pending_extensions) {
   for (int i = 1; i <= count; i++) {
     PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install =
         (i % 2 == 0) ? &ShouldInstallThemesOnly : &ShouldInstallExtensionsOnly;
@@ -164,16 +157,10 @@ void SetupPendingExtensionManagerForTest(
     const Extension::State kInitialState = Extension::ENABLED;
     const bool kInitialIncognitoEnabled = false;
     std::string id = GenerateId(base::StringPrintf("extension%i", i));
-
-    pending_extension_manager->AddForTesting(
-        id,
-        PendingExtensionInfo(update_url,
-                             should_allow_install,
-                             kIsFromSync,
-                             kInstallSilently,
-                             kInitialState,
-                             kInitialIncognitoEnabled,
-                             Extension::INTERNAL));
+    (*pending_extensions)[id] =
+        PendingExtensionInfo(update_url, should_allow_install,
+                             kIsFromSync, kInstallSilently, kInitialState,
+                             kInitialIncognitoEnabled, Extension::INTERNAL);
   }
 }
 
@@ -183,27 +170,31 @@ class ServiceForManifestTests : public MockService {
 
   virtual ~ServiceForManifestTests() {}
 
-  virtual const Extension* GetExtensionById(const std::string& id,
-                                            bool include_disabled) const {
-    for (ExtensionList::const_iterator iter = extensions_.begin();
+  virtual const Extension* GetExtensionById(const std::string& id, bool) {
+    for (ExtensionList::iterator iter = extensions_.begin();
         iter != extensions_.end(); ++iter) {
-      if ((*iter)->id() == id) {
-        return *iter;
-      }
+     if ((*iter)->id() == id) {
+       return *iter;
+     }
     }
     return NULL;
   }
 
   virtual const ExtensionList* extensions() const { return &extensions_; }
 
-  virtual PendingExtensionManager* pending_extension_manager() {
-    return &pending_extension_manager_;
+  virtual const PendingExtensionMap& pending_extensions() const {
+    return pending_extensions_;
   }
 
   void set_extensions(ExtensionList extensions) {
     extensions_ = extensions;
   }
 
+  void set_pending_extensions(
+      const PendingExtensionMap& pending_extensions) {
+    pending_extensions_ = pending_extensions;
+  }
+
   virtual bool HasInstalledExtensions() {
     return has_installed_extensions_;
   }
@@ -227,19 +218,24 @@ class ServiceForDownloadTests : public MockService {
     download_url_ = download_url;
   }
 
-  virtual PendingExtensionManager* pending_extension_manager() {
-    return &pending_extension_manager_;
+  virtual const PendingExtensionMap& pending_extensions() const {
+    return pending_extensions_;
   }
 
-  virtual const Extension* GetExtensionById(const std::string& id, bool) const {
+  virtual const Extension* GetExtensionById(const std::string& id, bool) {
     last_inquired_extension_id_ = id;
     return NULL;
   }
 
-  const std::string& extension_id() const { return extension_id_; }
-  const FilePath& install_path() const { return install_path_; }
-  const GURL& download_url() const { return download_url_; }
-  const std::string& last_inquired_extension_id() const {
+  void set_pending_extensions(
+      const PendingExtensionMap& pending_extensions) {
+    pending_extensions_ = pending_extensions;
+  }
+
+  const std::string& extension_id() { return extension_id_; }
+  const FilePath& install_path() { return install_path_; }
+  const GURL& download_url() { return download_url_; }
+  const std::string& last_inquired_extension_id() {
     return last_inquired_extension_id_;
   }
 
@@ -248,11 +244,8 @@ class ServiceForDownloadTests : public MockService {
   FilePath install_path_;
   GURL download_url_;
 
-  // The last extension ID that GetExtensionById was called with.
-  // Mutable because the method that sets it (GetExtensionById) is const
-  // in the actual extension service, but must record the last extension
-  // ID in this test class.
-  mutable std::string last_inquired_extension_id_;
+  // The last extension_id that GetExtensionById was called with.
+  std::string last_inquired_extension_id_;
 };
 
 class ServiceForBlacklistTests : public MockService {
@@ -331,11 +324,10 @@ class ExtensionUpdaterTest : public testing::Test {
     ServiceForManifestTests service;
     std::string update_url("http://foo.com/bar");
     ExtensionList extensions;
-    PendingExtensionManager* pending_extension_manager =
-        service.pending_extension_manager();
+    PendingExtensionMap pending_extensions;
     if (pending) {
-      SetupPendingExtensionManagerForTest(1, GURL(update_url),
-                                          pending_extension_manager);
+      CreateTestPendingExtensions(1, GURL(update_url), &pending_extensions);
+      service.set_pending_extensions(pending_extensions);
     } else {
       service.CreateTestExtensions(1, 1, &extensions, &update_url,
                                    Extension::INTERNAL);
@@ -374,7 +366,7 @@ class ExtensionUpdaterTest : public testing::Test {
     std::map<std::string, std::string> params;
     ExtractParameters(decoded, &params);
     if (pending) {
-      EXPECT_EQ(pending_extension_manager->begin()->first, params["id"]);
+      EXPECT_EQ(pending_extensions.begin()->first, params["id"]);
       EXPECT_EQ("0.0.0.0", params["v"]);
     } else {
       EXPECT_EQ(extensions[0]->id(), params["id"]);
@@ -546,9 +538,9 @@ class ExtensionUpdaterTest : public testing::Test {
   static void TestDetermineUpdatesPending() {
     // Create a set of test extensions
     ServiceForManifestTests service;
-    PendingExtensionManager* pending_extension_manager =
-        service.pending_extension_manager();
-    SetupPendingExtensionManagerForTest(3, GURL(), pending_extension_manager);
+    PendingExtensionMap pending_extensions;
+    CreateTestPendingExtensions(3, GURL(), &pending_extensions);
+    service.set_pending_extensions(pending_extensions);
 
     MessageLoop message_loop;
     scoped_refptr<ExtensionUpdater> updater(
@@ -558,9 +550,8 @@ class ExtensionUpdaterTest : public testing::Test {
 
     ManifestFetchData fetch_data(GURL("http://localhost/foo"));
     UpdateManifest::Results updates;
-    PendingExtensionManager::const_iterator it;
-    for (it = pending_extension_manager->begin();
-         it != pending_extension_manager->end(); ++it) {
+    for (PendingExtensionMap::const_iterator it = pending_extensions.begin();
+         it != pending_extensions.end(); ++it) {
       fetch_data.AddExtension(it->first, "1.0.0.0",
                               kNeverPingedData,
                               kEmptyUpdateUrlData);
@@ -678,13 +669,12 @@ class ExtensionUpdaterTest : public testing::Test {
       const bool kInstallSilently = true;
       const Extension::State kInitialState = Extension::ENABLED;
       const bool kInitialIncognitoEnabled = false;
-      PendingExtensionManager* pending_extension_manager =
-          service->pending_extension_manager();
-      pending_extension_manager->AddForTesting(
-          id,
+      PendingExtensionMap pending_extensions;
+      pending_extensions[id] =
           PendingExtensionInfo(test_url, &ShouldAlwaysInstall, kIsFromSync,
                                kInstallSilently, kInitialState,
-                               kInitialIncognitoEnabled, Extension::INTERNAL));
+                               kInitialIncognitoEnabled, Extension::INTERNAL);
+      service->set_pending_extensions(pending_extensions);
     }
 
     // Call back the ExtensionUpdater with a 200 response and some test data