ExtensionSyncService cleanup

Remove the extension_prefs_ and extension_service_ members; just query them on demand.

BUG=None

Review URL: https://codereview.chromium.org/1271273002

Cr-Commit-Position: refs/heads/master@{#342602}

4 files changed, 43 insertions, 48 deletions

diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 554f17c..899d3dc 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -5797,7 +5797,7 @@ TEST_F(ExtensionServiceTest, DeferredSyncStartupPreInstalledComponent) {
   bool flare_was_called = false;
   syncer::ModelType triggered_type(syncer::UNSPECIFIED);
   base::WeakPtrFactory<ExtensionServiceTest> factory(this);
-  extension_sync_service()->SetSyncStartFlare(
+  extension_sync_service()->SetSyncStartFlareForTesting(
       base::Bind(&ExtensionServiceTest::MockSyncStartFlare,
                  factory.GetWeakPtr(),
                  &flare_was_called,  // Safe due to WeakPtrFactory scope.
@@ -5823,7 +5823,7 @@ TEST_F(ExtensionServiceTest, DeferredSyncStartupPreInstalledNormal) {
   bool flare_was_called = false;
   syncer::ModelType triggered_type(syncer::UNSPECIFIED);
   base::WeakPtrFactory<ExtensionServiceTest> factory(this);
-  extension_sync_service()->SetSyncStartFlare(
+  extension_sync_service()->SetSyncStartFlareForTesting(
       base::Bind(&ExtensionServiceTest::MockSyncStartFlare,
                  factory.GetWeakPtr(),
                  &flare_was_called,  // Safe due to WeakPtrFactory scope.
@@ -5847,7 +5847,7 @@ TEST_F(ExtensionServiceTest, DeferredSyncStartupOnInstall) {
   bool flare_was_called = false;
   syncer::ModelType triggered_type(syncer::UNSPECIFIED);
   base::WeakPtrFactory<ExtensionServiceTest> factory(this);
-  extension_sync_service()->SetSyncStartFlare(
+  extension_sync_service()->SetSyncStartFlareForTesting(
       base::Bind(&ExtensionServiceTest::MockSyncStartFlare,
                  factory.GetWeakPtr(),
                  &flare_was_called,  // Safe due to WeakPtrFactory scope.
diff --git a/chrome/browser/extensions/extension_sync_service.cc b/chrome/browser/extensions/extension_sync_service.cc
index 344946d..cb85356 100644
--- a/chrome/browser/extensions/extension_sync_service.cc
+++ b/chrome/browser/extensions/extension_sync_service.cc
@@ -92,14 +92,9 @@ syncer::SyncDataList ToSyncerSyncDataList(
 
 }  // namespace
 
-ExtensionSyncService::ExtensionSyncService(Profile* profile,
-                                           ExtensionPrefs* extension_prefs,
-                                           ExtensionService* extension_service)
+ExtensionSyncService::ExtensionSyncService(Profile* profile)
     : profile_(profile),
-      extension_prefs_(extension_prefs),
-      extension_service_(extension_service) {
-  SetSyncStartFlare(sync_start_util::GetFlareForSyncableService(
-      profile_->GetPath()));
+      flare_(sync_start_util::GetFlareForSyncableService(profile->GetPath())) {
 }
 
 ExtensionSyncService::~ExtensionSyncService() {}
@@ -125,7 +120,7 @@ void ExtensionSyncService::SyncUninstallExtension(
   if (bundle->IsSyncing()) {
     bundle->PushSyncDeletion(extension.id(),
                              CreateSyncData(extension).GetSyncData());
-  } else if (extension_service_->is_ready() && !flare_.is_null()) {
+  } else if (extension_service()->is_ready() && !flare_.is_null()) {
     flare_.Run(type);  // Tell sync to start ASAP.
   }
 }
@@ -144,7 +139,7 @@ void ExtensionSyncService::SyncExtensionChangeIfNeeded(
     DCHECK(!ExtensionPrefs::Get(profile_)->NeedsSync(extension.id()));
   } else {
     ExtensionPrefs::Get(profile_)->SetNeedsSync(extension.id(), true);
-    if (extension_service_->is_ready() && !flare_.is_null())
+    if (extension_service()->is_ready() && !flare_.is_null())
       flare_.Run(type);  // Tell sync to start ASAP.
   }
 }
@@ -229,13 +224,14 @@ syncer::SyncError ExtensionSyncService::ProcessSyncChanges(
 
 ExtensionSyncData ExtensionSyncService::CreateSyncData(
     const Extension& extension) const {
-  bool enabled = extension_service_->IsExtensionEnabled(extension.id());
-  int disable_reasons = extension_prefs_->GetDisableReasons(extension.id());
+  const ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(profile_);
+  bool enabled = extension_service()->IsExtensionEnabled(extension.id());
+  int disable_reasons = extension_prefs->GetDisableReasons(extension.id());
   bool incognito_enabled = extensions::util::IsIncognitoEnabled(extension.id(),
                                                                 profile_);
   bool remote_install =
-      extension_prefs_->HasDisableReason(extension.id(),
-                                         Extension::DISABLE_REMOTE_INSTALL);
+      extension_prefs->HasDisableReason(extension.id(),
+                                        Extension::DISABLE_REMOTE_INSTALL);
   ExtensionSyncData::OptionalBoolean allowed_on_all_url =
       GetAllowedOnAllUrlsOptionalBoolean(extension.id(), profile_);
   if (extension.is_app()) {
@@ -245,7 +241,7 @@ ExtensionSyncData ExtensionSyncService::CreateSyncData(
         allowed_on_all_url,
         app_sorting->GetAppLaunchOrdinal(extension.id()),
         app_sorting->GetPageOrdinal(extension.id()),
-        extensions::GetLaunchTypePrefValue(extension_prefs_, extension.id()));
+        extensions::GetLaunchTypePrefValue(extension_prefs, extension.id()));
   }
   return ExtensionSyncData(
       extension, enabled, disable_reasons, incognito_enabled, remote_install,
@@ -285,7 +281,7 @@ bool ExtensionSyncService::ApplySyncData(
 
   if (!ApplyExtensionSyncDataHelper(extension_sync_data, type)) {
     bundle->AddPendingExtension(id, extension_sync_data);
-    extension_service_->CheckForUpdatesSoon();
+    extension_service()->CheckForUpdatesSoon();
     return false;
   }
 
@@ -304,7 +300,7 @@ void ExtensionSyncService::ApplyBookmarkAppSyncData(
   }
 
   const Extension* extension =
-      extension_service_->GetInstalledExtension(extension_sync_data.id());
+      extension_service()->GetInstalledExtension(extension_sync_data.id());
 
   // Return if there are no bookmark app details that need updating.
   if (extension &&
@@ -334,21 +330,25 @@ void ExtensionSyncService::ApplyBookmarkAppSyncData(
 
   // If the bookmark app already exists, keep the old icons.
   if (!extension) {
-    CreateOrUpdateBookmarkApp(extension_service_, &web_app_info);
+    CreateOrUpdateBookmarkApp(extension_service(), &web_app_info);
   } else {
     GetWebApplicationInfoFromApp(profile_,
                                  extension,
                                  base::Bind(&OnWebApplicationInfoLoaded,
                                             web_app_info,
-                                            extension_service_->AsWeakPtr()));
+                                            extension_service()->AsWeakPtr()));
   }
 }
 
-void ExtensionSyncService::SetSyncStartFlare(
+void ExtensionSyncService::SetSyncStartFlareForTesting(
     const syncer::SyncableService::StartSyncFlare& flare) {
   flare_ = flare;
 }
 
+ExtensionService* ExtensionSyncService::extension_service() const {
+  return ExtensionSystem::Get(profile_)->extension_service();
+}
+
 SyncBundle* ExtensionSyncService::GetSyncBundle(syncer::ModelType type) {
   return const_cast<SyncBundle*>(
       const_cast<const ExtensionSyncService&>(*this).GetSyncBundle(type));
@@ -413,8 +413,8 @@ bool ExtensionSyncService::ApplyExtensionSyncDataHelper(
 
   // Handle uninstalls first.
   if (extension_sync_data.uninstalled()) {
-    if (!extension_service_->UninstallExtensionHelper(
-            extension_service_, id, extensions::UNINSTALL_REASON_SYNC)) {
+    if (!ExtensionService::UninstallExtensionHelper(
+            extension_service(), id, extensions::UNINSTALL_REASON_SYNC)) {
       LOG(WARNING) << "Could not uninstall extension " << id << " for sync";
     }
     return true;
@@ -445,9 +445,9 @@ bool ExtensionSyncService::ApplyExtensionSyncDataHelper(
         extension_sync_data.supports_disable_reasons() &&
         extension && (version_compare_result == 0);
     if (grant_permissions)
-      extension_service_->GrantPermissionsAndEnableExtension(extension);
+      extension_service()->GrantPermissionsAndEnableExtension(extension);
     else
-      extension_service_->EnableExtension(id);
+      extension_service()->EnableExtension(id);
   } else {
     int disable_reasons = extension_sync_data.disable_reasons();
     if (extension_sync_data.remote_install()) {
@@ -468,7 +468,7 @@ bool ExtensionSyncService::ApplyExtensionSyncDataHelper(
     if (extension_sync_data.supports_disable_reasons())
       ExtensionPrefs::Get(profile_)->ClearDisableReasons(id);
 
-    extension_service_->DisableExtension(id, disable_reasons);
+    extension_service()->DisableExtension(id, disable_reasons);
   }
 
   // We need to cache some information here because setting the incognito flag
@@ -479,7 +479,7 @@ bool ExtensionSyncService::ApplyExtensionSyncDataHelper(
   // be promoted to a regular installed extension and downloading from the Web
   // Store is not necessary.
   if (extension && extensions::util::IsEphemeralApp(id, profile_))
-    extension_service_->PromoteEphemeralApp(extension, true);
+    extension_service()->PromoteEphemeralApp(extension, true);
 
   // Update the incognito flag.
   extensions::util::SetIsIncognitoEnabled(
@@ -502,7 +502,7 @@ bool ExtensionSyncService::ApplyExtensionSyncDataHelper(
     }
   } else {
     CHECK(type == syncer::EXTENSIONS || type == syncer::APPS);
-    if (!extension_service_->pending_extension_manager()->AddFromSync(
+    if (!extension_service()->pending_extension_manager()->AddFromSync(
             id,
             extension_sync_data.update_url(),
             extensions::sync_helper::IsSyncable,
diff --git a/chrome/browser/extensions/extension_sync_service.h b/chrome/browser/extensions/extension_sync_service.h
index 99b0661..cbc8fb5 100644
--- a/chrome/browser/extensions/extension_sync_service.h
+++ b/chrome/browser/extensions/extension_sync_service.h
@@ -34,10 +34,7 @@ class SyncErrorFactory;
 class ExtensionSyncService : public syncer::SyncableService,
                              public KeyedService {
  public:
-  ExtensionSyncService(Profile* profile,
-                       extensions::ExtensionPrefs* extension_prefs,
-                       ExtensionService* extension_service);
-
+  explicit ExtensionSyncService(Profile* profile);
   ~ExtensionSyncService() override;
 
   // Convenience function to get the ExtensionSyncService for a BrowserContext.
@@ -62,17 +59,24 @@ class ExtensionSyncService : public syncer::SyncableService,
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
 
-  // |flare| provides a StartSyncFlare to the SyncableService. See
-  // sync_start_util for more. Public for testing.
-  void SetSyncStartFlare(const syncer::SyncableService::StartSyncFlare& flare);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(TwoClientAppsSyncTest, UnexpectedLaunchType);
   FRIEND_TEST_ALL_PREFIXES(ExtensionDisabledGlobalErrorTest,
                            HigherPermissionsFromSync);
   FRIEND_TEST_ALL_PREFIXES(ExtensionDisabledGlobalErrorTest, RemoteInstall);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
+                           DeferredSyncStartupPreInstalledComponent);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
+                           DeferredSyncStartupPreInstalledNormal);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
+                           DeferredSyncStartupOnInstall);
   friend class EphemeralAppBrowserTest;
 
+  void SetSyncStartFlareForTesting(
+      const syncer::SyncableService::StartSyncFlare& flare);
+
+  ExtensionService* extension_service() const;
+
   // Gets the SyncBundle for the given |type|.
   extensions::SyncBundle* GetSyncBundle(syncer::ModelType type);
   const extensions::SyncBundle* GetSyncBundle(syncer::ModelType type) const;
@@ -111,14 +115,9 @@ class ExtensionSyncService : public syncer::SyncableService,
   void ApplyBookmarkAppSyncData(
       const extensions::ExtensionSyncData& extension_sync_data);
 
-  // The normal profile associated with this ExtensionService.
+  // The normal profile associated with this ExtensionSyncService.
   Profile* profile_;
 
-  // Preferences for the owning profile.
-  extensions::ExtensionPrefs* extension_prefs_;
-
-  ExtensionService* extension_service_;
-
   extensions::SyncBundle app_sync_bundle_;
   extensions::SyncBundle extension_sync_bundle_;
 
diff --git a/chrome/browser/extensions/extension_sync_service_factory.cc b/chrome/browser/extensions/extension_sync_service_factory.cc
index ea97ca3..54ac963 100644
--- a/chrome/browser/extensions/extension_sync_service_factory.cc
+++ b/chrome/browser/extensions/extension_sync_service_factory.cc
@@ -37,11 +37,7 @@ ExtensionSyncServiceFactory::~ExtensionSyncServiceFactory() {}
 
 KeyedService* ExtensionSyncServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  Profile* profile = Profile::FromBrowserContext(context);
-  return new ExtensionSyncService(
-      profile,
-      extensions::ExtensionPrefsFactory::GetForBrowserContext(context),
-      extensions::ExtensionSystem::Get(profile)->extension_service());
+  return new ExtensionSyncService(Profile::FromBrowserContext(context));
 }
 
 content::BrowserContext* ExtensionSyncServiceFactory::GetBrowserContextToUse(