[NTP::Cleanup] Avoid nested complex function calls

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 b3befc0d38ee143688df8522e50c1136b7af90f5..380dedb114f6d7bfd44ba91fa2d628f54290bc6c 100644
--- a/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
+++ b/chrome/browser/ntp_snippets/content_suggestions_service_factory.cc
@@ -169,23 +169,29 @@ void RegisterArticleProvider(SigninManagerBase* signin_manager,
           ->GetSequencedTaskRunnerWithShutdownBehavior(
               base::SequencedWorkerPool::GetSequenceToken(),
               base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
-  bool is_stable_channel =
-      chrome::GetChannel() == version_info::Channel::STABLE;
-  auto provider = base::MakeUnique<RemoteSuggestionsProviderImpl>(
-      service, pref_service, g_browser_process->GetApplicationLocale(),
-      service->category_ranker(),
-      base::MakeUnique<RemoteSuggestionsFetcher>(
-          signin_manager, token_service, request_context, pref_service,
-          language_model, base::Bind(&safe_json::SafeJsonParser::Parse),
-          is_stable_channel ? google_apis::GetAPIKey()
-                            : google_apis::GetNonStableAPIKey(),
-          service->user_classifier()),
-      base::MakeUnique<ImageFetcherImpl>(base::MakeUnique<ImageDecoderImpl>(),
-                                         request_context.get()),
-      base::MakeUnique<ImageDecoderImpl>(),
-      base::MakeUnique<RemoteSuggestionsDatabase>(database_dir, task_runner),
-      base::MakeUnique<RemoteSuggestionsStatusService>(signin_manager,
-                                                       pref_service));
+
+  std::unique_ptr<RemoteSuggestionsProviderImpl> provider;
+  {

[tschumann, 2017/02/01 15:07:59]

sorry for the late response. I'm not a fan of such scopes. They usually only
cure a symptom, not the underlying problem. Usually, the better answer for
reducing scope is to extract a function but there are a lot of variables to
pass.

I'd be in favor of the change that only moves 'suggestions_fetcher' into a
separate variable and leaves the rest unchanged. The original problem stemmed
from the long parameter list of the fetcher's constructor which might got
confused with the provider's constructor. So let's just fix that and keep the
rest.

+    bool is_stable_channel =
+        chrome::GetChannel() == version_info::Channel::STABLE;
+    auto suggestions_fetcher = base::MakeUnique<RemoteSuggestionsFetcher>(
+        signin_manager, token_service, request_context, pref_service,
+        language_model, base::Bind(&safe_json::SafeJsonParser::Parse),
+        is_stable_channel ? google_apis::GetAPIKey()
+                          : google_apis::GetNonStableAPIKey(),
+        service->user_classifier());
+    auto image_fetcher = base::MakeUnique<ImageFetcherImpl>(
+        base::MakeUnique<ImageDecoderImpl>(), request_context.get());
+    auto database =
+        base::MakeUnique<RemoteSuggestionsDatabase>(database_dir, task_runner);
+    auto status_service = base::MakeUnique<RemoteSuggestionsStatusService>(
+        signin_manager, pref_service);
+    provider = base::MakeUnique<RemoteSuggestionsProviderImpl>(
+        service, pref_service, g_browser_process->GetApplicationLocale(),
+        service->category_ranker(), std::move(suggestions_fetcher),
+        std::move(image_fetcher), base::MakeUnique<ImageDecoderImpl>(),
+        std::move(database), std::move(status_service));
+  }
 
   PersistentScheduler* scheduler = nullptr;
 #if defined(OS_ANDROID)