ContentSuggestionsService providers registration refactoring.

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

Index: chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
diff --git a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
index c377216b88e82d4dab595dbe9cee098f7462b4bb..142d91c87b9342dbf00a18aec8c76c4fdc073af2 100644
--- a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
+++ b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
@@ -77,6 +77,81 @@ void ClearScheduledTasks() {
 #endif  // OS_ANDROID
 }
 
+void RegisterOfflinePageSuggestionsProvider(ContentSuggestionsService* service,

[Marc Treib, 2016/08/12 12:18:45]

optional naming proposal: Should we remove "Suggestions" from all these function
names? It'd make them a bit shorter, and it's IMO clear from context.

[vitaliii, 2016/08/12 12:33:29]

Done.

+                                            Profile* profile) {
+  OfflinePageModel* offline_page_model =
+      OfflinePageModelFactory::GetForBrowserContext(profile);
+

[Marc Treib, 2016/08/12 12:18:45]

nit: I'd remove the empty line; the other Register* functions don't have it
either.

[vitaliii, 2016/08/12 12:33:29]

Done.

+  std::unique_ptr<OfflinePageSuggestionsProvider>
+      offline_page_suggestions_provider =
+          base::MakeUnique<OfflinePageSuggestionsProvider>(
+              service, service->category_factory(), offline_page_model);
+  service->RegisterProvider(std::move(offline_page_suggestions_provider));
+}
+
+void RegisterBookmarkSuggestionsProvider(ContentSuggestionsService* service,
+                                         Profile* profile) {
+  BookmarkModel* bookmark_model =
+      BookmarkModelFactory::GetForBrowserContext(profile);
+  std::unique_ptr<BookmarkSuggestionsProvider> bookmark_suggestions_provider =
+      base::MakeUnique<BookmarkSuggestionsProvider>(
+          service, service->category_factory(), bookmark_model);
+  service->RegisterProvider(std::move(bookmark_suggestions_provider));
+}
+
+void RegisterPhysicalWebSuggestionsProvider(
+    ContentSuggestionsService* service) {
+  std::unique_ptr<PhysicalWebPageSuggestionsProvider>
+      physical_web_page_suggestions_provider =
+          base::MakeUnique<PhysicalWebPageSuggestionsProvider>(
+              service, service->category_factory());
+  service->RegisterProvider(std::move(physical_web_page_suggestions_provider));
+}
+
+void RegisterArticleSuggestionsProvider(ContentSuggestionsService* service,
+                                        Profile* profile,
+                                        content::BrowserContext* context) {

[Marc Treib, 2016/08/12 12:18:45]

Profile and BrowserContext are actually the same, it's enough to pass just the
Profile and use that instead of context

[vitaliii, 2016/08/12 12:33:29]

Done.

+  SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfile(profile);
+  OAuth2TokenService* token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
+  scoped_refptr<net::URLRequestContextGetter> request_context =
+      content::BrowserContext::GetDefaultStoragePartition(context)
+          ->GetURLRequestContext();
+  SuggestionsService* suggestions_service =
+      SuggestionsServiceFactory::GetForProfile(profile);
+  NTPSnippetsScheduler* scheduler = nullptr;
+#if defined(OS_ANDROID)
+  scheduler = NTPSnippetsLauncher::Get();
+#endif  // OS_ANDROID
+  base::FilePath database_dir(
+      profile->GetPath().Append(ntp_snippets::kDatabaseFolder));
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      BrowserThread::GetBlockingPool()
+          ->GetSequencedTaskRunnerWithShutdownBehavior(
+              base::SequencedWorkerPool::GetSequenceToken(),
+              base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+  bool is_stable_channel =
+      chrome::GetChannel() == version_info::Channel::STABLE;
+  std::unique_ptr<NTPSnippetsService> ntp_snippets_service =
+      base::MakeUnique<NTPSnippetsService>(
+          service, service->category_factory(), profile->GetPrefs(),
+          suggestions_service, g_browser_process->GetApplicationLocale(),
+          scheduler,
+          base::MakeUnique<NTPSnippetsFetcher>(
+              signin_manager, token_service, request_context,
+              profile->GetPrefs(),
+              base::Bind(&safe_json::SafeJsonParser::Parse), is_stable_channel),
+          base::MakeUnique<ImageFetcherImpl>(
+              base::MakeUnique<ImageDecoderImpl>(), request_context.get()),
+          base::MakeUnique<ImageDecoderImpl>(),
+          base::MakeUnique<NTPSnippetsDatabase>(database_dir, task_runner),
+          base::MakeUnique<NTPSnippetsStatusService>(signin_manager,
+                                                     profile->GetPrefs()));
+  service->set_ntp_snippets_service(ntp_snippets_service.get());
+  service->RegisterProvider(std::move(ntp_snippets_service));
+}
+
 }  // namespace
 
 // static
@@ -127,85 +202,25 @@ KeyedService* ContentSuggestionsServiceFactory::BuildServiceInstanceFor(
   }
 
 #if defined(OS_ANDROID)
-  // Create the OfflinePageSuggestionsProvider.
   if (base::FeatureList::IsEnabled(
-      ntp_snippets::kOfflinePageSuggestionsFeature)) {
-    OfflinePageModel* offline_page_model =
-        OfflinePageModelFactory::GetForBrowserContext(profile);
-
-    std::unique_ptr<OfflinePageSuggestionsProvider>
-        offline_page_suggestions_provider =
-            base::MakeUnique<OfflinePageSuggestionsProvider>(
-                service, service->category_factory(), offline_page_model);
-    service->RegisterProvider(std::move(offline_page_suggestions_provider));
+          ntp_snippets::kOfflinePageSuggestionsFeature)) {
+    RegisterOfflinePageSuggestionsProvider(service, profile);
   }
 #endif  // OS_ANDROID
 
-  // Create the BookmarkSuggestionsProvider.
-  if (base::FeatureList::IsEnabled(
-          ntp_snippets::kBookmarkSuggestionsFeature)) {
-    BookmarkModel* bookmark_model =
-        BookmarkModelFactory::GetForBrowserContext(profile);
-    std::unique_ptr<BookmarkSuggestionsProvider> bookmark_suggestions_provider =
-        base::MakeUnique<BookmarkSuggestionsProvider>(
-            service, service->category_factory(), bookmark_model);
-    service->RegisterProvider(std::move(bookmark_suggestions_provider));
+  if (base::FeatureList::IsEnabled(ntp_snippets::kBookmarkSuggestionsFeature)) {
+    RegisterBookmarkSuggestionsProvider(service, profile);
   }
 
 #if defined(OS_ANDROID)
-  // Create the PhysicalWebPageSuggestionsProvider.
   if (base::FeatureList::IsEnabled(
           ntp_snippets::kPhysicalWebPageSuggestionsFeature)) {
-    std::unique_ptr<PhysicalWebPageSuggestionsProvider>
-        physical_web_page_suggestions_provider =
-            base::MakeUnique<PhysicalWebPageSuggestionsProvider>(
-                service, service->category_factory());
-    service->RegisterProvider(
-        std::move(physical_web_page_suggestions_provider));
+    RegisterPhysicalWebSuggestionsProvider(service);
   }
 #endif  // OS_ANDROID
 
   if (base::FeatureList::IsEnabled(ntp_snippets::kArticleSuggestionsFeature)) {
-    // Create the NTPSnippetsService (articles provider).
-    SigninManagerBase* signin_manager =
-        SigninManagerFactory::GetForProfile(profile);
-    OAuth2TokenService* token_service =
-        ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
-    scoped_refptr<net::URLRequestContextGetter> request_context =
-        content::BrowserContext::GetDefaultStoragePartition(context)
-            ->GetURLRequestContext();
-    SuggestionsService* suggestions_service =
-        SuggestionsServiceFactory::GetForProfile(profile);
-    NTPSnippetsScheduler* scheduler = nullptr;
-#if defined(OS_ANDROID)
-    scheduler = NTPSnippetsLauncher::Get();
-#endif  // OS_ANDROID
-    base::FilePath database_dir(
-        profile->GetPath().Append(ntp_snippets::kDatabaseFolder));
-    scoped_refptr<base::SequencedTaskRunner> task_runner =
-        BrowserThread::GetBlockingPool()
-            ->GetSequencedTaskRunnerWithShutdownBehavior(
-                base::SequencedWorkerPool::GetSequenceToken(),
-                base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
-    bool is_stable_channel =
-        chrome::GetChannel() == version_info::Channel::STABLE;
-    std::unique_ptr<NTPSnippetsService> ntp_snippets_service =
-        base::MakeUnique<NTPSnippetsService>(
-            service, service->category_factory(), profile->GetPrefs(),
-            suggestions_service, g_browser_process->GetApplicationLocale(),
-            scheduler, base::MakeUnique<NTPSnippetsFetcher>(
-                           signin_manager, token_service, request_context,
-                           profile->GetPrefs(),
-                           base::Bind(&safe_json::SafeJsonParser::Parse),
-                           is_stable_channel),
-            base::MakeUnique<ImageFetcherImpl>(
-                base::MakeUnique<ImageDecoderImpl>(), request_context.get()),
-            base::MakeUnique<ImageDecoderImpl>(),
-            base::MakeUnique<NTPSnippetsDatabase>(database_dir, task_runner),
-            base::MakeUnique<NTPSnippetsStatusService>(signin_manager,
-                                                       profile->GetPrefs()));
-    service->set_ntp_snippets_service(ntp_snippets_service.get());
-    service->RegisterProvider(std::move(ntp_snippets_service));
+    RegisterArticleSuggestionsProvider(service, profile, context);
   }
 
   return service;