Split PendingExtensionInfo into its own file. No behavior changed.

I will be doing some refactoring that will change the way pending extension records for a single extension ID are merged.  This change will make PendingExtensionInfo a class with nontrivial member functions.  As a first step, this change moves PendingExtensionInfo into its own file.

Fixing crbug.com/61000 by merging pending requests is an example of the changes that will add members to class PendingExtensionInfo.

I changed the type ShouldInstallExtensionPredicate to PendingExtensionInfo::ShouldAllowInstallPredicate.  I wanted to get the type defined inside the class, and changing the name avoids some ugly 80-colunm gymnastics because it is slightly shorter.

BUG=None
TEST=ExtensionServiceTest.*

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

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

8 files changed, 141 insertions, 92 deletions

diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index e15fa03..6116e99 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -109,31 +109,6 @@ ManifestReloadReason ShouldReloadExtensionManifest(const ExtensionInfo& info) {
 
 }  // namespace
 
-PendingExtensionInfo::PendingExtensionInfo(
-    const GURL& update_url,
-    ShouldInstallExtensionPredicate should_install_extension,
-    bool is_from_sync,
-    bool install_silently,
-    bool enable_on_install,
-    bool enable_incognito_on_install,
-    Extension::Location location)
-    : update_url(update_url),
-      should_install_extension(should_install_extension),
-      is_from_sync(is_from_sync),
-      install_silently(install_silently),
-      enable_on_install(enable_on_install),
-      enable_incognito_on_install(enable_incognito_on_install),
-      install_source(location) {}
-
-PendingExtensionInfo::PendingExtensionInfo()
-    : update_url(),
-      should_install_extension(NULL),
-      is_from_sync(true),
-      install_silently(false),
-      enable_on_install(false),
-      enable_incognito_on_install(false),
-      install_source(Extension::INVALID) {}
-
 
 ExtensionService::ExtensionRuntimeData::ExtensionRuntimeData()
     : background_page_ready(false),
@@ -532,7 +507,7 @@ void ExtensionService::UpdateExtension(const std::string& id,
   // We want a silent install only for non-pending extensions and
   // pending extensions that have install_silently set.
   ExtensionInstallUI* client =
-      (!is_pending_extension || it->second.install_silently) ?
+      (!is_pending_extension || it->second.install_silently()) ?
       NULL : new ExtensionInstallUI(profile_);
 
   scoped_refptr<CrxInstaller> installer(
@@ -540,7 +515,7 @@ void ExtensionService::UpdateExtension(const std::string& id,
                        client));
   installer->set_expected_id(id);
   if (is_pending_extension)
-    installer->set_install_source(it->second.install_source);
+    installer->set_install_source(it->second.install_source());
   else if (extension)
     installer->set_install_source(extension->location());
   installer->set_delete_source(true);
@@ -550,7 +525,7 @@ void ExtensionService::UpdateExtension(const std::string& id,
 
 void ExtensionService::AddPendingExtensionFromSync(
     const std::string& id, const GURL& update_url,
-    ShouldInstallExtensionPredicate should_install_extension,
+    PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install,
     bool install_silently, bool enable_on_install,
     bool enable_incognito_on_install) {
   if (GetExtensionByIdInternal(id, true, true)) {
@@ -559,7 +534,7 @@ void ExtensionService::AddPendingExtensionFromSync(
     return;
   }
 
-  AddPendingExtensionInternal(id, update_url, should_install_extension, true,
+  AddPendingExtensionInternal(id, update_url, should_allow_install, true,
                               install_silently, enable_on_install,
                               enable_incognito_on_install,
                               Extension::INTERNAL);
@@ -626,7 +601,7 @@ void ExtensionService::AddPendingExtensionFromDefaultAppList(
 
 void ExtensionService::AddPendingExtensionInternal(
     const std::string& id, const GURL& update_url,
-    ShouldInstallExtensionPredicate should_install_extension,
+    PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install,
     bool is_from_sync, bool install_silently,
     bool enable_on_install, bool enable_incognito_on_install,
     Extension::Location install_source) {
@@ -644,14 +619,14 @@ void ExtensionService::AddPendingExtensionInternal(
   if (it != pending_extensions_.end()) {
     VLOG(1) << "Extension id " << id
             << " was entered for update more than once."
-            << "  old is_from_sync = " << it->second.is_from_sync
+            << "  old is_from_sync = " << it->second.is_from_sync()
             << "  new is_from_sync = " << is_from_sync;
-    if (!it->second.is_from_sync && is_from_sync)
+    if (!it->second.is_from_sync() && is_from_sync)
       return;
   }
 
   pending_extensions_[id] =
-      PendingExtensionInfo(update_url, should_install_extension,
+      PendingExtensionInfo(update_url, should_allow_install,
                            is_from_sync, install_silently, enable_on_install,
                            enable_incognito_on_install, install_source);
 }
@@ -1533,12 +1508,10 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) {
     pending_extensions_.erase(it);
     it = pending_extensions_.end();
 
-    if (!pending_extension_info.should_install_extension(*extension)) {
+    if (!pending_extension_info.ShouldAllowInstall(*extension)) {
       LOG(WARNING)
-          << "should_install_extension ("
-          << pending_extension_info.should_install_extension
-          << ") returned false for " << extension->id()
-          << " of type " << extension->GetType()
+          << "should_install_extension() returned false for "
+          << extension->id() << " of type " << extension->GetType()
           << " and update URL " << extension->update_url().spec()
           << "; not installing";
       // Delete the extension directory since we're not going to
@@ -1550,16 +1523,16 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) {
     }
 
     if (extension->is_theme()) {
-      DCHECK(pending_extension_info.enable_on_install);
+      DCHECK(pending_extension_info.enable_on_install());
       initial_state = Extension::ENABLED;
-      DCHECK(!pending_extension_info.enable_incognito_on_install);
+      DCHECK(!pending_extension_info.enable_incognito_on_install());
       initial_enable_incognito = false;
     } else {
       initial_state =
-          pending_extension_info.enable_on_install ?
+          pending_extension_info.enable_on_install() ?
           Extension::ENABLED : Extension::DISABLED;
       initial_enable_incognito =
-          pending_extension_info.enable_incognito_on_install;
+          pending_extension_info.enable_incognito_on_install();
     }
   } else {
     // Make sure we preserve enabled/disabled states.
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 547ae0b..4c9702a 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -27,6 +27,7 @@
 #include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/extensions/extensions_quota_service.h"
 #include "chrome/browser/extensions/external_extension_provider_interface.h"
+#include "chrome/browser/extensions/pending_extension_info.h"
 #include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/common/notification_observer.h"
@@ -43,40 +44,6 @@ class GURL;
 class Profile;
 class Version;
 
-typedef bool (*ShouldInstallExtensionPredicate)(const Extension&);
-
-// A pending extension is an extension that hasn't been installed yet
-// and is intended to be installed in the next auto-update cycle.  The
-// update URL of a pending extension may be blank, in which case a
-// default one is assumed.
-struct PendingExtensionInfo {
-  PendingExtensionInfo(
-      const GURL& update_url,
-      ShouldInstallExtensionPredicate should_install_extension,
-      bool is_from_sync,
-      bool install_silently,
-      bool enable_on_install,
-      bool enable_incognito_on_install,
-      Extension::Location install_source);
-
-  PendingExtensionInfo();
-
-  GURL update_url;
-  // When the extension is about to be installed, this function is
-  // called.  If this function returns true, the install proceeds.  If
-  // this function returns false, the install is aborted.
-  ShouldInstallExtensionPredicate should_install_extension;
-  bool is_from_sync;  // This update check was initiated from sync.
-  bool install_silently;
-  bool enable_on_install;
-  bool enable_incognito_on_install;
-  Extension::Location install_source;
-};
-
-// A PendingExtensionMap is a map from IDs of pending extensions to
-// their info.
-typedef std::map<std::string, PendingExtensionInfo> PendingExtensionMap;
-
 // This is an interface class to encapsulate the dependencies that
 // ExtensionUpdater has on ExtensionService. This allows easy mocking.
 class ExtensionUpdateService {
@@ -239,7 +206,7 @@ class ExtensionService
   // pre-enabled permissions.
   void AddPendingExtensionFromSync(
       const std::string& id, const GURL& update_url,
-      ShouldInstallExtensionPredicate should_install_extension,
+      PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install,
       bool install_silently, bool enable_on_install,
       bool enable_incognito_on_install);
 
@@ -486,7 +453,7 @@ class ExtensionService
   // extension with the same id is not already installed.
   void AddPendingExtensionInternal(
       const std::string& id, const GURL& update_url,
-      ShouldInstallExtensionPredicate should_install_extension,
+      PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install,
       bool is_from_sync, bool install_silently,
       bool enable_on_install, bool enable_incognito_on_install,
       Extension::Location install_source);
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index ddef3cc..5134bfe 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -2066,9 +2066,9 @@ TEST_F(ExtensionServiceTest, AddPendingExtensionFromSync) {
   PendingExtensionMap::const_iterator it =
       service_->pending_extensions().find(kFakeId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
-  EXPECT_EQ(kFakeUpdateURL, it->second.update_url);
-  EXPECT_EQ(&IsExtension, it->second.should_install_extension);
-  EXPECT_EQ(kFakeInstallSilently, it->second.install_silently);
+  EXPECT_EQ(kFakeUpdateURL, it->second.update_url());
+  EXPECT_EQ(&IsExtension, it->second.should_allow_install_);
+  EXPECT_EQ(kFakeInstallSilently, it->second.install_silently());
 }
 
 namespace {
@@ -2182,7 +2182,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrxWinsOverSync) {
   PendingExtensionMap::const_iterator it;
   it = service_->pending_extensions().find(kGoodId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
-  EXPECT_TRUE(it->second.is_from_sync);
+  EXPECT_TRUE(it->second.is_from_sync());
 
   // Add a crx to be updated, with the same ID, from a non-sync source.
   service_->AddPendingExtensionFromExternalUpdateUrl(
@@ -2191,7 +2191,8 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrxWinsOverSync) {
   // Check that there is a pending crx, with is_from_sync set to false.
   it = service_->pending_extensions().find(kGoodId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
-  EXPECT_FALSE(it->second.is_from_sync);
+  EXPECT_FALSE(it->second.is_from_sync());
+  EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, it->second.install_source());
 
   // Add a crx to be installed from the update mechanism.
   service_->AddPendingExtensionFromSync(
@@ -2202,7 +2203,8 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrxWinsOverSync) {
   // Check that the external, non-sync update was not overridden.
   it = service_->pending_extensions().find(kGoodId);
   ASSERT_TRUE(it != service_->pending_extensions().end());
-  EXPECT_FALSE(it->second.is_from_sync);
+  EXPECT_FALSE(it->second.is_from_sync());
+  EXPECT_EQ(Extension::EXTERNAL_PREF_DOWNLOAD, it->second.install_source());
 }
 
 // Updating a theme should fail if the updater is explicitly told that
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index 8e8aa72..c15f77f 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -204,8 +204,8 @@ void ManifestFetchesBuilder::AddPendingExtension(
       Version::GetVersionFromString("0.0.0.0"));
 
   AddExtensionData(
-      info.install_source, id, *version,
-      Extension::TYPE_UNKNOWN, info.update_url, "");
+      info.install_source(), id, *version,
+      Extension::TYPE_UNKNOWN, info.update_url(), "");
 }
 
 void ManifestFetchesBuilder::ReportStats() const {
@@ -768,7 +768,7 @@ void ExtensionUpdater::CheckNow() {
       service_->pending_extensions();
   for (PendingExtensionMap::const_iterator iter = pending_extensions.begin();
        iter != pending_extensions.end(); ++iter) {
-    Extension::Location location = iter->second.install_source;
+    Extension::Location location = iter->second.install_source();
     if (location != Extension::EXTERNAL_PREF &&
         location != Extension::EXTERNAL_REGISTRY)
       fetches_builder.AddPendingExtension(iter->first, iter->second);
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 067bd86..74de7f7 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -147,16 +147,15 @@ bool ShouldAlwaysInstall(const Extension& extension) {
 void CreateTestPendingExtensions(int count, const GURL& update_url,
                                  PendingExtensionMap* pending_extensions) {
   for (int i = 1; i <= count; i++) {
-    ShouldInstallExtensionPredicate should_install_extension =
-        (i % 2 == 0) ? &ShouldInstallThemesOnly :
-        &ShouldInstallExtensionsOnly;
+    PendingExtensionInfo::ShouldAllowInstallPredicate should_allow_install =
+        (i % 2 == 0) ? &ShouldInstallThemesOnly : &ShouldInstallExtensionsOnly;
     const bool kIsFromSync = true;
     const bool kInstallSilently = true;
     const Extension::State kInitialState = Extension::ENABLED;
     const bool kInitialIncognitoEnabled = false;
     std::string id = GenerateId(base::StringPrintf("extension%i", i));
     (*pending_extensions)[id] =
-        PendingExtensionInfo(update_url, should_install_extension,
+        PendingExtensionInfo(update_url, should_allow_install,
                              kIsFromSync, kInstallSilently, kInitialState,
                              kInitialIncognitoEnabled, Extension::INTERNAL);
   }
diff --git a/chrome/browser/extensions/pending_extension_info.cc b/chrome/browser/extensions/pending_extension_info.cc
new file mode 100644
index 0000000..22ee249
--- /dev/null
+++ b/chrome/browser/extensions/pending_extension_info.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2011 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.
+
+#include "chrome/browser/extensions/pending_extension_info.h"
+
+PendingExtensionInfo::PendingExtensionInfo(
+    const GURL& update_url,
+    ShouldAllowInstallPredicate should_allow_install,
+    bool is_from_sync,
+    bool install_silently,
+    bool enable_on_install,
+    bool enable_incognito_on_install,
+    Extension::Location install_source)
+    : update_url_(update_url),
+      should_allow_install_(should_allow_install),
+      is_from_sync_(is_from_sync),
+      install_silently_(install_silently),
+      enable_on_install_(enable_on_install),
+      enable_incognito_on_install_(enable_incognito_on_install),
+      install_source_(install_source) {}
+
+PendingExtensionInfo::PendingExtensionInfo()
+    : update_url_(),
+      should_allow_install_(NULL),
+      is_from_sync_(true),
+      install_silently_(false),
+      enable_on_install_(false),
+      enable_incognito_on_install_(false),
+      install_source_(Extension::INVALID) {}
diff --git a/chrome/browser/extensions/pending_extension_info.h b/chrome/browser/extensions/pending_extension_info.h
new file mode 100644
index 0000000..c3812f3
--- /dev/null
+++ b/chrome/browser/extensions/pending_extension_info.h
@@ -0,0 +1,76 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_
+#define CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_
+#pragma once
+
+#include <map>
+#include <string>
+
+#include "chrome/common/extensions/extension.h"
+
+class GURL;
+
+// A pending extension is an extension that hasn't been installed yet
+// and is intended to be installed in the next auto-update cycle.  The
+// update URL of a pending extension may be blank, in which case a
+// default one is assumed.
+class PendingExtensionInfo {
+ public:
+  typedef bool (*ShouldAllowInstallPredicate)(const Extension&);
+
+  PendingExtensionInfo(
+      const GURL& update_url,
+      ShouldAllowInstallPredicate should_allow_install,
+      bool is_from_sync,
+      bool install_silently,
+      bool enable_on_install,
+      bool enable_incognito_on_install,
+      Extension::Location install_source);
+
+  // Required for STL container membership.  Should not be used.
+  PendingExtensionInfo();
+
+  const GURL& update_url() const { return update_url_; }
+
+  // ShouldAllowInstall() returns the result of running constructor argument
+  // |should_allow_install| on an extension. After an extension is unpacked,
+  // this function is run. If it returns true, the extension is installed.
+  // If not, the extension is discarded. This allows creators of
+  // PendingExtensionInfo objects to ensure that extensions meet some criteria
+  // that can only be tested once the extension is unpacked.
+  const bool ShouldAllowInstall(const Extension& extension) const {
+    return should_allow_install_(extension);
+  }
+  bool is_from_sync() const { return is_from_sync_; }
+  bool install_silently() const { return install_silently_; }
+  bool enable_on_install() const { return enable_on_install_; }
+  bool enable_incognito_on_install() const {
+    return enable_incognito_on_install_;
+  }
+  Extension::Location install_source() const { return install_source_; }
+
+ private:
+  GURL update_url_;
+
+  // When the extension is about to be installed, this function is
+  // called.  If this function returns true, the install proceeds.  If
+  // this function returns false, the install is aborted.
+  ShouldAllowInstallPredicate should_allow_install_;
+
+  bool is_from_sync_;  // This update check was initiated from sync.
+  bool install_silently_;
+  bool enable_on_install_;
+  bool enable_incognito_on_install_;
+  Extension::Location install_source_;
+
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, AddPendingExtensionFromSync);
+};
+
+// A PendingExtensionMap is a map from IDs of pending extensions to
+// their info.
+typedef std::map<std::string, PendingExtensionInfo> PendingExtensionMap;
+
+#endif  // CHROME_BROWSER_EXTENSIONS_PENDING_EXTENSION_INFO_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 590aaaa..9fa63ea 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -1065,6 +1065,8 @@
         'browser/extensions/key_identifier_conversion_views.h',
         'browser/extensions/pack_extension_job.cc',
         'browser/extensions/pack_extension_job.h',
+        'browser/extensions/pending_extension_info.h',
+        'browser/extensions/pending_extension_info.cc',
         'browser/extensions/sandboxed_extension_unpacker.cc',
         'browser/extensions/sandboxed_extension_unpacker.h',
         'browser/extensions/theme_installed_infobar_delegate.cc',
@@ -1704,7 +1706,7 @@
         'browser/renderer_host/safe_browsing_resource_handler.cc',
         'browser/renderer_host/safe_browsing_resource_handler.h',
         'browser/renderer_host/save_file_resource_handler.cc',
-        'browser/renderer_host/save_file_resource_handler.h',     
+        'browser/renderer_host/save_file_resource_handler.h',
         'browser/renderer_host/web_cache_manager.cc',
         'browser/renderer_host/web_cache_manager.h',
         'browser/renderer_preferences_util.cc',