Revert 80539 - Refactor ExtensionService/ExtensionServiceInterface to be more testableIn particular:- Add various status accessors to ExtensionServiceInterface (e.g., IsExtensionEnabled())- Convert IsIncognitoEnabled() to take an ID instead of an Extension*.- Add CheckForUpdates() to ExtensionServiceInterface.- Remove various unneeded accessors from ExtensionServiceInterface.Rewrite some sync utility functions to take advantage of the newinterface (although it will be rewritten more in a future CL).BUG=77995TEST=Review URL: http://codereview.chromium.org/6720042

TBR=akalin@chromium.org
Review URL: http://codereview.chromium.org/6804010

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

15 files changed, 132 insertions, 208 deletions

diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 670789a..bbadbfa4 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -88,7 +88,7 @@ bool ExtensionBrowserTest::LoadExtensionImpl(const FilePath& path,
   // are set up with the defaults.
   service->extension_prefs()->OnExtensionInstalled(
       extension, Extension::ENABLED, false);
-  service->SetIsIncognitoEnabled(extension->id(), incognito_enabled);
+  service->SetIsIncognitoEnabled(extension, incognito_enabled);
   service->SetAllowFileAccess(extension, fileaccess_enabled);
 
   return WaitForExtensionHostsToLoad();
diff --git a/chrome/browser/extensions/extension_event_router.cc b/chrome/browser/extensions/extension_event_router.cc
index a86bf1c..257a7f2 100644
--- a/chrome/browser/extensions/extension_event_router.cc
+++ b/chrome/browser/extensions/extension_event_router.cc
@@ -75,9 +75,8 @@ bool ExtensionEventRouter::CanCrossIncognito(Profile* profile,
   // We allow the extension to see events and data from another profile iff it
   // uses "spanning" behavior and it has incognito access. "split" mode
   // extensions only see events for a matching profile.
-  return
-      (profile->GetExtensionService()->IsIncognitoEnabled(extension->id()) &&
-       !extension->incognito_split_mode());
+  return (profile->GetExtensionService()->IsIncognitoEnabled(extension) &&
+          !extension->incognito_split_mode());
 }
 
 ExtensionEventRouter::ExtensionEventRouter(Profile* profile)
diff --git a/chrome/browser/extensions/extension_module.cc b/chrome/browser/extensions/extension_module.cc
index a1c884b..bb13067 100644
--- a/chrome/browser/extensions/extension_module.cc
+++ b/chrome/browser/extensions/extension_module.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -27,7 +27,7 @@ bool IsAllowedIncognitoAccessFunction::RunImpl() {
   const Extension* extension = GetExtension();
 
   result_.reset(Value::CreateBooleanValue(
-      ext_service->IsIncognitoEnabled(extension->id())));
+        ext_service->IsIncognitoEnabled(extension)));
   return true;
 }
 
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index a9a7e81..8ff58e6f 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -439,7 +439,7 @@ bool IncognitoExtensionProcessManager::IsIncognitoEnabled(
     const Extension* extension) {
   ExtensionService* service =
       browsing_instance_->profile()->GetExtensionService();
-  return service && service->IsIncognitoEnabled(extension->id());
+  return service && service->IsIncognitoEnabled(extension);
 }
 
 void IncognitoExtensionProcessManager::Observe(
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 361e165..a45e2bb 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -428,9 +428,7 @@ ExtensionService::ExtensionService(Profile* profile,
           &update_frequency);
     }
     updater_ = new ExtensionUpdater(this,
-                                    extension_prefs,
                                     profile->GetPrefs(),
-                                    profile,
                                     update_frequency);
   }
 
@@ -675,21 +673,6 @@ void ExtensionService::ClearExtensionData(const GURL& extension_url) {
   deleter->StartDeleting();
 }
 
-bool ExtensionService::IsExtensionEnabled(
-    const std::string& extension_id) const {
-  // TODO(akalin): GetExtensionState() isn't very safe as it returns
-  // Extension::ENABLED by default; either change it to return
-  // something else by default or create a separate function that does
-  // so.
-  return
-      extension_prefs_->GetExtensionState(extension_id) == Extension::ENABLED;
-}
-
-bool ExtensionService::IsExternalExtensionUninstalled(
-    const std::string& extension_id) const {
-  return extension_prefs_->IsExternalExtensionUninstalled(extension_id);
-}
-
 void ExtensionService::EnableExtension(const std::string& extension_id) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -1106,6 +1089,10 @@ ExtensionPrefs* ExtensionService::extension_prefs() {
   return extension_prefs_;
 }
 
+const ExtensionPrefs& ExtensionService::const_extension_prefs() const {
+  return *extension_prefs_;
+}
+
 ExtensionUpdater* ExtensionService::updater() {
   return updater_.get();
 }
@@ -1126,44 +1113,30 @@ void ExtensionService::CheckAdminBlacklist() {
     UnloadExtension(to_be_removed[i], UnloadedExtensionInfo::DISABLE);
 }
 
-void ExtensionService::CheckForUpdates() {
-  if (updater()) {
-    updater()->CheckNow();
-  } else {
-    LOG(WARNING) << "CheckForUpdates() called with auto-update turned off";
-  }
-}
-
-bool ExtensionService::IsIncognitoEnabled(
-    const std::string& extension_id) const {
-  // If this is an existing component extension we always allow it to
-  // work in incognito mode.
-  const Extension* extension = GetExtensionById(extension_id, true);
-  if (extension && extension->location() == Extension::COMPONENT)
+bool ExtensionService::IsIncognitoEnabled(const Extension* extension) {
+  // If this is a component extension we always allow it to work in incognito
+  // mode.
+  if (extension->location() == Extension::COMPONENT)
     return true;
 
   // Check the prefs.
-  return extension_prefs_->IsIncognitoEnabled(extension_id);
+  return extension_prefs_->IsIncognitoEnabled(extension->id());
 }
 
-void ExtensionService::SetIsIncognitoEnabled(
-    const std::string& extension_id, bool enabled) {
-  const Extension* extension = GetExtensionById(extension_id, false);
-  if (extension && extension->location() == Extension::COMPONENT) {
-    // This shouldn't be called for component extensions.
-    NOTREACHED();
-    return;
-  }
-
+void ExtensionService::SetIsIncognitoEnabled(const Extension* extension,
+                                              bool enabled) {
   // Broadcast unloaded and loaded events to update browser state. Only bother
   // if the value changed and the extension is actually enabled, since there is
   // no UI otherwise.
-  bool old_enabled = extension_prefs_->IsIncognitoEnabled(extension_id);
+  bool old_enabled = extension_prefs_->IsIncognitoEnabled(extension->id());
   if (enabled == old_enabled)
     return;
 
-  extension_prefs_->SetIsIncognitoEnabled(extension_id, enabled);
-  if (extension) {
+  extension_prefs_->SetIsIncognitoEnabled(extension->id(), enabled);
+
+  bool extension_is_enabled = std::find(extensions_.begin(), extensions_.end(),
+                                        extension) != extensions_.end();
+  if (extension_is_enabled) {
     NotifyExtensionUnloaded(extension, UnloadedExtensionInfo::DISABLE);
     NotifyExtensionLoaded(extension);
   }
@@ -1173,8 +1146,7 @@ bool ExtensionService::CanCrossIncognito(const Extension* extension) {
   // We allow the extension to see events and data from another profile iff it
   // uses "spanning" behavior and it has incognito access. "split" mode
   // extensions only see events for a matching profile.
-  return IsIncognitoEnabled(extension->id()) &&
-      !extension->incognito_split_mode();
+  return IsIncognitoEnabled(extension) && !extension->incognito_split_mode();
 }
 
 bool ExtensionService::AllowFileAccess(const Extension* extension) {
@@ -1544,18 +1516,18 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) {
 
   // Ensure extension is deleted unless we transfer ownership.
   scoped_refptr<const Extension> scoped_extension(extension);
-  const std::string& id = extension->id();
-  bool initial_enable = false;
+  Extension::State initial_state = Extension::DISABLED;
   bool initial_enable_incognito = false;
 
   PendingExtensionInfo pending_extension_info;
-  if (pending_extension_manager()->GetById(id, &pending_extension_info)) {
-    pending_extension_manager()->Remove(id);
+  if (pending_extension_manager()->GetById(extension->id(),
+                                           &pending_extension_info)) {
+    pending_extension_manager()->Remove(extension->id());
 
     if (!pending_extension_info.ShouldAllowInstall(*extension)) {
       LOG(WARNING)
           << "ShouldAllowInstall() returned false for "
-          << id << " of type " << extension->GetType()
+          << extension->id() << " of type " << extension->GetType()
           << " and update URL " << extension->update_url().spec()
           << "; not installing";
 
@@ -1575,35 +1547,38 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) {
 
     if (extension->is_theme()) {
       DCHECK(pending_extension_info.enable_on_install());
-      initial_enable = true;
+      initial_state = Extension::ENABLED;
       DCHECK(!pending_extension_info.enable_incognito_on_install());
       initial_enable_incognito = false;
     } else {
-      initial_enable = pending_extension_info.enable_on_install();
+      initial_state =
+          pending_extension_info.enable_on_install() ?
+          Extension::ENABLED : Extension::DISABLED;
       initial_enable_incognito =
           pending_extension_info.enable_incognito_on_install();
     }
   } else {
-    // We explicitly want to re-enable an uninstalled external
-    // extension; if we're here, that means the user is manually
-    // installing the extension.
-    initial_enable =
-        IsExtensionEnabled(id) || IsExternalExtensionUninstalled(id);
-    initial_enable_incognito = IsIncognitoEnabled(id);
+    // Make sure we preserve enabled/disabled states.
+    Extension::State existing_state =
+        extension_prefs_->GetExtensionState(extension->id());
+    initial_state =
+        (existing_state == Extension::DISABLED) ?
+        Extension::DISABLED : Extension::ENABLED;
+    initial_enable_incognito =
+        extension_prefs_->IsIncognitoEnabled(extension->id());
   }
 
   UMA_HISTOGRAM_ENUMERATION("Extensions.InstallType",
                             extension->GetType(), 100);
   ShownSectionsHandler::OnExtensionInstalled(profile_->GetPrefs(), extension);
   extension_prefs_->OnExtensionInstalled(
-      extension, initial_enable ? Extension::ENABLED : Extension::DISABLED,
-      initial_enable_incognito);
+      extension, initial_state, initial_enable_incognito);
 
   // Unpacked extensions default to allowing file access, but if that has been
   // overridden, don't reset the value.
   if (Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD) &&
-      !extension_prefs_->HasAllowFileAccessSetting(id)) {
-    extension_prefs_->SetAllowFileAccess(id, true);
+      !extension_prefs_->HasAllowFileAccessSetting(extension->id())) {
+    extension_prefs_->SetAllowFileAccess(extension->id(), true);
   }
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
@@ -1622,7 +1597,7 @@ void ExtensionService::OnExtensionInstalled(const Extension* extension) {
 
   if (extension->is_app()) {
     ExtensionIdSet installed_ids = GetAppIds();
-    installed_ids.insert(id);
+    installed_ids.insert(extension->id());
     default_apps_.DidInstallApp(installed_ids);
   }
 
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 54f70d6..a8f55ee 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -63,9 +63,6 @@ class ExtensionServiceInterface {
   virtual void UninstallExtension(const std::string& extension_id,
                                   bool external_uninstall) = 0;
 
-  virtual bool IsExtensionEnabled(const std::string& extension_id) const = 0;
-  virtual bool IsExternalExtensionUninstalled(
-      const std::string& extension_id) const = 0;
   virtual void EnableExtension(const std::string& extension_id) = 0;
   virtual void DisableExtension(const std::string& extension_id) = 0;
 
@@ -74,11 +71,17 @@ class ExtensionServiceInterface {
   virtual void CheckAdminBlacklist() = 0;
   virtual bool HasInstalledExtensions() = 0;
 
-  virtual bool IsIncognitoEnabled(const std::string& extension_id) const = 0;
-  virtual void SetIsIncognitoEnabled(const std::string& extension_id,
+  virtual void SetIsIncognitoEnabled(const Extension* extension,
                                      bool enabled) = 0;
 
-  virtual void CheckForUpdates() = 0;
+  // TODO(skerner): Change to const ExtensionPrefs& extension_prefs() const,
+  // ExtensionPrefs* mutable_extension_prefs().
+  virtual ExtensionPrefs* extension_prefs() = 0;
+  virtual const ExtensionPrefs& const_extension_prefs() const = 0;
+
+  virtual ExtensionUpdater* updater() = 0;
+
+  virtual Profile* profile() = 0;
 };
 
 // Manages installed and running Chromium extensions.
@@ -165,9 +168,8 @@ class ExtensionService
   DefaultApps* default_apps() { return &default_apps_; }
 
   // Whether this extension can run in an incognito window.
-  virtual bool IsIncognitoEnabled(const std::string& extension_id) const;
-  virtual void SetIsIncognitoEnabled(const std::string& extension_id,
-                                     bool enabled);
+  bool IsIncognitoEnabled(const Extension* extension);
+  virtual void SetIsIncognitoEnabled(const Extension* extension, bool enabled);
 
   // Returns true if the given extension can see events and data from another
   // sub-profile (incognito to original profile, or vice versa).
@@ -231,10 +233,6 @@ class ExtensionService
   virtual void UninstallExtension(const std::string& extension_id,
                                   bool external_uninstall);
 
-  virtual bool IsExtensionEnabled(const std::string& extension_id) const;
-  virtual bool IsExternalExtensionUninstalled(
-      const std::string& extension_id) const;
-
   // Enable or disable an extension. No action if the extension is already
   // enabled/disabled.
   virtual void EnableExtension(const std::string& extension_id);
@@ -337,8 +335,6 @@ class ExtensionService
   // set of extensions.
   virtual void CheckAdminBlacklist();
 
-  virtual void CheckForUpdates();
-
   void set_extensions_enabled(bool enabled) { extensions_enabled_ = enabled; }
   bool extensions_enabled() { return extensions_enabled_; }
 
@@ -350,22 +346,21 @@ class ExtensionService
     return show_extensions_prompts_;
   }
 
-  Profile* profile();
+  virtual Profile* profile();
 
   // Profile calls this when it is being destroyed so that we know not to call
   // it.
   void DestroyingProfile();
 
-  // TODO(skerner): Change to const ExtensionPrefs& extension_prefs() const,
-  // ExtensionPrefs* mutable_extension_prefs().
-  ExtensionPrefs* extension_prefs();
+  virtual ExtensionPrefs* extension_prefs();
+  virtual const ExtensionPrefs& const_extension_prefs() const;
 
   // Whether the extension service is ready.
   // TODO(skerner): Get rid of this method.  crbug.com/63756
   bool is_ready() { return ready_; }
 
   // Note that this may return NULL if autoupdate is not turned on.
-  ExtensionUpdater* updater();
+  virtual ExtensionUpdater* updater();
 
   ExtensionToolbarModel* toolbar_model() { return &toolbar_model_; }
 
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 4ab233f..b3d1028 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -2023,14 +2023,14 @@ TEST_F(ExtensionServiceTest, UpdateExtensionPreservesState) {
   // Disable it and allow it to run in incognito. These settings should carry
   // over to the updated version.
   service_->DisableExtension(good->id());
-  service_->SetIsIncognitoEnabled(good->id(), true);
+  service_->SetIsIncognitoEnabled(good, true);
 
   path = extensions_path.AppendASCII("good2.crx");
   UpdateExtension(good_crx, path, INSTALLED);
   ASSERT_EQ(1u, service_->disabled_extensions()->size());
   const Extension* good2 = service_->disabled_extensions()->at(0);
   ASSERT_EQ("1.0.0.1", good2->version()->GetString());
-  EXPECT_TRUE(service_->IsIncognitoEnabled(good2->id()));
+  EXPECT_TRUE(service_->IsIncognitoEnabled(good2));
 }
 
 // Tests that updating preserves extension location.
@@ -2167,7 +2167,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExtension) {
   EXPECT_EQ(kGoodInitialState,
             service_->extension_prefs()->GetExtensionState(extension->id()));
   EXPECT_EQ(kGoodInitialIncognitoEnabled,
-            service_->IsIncognitoEnabled(extension->id()));
+            service_->IsIncognitoEnabled(extension));
 }
 
 namespace {
@@ -2199,7 +2199,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingTheme) {
 
   EXPECT_EQ(Extension::ENABLED,
             service_->extension_prefs()->GetExtensionState(extension->id()));
-  EXPECT_FALSE(service_->IsIncognitoEnabled(extension->id()));
+  EXPECT_FALSE(service_->IsIncognitoEnabled(extension));
 }
 
 // Test updating a pending CRX as if the source is an external extension
@@ -2225,7 +2225,7 @@ TEST_F(ExtensionServiceTest, UpdatePendingExternalCrx) {
 
   EXPECT_EQ(Extension::ENABLED,
             service_->extension_prefs()->GetExtensionState(extension->id()));
-  EXPECT_FALSE(service_->IsIncognitoEnabled(extension->id()));
+  EXPECT_FALSE(service_->IsIncognitoEnabled(extension));
 }
 
 // Test updating a pending CRX as if the source is an external extension
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index 1c3a403..11bfeb7 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -238,7 +238,7 @@ int ExtensionToolbarModel::IncognitoIndexToOriginal(int incognito_index) {
   int original_index = 0, i = 0;
   for (ExtensionList::iterator iter = begin(); iter != end();
        ++iter, ++original_index) {
-    if (service_->IsIncognitoEnabled((*iter)->id())) {
+    if (service_->IsIncognitoEnabled(*iter)) {
       if (incognito_index == i)
         break;
       ++i;
@@ -253,7 +253,7 @@ int ExtensionToolbarModel::OriginalIndexToIncognito(int original_index) {
        ++iter, ++i) {
     if (original_index == i)
       break;
-    if (service_->IsIncognitoEnabled((*iter)->id()))
+    if (service_->IsIncognitoEnabled(*iter))
       ++incognito_index;
   }
   return incognito_index;
diff --git a/chrome/browser/extensions/extension_updater.cc b/chrome/browser/extensions/extension_updater.cc
index e14ba2a..22af3a1 100644
--- a/chrome/browser/extensions/extension_updater.cc
+++ b/chrome/browser/extensions/extension_updater.cc
@@ -205,11 +205,8 @@ static int CalculateActivePingDays(const Time& last_active_ping_day,
 }  // namespace
 
 ManifestFetchesBuilder::ManifestFetchesBuilder(
-    ExtensionServiceInterface* service,
-    ExtensionPrefs* prefs)
-    : service_(service), prefs_(prefs) {
+    ExtensionServiceInterface* service) : service_(service) {
   DCHECK(service_);
-  DCHECK(prefs_);
 }
 
 ManifestFetchesBuilder::~ManifestFetchesBuilder() {}
@@ -227,7 +224,8 @@ void ManifestFetchesBuilder::AddExtension(const Extension& extension) {
   // communicate to the the gallery update servers.
   std::string update_url_data;
   if (!extension.UpdatesFromGallery())
-    update_url_data = prefs_->GetUpdateUrlData(extension.id());
+    update_url_data = service_->extension_prefs()->
+        GetUpdateUrlData(extension.id());
 
   AddExtensionData(extension.location(),
                    extension.id(),
@@ -341,11 +339,11 @@ void ManifestFetchesBuilder::AddExtensionData(
       fetches_.find(update_url);
 
   // Find or create a ManifestFetchData to add this extension to.
+  ExtensionPrefs* prefs = service_->extension_prefs();
   ManifestFetchData::PingData ping_data;
-  ping_data.rollcall_days = CalculatePingDays(prefs_->LastPingDay(id));
-  ping_data.active_days =
-      CalculateActivePingDays(prefs_->LastActivePingDay(id),
-                              prefs_->GetActiveBit(id));
+  ping_data.rollcall_days = CalculatePingDays(prefs->LastPingDay(id));
+  ping_data.active_days = CalculateActivePingDays(prefs->LastActivePingDay(id),
+                                                  prefs->GetActiveBit(id));
   while (existing_iter != fetches_.end()) {
     if (existing_iter->second->AddExtension(id, version.GetString(),
                                             ping_data, update_url_data)) {
@@ -425,13 +423,10 @@ ExtensionUpdater::ExtensionFetch::ExtensionFetch(const std::string& i,
 ExtensionUpdater::ExtensionFetch::~ExtensionFetch() {}
 
 ExtensionUpdater::ExtensionUpdater(ExtensionServiceInterface* service,
-                                   ExtensionPrefs* extension_prefs,
                                    PrefService* prefs,
-                                   Profile* profile,
                                    int frequency_seconds)
     : alive_(false), service_(service), frequency_seconds_(frequency_seconds),
-      extension_prefs_(extension_prefs), prefs_(prefs), profile_(profile),
-      file_handler_(new ExtensionUpdaterFileHandler()),
+      prefs_(prefs), file_handler_(new ExtensionUpdaterFileHandler()),
       blacklist_checks_enabled_(true) {
   Init();
 }
@@ -511,9 +506,7 @@ void ExtensionUpdater::Start() {
   // If these are NULL, then that means we've been called after Stop()
   // has been called.
   DCHECK(service_);
-  DCHECK(extension_prefs_);
   DCHECK(prefs_);
-  DCHECK(profile_);
   alive_ = true;
   // Make sure our prefs are registered, then schedule the first check.
   EnsureInt64PrefRegistered(prefs_, kLastExtensionsUpdateCheck);
@@ -525,9 +518,7 @@ void ExtensionUpdater::Start() {
 void ExtensionUpdater::Stop() {
   alive_ = false;
   service_ = NULL;
-  extension_prefs_ = NULL;
   prefs_ = NULL;
-  profile_ = NULL;
   timer_.Stop();
   manifest_fetcher_.reset();
   extension_fetcher_.reset();
@@ -698,17 +689,18 @@ void ExtensionUpdater::HandleManifestResults(
 
     const std::set<std::string>& extension_ids = fetch_data.extension_ids();
     std::set<std::string>::const_iterator i;
+    ExtensionPrefs* prefs = service_->extension_prefs();
     for (i = extension_ids.begin(); i != extension_ids.end(); i++) {
       if (fetch_data.DidPing(*i, ManifestFetchData::ROLLCALL)) {
         if (*i == kBlacklistAppID) {
-          extension_prefs_->SetBlacklistLastPingDay(daystart);
+          prefs->SetBlacklistLastPingDay(daystart);
         } else if (service_->GetExtensionById(*i, true) != NULL) {
-          extension_prefs_->SetLastPingDay(*i, daystart);
+          prefs->SetLastPingDay(*i, daystart);
         }
       }
-      if (extension_prefs_->GetActiveBit(*i)) {
-        extension_prefs_->SetActiveBit(*i, false);
-        extension_prefs_->SetLastActivePingDay(*i, daystart);
+      if (prefs->GetActiveBit(*i)) {
+        prefs->SetActiveBit(*i, false);
+        prefs->SetLastActivePingDay(*i, daystart);
       }
     }
   }
@@ -847,7 +839,7 @@ void ExtensionUpdater::TimerFired() {
 void ExtensionUpdater::CheckNow() {
   DCHECK(alive_);
   NotifyStarted();
-  ManifestFetchesBuilder fetches_builder(service_, extension_prefs_);
+  ManifestFetchesBuilder fetches_builder(service_);
 
   const ExtensionList* extensions = service_->extensions();
   for (ExtensionList::const_iterator iter = extensions->begin();
@@ -883,7 +875,7 @@ void ExtensionUpdater::CheckNow() {
     std::string version = prefs_->GetString(kExtensionBlacklistUpdateVersion);
     ManifestFetchData::PingData ping_data;
     ping_data.rollcall_days =
-        CalculatePingDays(extension_prefs_->BlacklistLastPingDay());
+        CalculatePingDays(service_->extension_prefs()->BlacklistLastPingDay());
     blacklist_fetch->AddExtension(kBlacklistAppID, version, ping_data, "");
     StartUpdateCheck(blacklist_fetch);
   }
@@ -1011,7 +1003,7 @@ void ExtensionUpdater::StartUpdateCheck(ManifestFetchData* fetch_data) {
         URLFetcher::Create(kManifestFetcherId, fetch_data->full_url(),
                            URLFetcher::GET, this));
     manifest_fetcher_->set_request_context(
-        profile_->GetRequestContext());
+        service_->profile()->GetRequestContext());
     manifest_fetcher_->set_load_flags(net::LOAD_DO_NOT_SEND_COOKIES |
                                       net::LOAD_DO_NOT_SAVE_COOKIES |
                                       net::LOAD_DISABLE_CACHE);
@@ -1039,7 +1031,7 @@ void ExtensionUpdater::FetchUpdatedExtension(const std::string& id,
     extension_fetcher_.reset(
         URLFetcher::Create(kExtensionFetcherId, url, URLFetcher::GET, this));
     extension_fetcher_->set_request_context(
-        profile_->GetRequestContext());
+        service_->profile()->GetRequestContext());
     extension_fetcher_->set_load_flags(net::LOAD_DO_NOT_SEND_COOKIES |
                                        net::LOAD_DO_NOT_SAVE_COOKIES |
                                        net::LOAD_DISABLE_CACHE);
@@ -1051,14 +1043,14 @@ void ExtensionUpdater::FetchUpdatedExtension(const std::string& id,
 void ExtensionUpdater::NotifyStarted() {
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_UPDATING_STARTED,
-      Source<Profile>(profile_),
+      Source<Profile>(service_->profile()),
       NotificationService::NoDetails());
 }
 
 void ExtensionUpdater::NotifyUpdateFound(const std::string& extension_id) {
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_UPDATE_FOUND,
-      Source<Profile>(profile_),
+      Source<Profile>(service_->profile()),
       Details<const std::string>(&extension_id));
 }
 
@@ -1066,7 +1058,7 @@ void ExtensionUpdater::NotifyIfFinished() {
   if (in_progress_ids_.empty()) {
     NotificationService::current()->Notify(
         NotificationType::EXTENSION_UPDATING_FINISHED,
-        Source<Profile>(profile_),
+        Source<Profile>(service_->profile()),
         NotificationService::NoDetails());
     VLOG(1) << "Sending EXTENSION_UPDATING_FINISHED";
   }
diff --git a/chrome/browser/extensions/extension_updater.h b/chrome/browser/extensions/extension_updater.h
index e5be521..328762b 100644
--- a/chrome/browser/extensions/extension_updater.h
+++ b/chrome/browser/extensions/extension_updater.h
@@ -25,11 +25,9 @@
 #include "googleurl/src/gurl.h"
 
 class Extension;
-class ExtensionPrefs;
 class ExtensionUpdaterTest;
 class ExtensionUpdaterFileHandler;
 class PrefService;
-class Profile;
 
 // To save on server resources we can request updates for multiple extensions
 // in one manifest check. This class helps us keep track of the id's for a
@@ -104,8 +102,7 @@ class ManifestFetchData {
 // extensions and pending extensions.
 class ManifestFetchesBuilder {
  public:
-  ManifestFetchesBuilder(ExtensionServiceInterface* service,
-                         ExtensionPrefs* prefs);
+  explicit ManifestFetchesBuilder(ExtensionServiceInterface* service);
   ~ManifestFetchesBuilder();
 
   void AddExtension(const Extension& extension);
@@ -141,8 +138,7 @@ class ManifestFetchesBuilder {
                         Extension::Type extension_type,
                         GURL update_url,
                         const std::string& update_url_data);
-  ExtensionServiceInterface* const service_;
-  ExtensionPrefs* const prefs_;
+  ExtensionServiceInterface* service_;
 
   // List of data on fetches we're going to do. We limit the number of
   // extensions grouped together in one batch to avoid running into the limits
@@ -171,9 +167,7 @@ class ExtensionUpdater
   // extensions and installing updated ones. The |frequency_seconds| parameter
   // controls how often update checks are scheduled.
   ExtensionUpdater(ExtensionServiceInterface* service,
-                   ExtensionPrefs* extension_prefs,
                    PrefService* prefs,
-                   Profile* profile,
                    int frequency_seconds);
 
   // Starts the updater running.  Should be called at most once.
@@ -328,9 +322,7 @@ class ExtensionUpdater
   base::OneShotTimer<ExtensionUpdater> timer_;
   int frequency_seconds_;
 
-  ExtensionPrefs* extension_prefs_;
   PrefService* prefs_;
-  Profile* profile_;
 
   scoped_refptr<ExtensionUpdaterFileHandler> file_handler_;
   bool blacklist_checks_enabled_;
diff --git a/chrome/browser/extensions/extension_updater_unittest.cc b/chrome/browser/extensions/extension_updater_unittest.cc
index 16de5b4..5c93424 100644
--- a/chrome/browser/extensions/extension_updater_unittest.cc
+++ b/chrome/browser/extensions/extension_updater_unittest.cc
@@ -64,6 +64,12 @@ class MockService : public ExtensionServiceInterface {
     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 void UpdateExtension(const std::string& id,
                                const FilePath& path,
                                const GURL& download_url) {
@@ -81,17 +87,6 @@ class MockService : public ExtensionServiceInterface {
     FAIL();
   }
 
-  virtual bool IsExtensionEnabled(const std::string& extension_id) const {
-    ADD_FAILURE();
-    return false;
-  }
-
-  virtual bool IsExternalExtensionUninstalled(
-      const std::string& extension_id) const {
-    ADD_FAILURE();
-    return false;
-  }
-
   virtual void EnableExtension(const std::string& extension_id) {
     FAIL();
   }
@@ -115,29 +110,22 @@ class MockService : public ExtensionServiceInterface {
     return false;
   }
 
-  virtual bool IsIncognitoEnabled(const std::string& id) const {
-    ADD_FAILURE();
-    return false;
-  }
-
-  virtual void SetIsIncognitoEnabled(const std::string& id,
+  virtual void SetIsIncognitoEnabled(const Extension* extension,
                                      bool enabled) {
     FAIL();
   }
 
-  virtual void CheckForUpdates() {
-    FAIL();
+  virtual ExtensionPrefs* extension_prefs() { return prefs_.prefs(); }
+  virtual const ExtensionPrefs& const_extension_prefs() const {
+    return prefs_.const_prefs();
   }
 
-  virtual PendingExtensionManager* pending_extension_manager() {
-    ADD_FAILURE() << "Subclass should override this if it will "
-                  << "be accessed by a test.";
-    return &pending_extension_manager_;
+  virtual ExtensionUpdater* updater() {
+    ADD_FAILURE();
+    return NULL;
   }
 
-  Profile* profile() { return &profile_; }
-
-  ExtensionPrefs* extension_prefs() { return prefs_.prefs(); }
+  virtual Profile* profile() { return &profile_; }
 
   PrefService* pref_service() { return prefs_.pref_service(); }
 
@@ -387,9 +375,7 @@ class ExtensionUpdaterTest : public testing::Test {
     TestURLFetcherFactory factory;
     URLFetcher::set_factory(&factory);
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), 60*60*24));
+        new ExtensionUpdater(&service, service.pref_service(), 60*60*24));
     updater->Start();
 
     // Tell the update that it's time to do update checks.
@@ -437,9 +423,7 @@ class ExtensionUpdaterTest : public testing::Test {
     TestURLFetcherFactory factory;
     URLFetcher::set_factory(&factory);
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), 60*60*24));
+        new ExtensionUpdater(&service, service.pref_service(), 60*60*24));
     updater->Start();
 
     // Tell the updater that it's time to do update checks.
@@ -526,7 +510,7 @@ class ExtensionUpdaterTest : public testing::Test {
 
   static void TestUpdateUrlDataFromGallery(const std::string& gallery_url) {
     MockService service;
-    ManifestFetchesBuilder builder(&service, service.extension_prefs());
+    ManifestFetchesBuilder builder(&service);
     ExtensionList extensions;
     std::string url(gallery_url);
 
@@ -557,9 +541,8 @@ class ExtensionUpdaterTest : public testing::Test {
     service.set_extensions(tmp);
 
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
 
     // Check passing an empty list of parse results to DetermineUpdates
@@ -598,9 +581,8 @@ class ExtensionUpdaterTest : public testing::Test {
 
     MessageLoop message_loop;
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
 
     ManifestFetchData fetch_data(GURL("http://localhost/foo"));
@@ -636,9 +618,7 @@ class ExtensionUpdaterTest : public testing::Test {
     URLFetcher::set_factory(&factory);
     scoped_ptr<ServiceForDownloadTests> service(new ServiceForDownloadTests);
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(service.get(), service->extension_prefs(),
-                             service->pref_service(),
-                             service->profile(),
+        new ExtensionUpdater(service.get(), service->pref_service(),
                              kUpdateFrequencySecs));
     updater->Start();
 
@@ -710,9 +690,7 @@ class ExtensionUpdaterTest : public testing::Test {
     URLFetcher::set_factory(&factory);
     scoped_ptr<ServiceForDownloadTests> service(new ServiceForDownloadTests);
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(service.get(), service->extension_prefs(),
-                             service->pref_service(),
-                             service->profile(),
+        new ExtensionUpdater(service.get(), service->pref_service(),
                              kUpdateFrequencySecs));
     updater->Start();
 
@@ -781,9 +759,8 @@ class ExtensionUpdaterTest : public testing::Test {
     URLFetcher::set_factory(&factory);
     ServiceForBlacklistTests service;
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
     GURL test_url("http://localhost/extension.crx");
 
@@ -828,9 +805,8 @@ class ExtensionUpdaterTest : public testing::Test {
     URLFetcher::set_factory(&factory);
     ServiceForDownloadTests service;
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
 
     GURL url1("http://localhost/extension1.crx");
@@ -941,9 +917,8 @@ class ExtensionUpdaterTest : public testing::Test {
       prefs->SetActiveBit(id, true);
 
     scoped_refptr<ExtensionUpdater> updater(
-      new ExtensionUpdater(
-          &service, service.extension_prefs(), service.pref_service(),
-          service.profile(), kUpdateFrequencySecs));
+      new ExtensionUpdater(&service, service.pref_service(),
+                           kUpdateFrequencySecs));
     updater->Start();
     updater->set_blacklist_checks_enabled(false);
 
@@ -1007,9 +982,8 @@ class ExtensionUpdaterTest : public testing::Test {
     ServiceForManifestTests service;
     MessageLoop message_loop;
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
 
     GURL update_url("http://www.google.com/manifest");
@@ -1132,7 +1106,7 @@ TEST(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) {
   BrowserThread file_thread(BrowserThread::FILE, &message_loop);
 
   MockService service;
-  ManifestFetchesBuilder builder(&service, service.extension_prefs());
+  ManifestFetchesBuilder builder(&service);
 
   // Non-internal non-external extensions should be rejected.
   {
@@ -1184,9 +1158,8 @@ TEST(ExtensionUpdaterTest, TestStartUpdateCheckMemory) {
     TestURLFetcherFactory factory;
     URLFetcher::set_factory(&factory);
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
     updater->StartUpdateCheck(new ManifestFetchData(GURL()));
     // This should delete the newly-created ManifestFetchData.
@@ -1204,9 +1177,8 @@ TEST(ExtensionUpdaterTest, TestAfterStopBehavior) {
 
     ServiceForManifestTests service;
     scoped_refptr<ExtensionUpdater> updater(
-        new ExtensionUpdater(
-            &service, service.extension_prefs(), service.pref_service(),
-            service.profile(), kUpdateFrequencySecs));
+        new ExtensionUpdater(&service, service.pref_service(),
+                             kUpdateFrequencySecs));
     updater->Start();
     updater->Stop();
     // All the below functions should do nothing.
diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc
index ea99d34..a8d017a 100644
--- a/chrome/browser/extensions/extension_web_ui.cc
+++ b/chrome/browser/extensions/extension_web_ui.cc
@@ -284,7 +284,7 @@ bool ExtensionWebUI::HandleChromeURLOverride(GURL* url, Profile* profile) {
     // extension uses split mode.
     bool incognito_override_allowed =
         extension->incognito_split_mode() &&
-        service->IsIncognitoEnabled(extension->id());
+        service->IsIncognitoEnabled(extension);
     if (profile->IsOffTheRecord() && !incognito_override_allowed) {
       ++i;
       continue;
diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc
index 8aca103..3b95bee 100644
--- a/chrome/browser/extensions/extensions_ui.cc
+++ b/chrome/browser/extensions/extensions_ui.cc
@@ -411,8 +411,7 @@ void ExtensionsDOMHandler::HandleEnableIncognitoMessage(const ListValue* args) {
   //
   // Bug: http://crbug.com/41384
   ignore_notifications_ = true;
-  extensions_service_->SetIsIncognitoEnabled(extension_id,
-                                             enable_str == "true");
+  extensions_service_->SetIsIncognitoEnabled(extension, enable_str == "true");
   ignore_notifications_ = false;
 }
 
@@ -700,7 +699,7 @@ DictionaryValue* ExtensionsDOMHandler::CreateExtensionDetailValue(
   extension_data->SetBoolean("enabled", enabled);
   extension_data->SetBoolean("terminated", terminated);
   extension_data->SetBoolean("enabledIncognito",
-      service ? service->IsIncognitoEnabled(extension->id()) : false);
+      service ? service->IsIncognitoEnabled(extension) : false);
   extension_data->SetBoolean("wantsFileAccess", extension->wants_file_access());
   extension_data->SetBoolean("allowFileAccess",
       service ? service->AllowFileAccess(extension) : false);
diff --git a/chrome/browser/extensions/pending_extension_manager.cc b/chrome/browser/extensions/pending_extension_manager.cc
index 5b8e829..268df59 100644
--- a/chrome/browser/extensions/pending_extension_manager.cc
+++ b/chrome/browser/extensions/pending_extension_manager.cc
@@ -81,7 +81,7 @@ void PendingExtensionManager::AddFromExternalUpdateUrl(
   const bool kEnableOnInstall = true;
   const bool kEnableIncognitoOnInstall = false;
 
-  if (service_.IsExternalExtensionUninstalled(id))
+  if (service_.const_extension_prefs().IsExternalExtensionUninstalled(id))
     return;
 
   if (service_.GetExtensionById(id, true)) {
diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc
index 88903d8..f038ffc 100644
--- a/chrome/browser/extensions/user_script_master.cc
+++ b/chrome/browser/extensions/user_script_master.cc
@@ -344,7 +344,7 @@ void UserScriptMaster::Observe(NotificationType type,
       // Add any content scripts inside the extension.
       const Extension* extension = Details<const Extension>(details).ptr();
       bool incognito_enabled = profile_->GetExtensionService()->
-          IsIncognitoEnabled(extension->id());
+          IsIncognitoEnabled(extension);
       const UserScriptList& scripts = extension->content_scripts();
       for (UserScriptList::const_iterator iter = scripts.begin();
            iter != scripts.end(); ++iter) {
@@ -377,7 +377,7 @@ void UserScriptMaster::Observe(NotificationType type,
       const Extension* extension = Details<const Extension>(details).ptr();
       UserScriptList new_lone_scripts;
       bool incognito_enabled = profile_->GetExtensionService()->
-          IsIncognitoEnabled(extension->id());
+          IsIncognitoEnabled(extension);
       for (UserScriptList::iterator iter = lone_scripts_.begin();
            iter != lone_scripts_.end(); ++iter) {
         if (iter->extension_id() == extension->id()) {