[NTP::Cleanup] Avoid nested complex function calls

chrome/browser/ntp_snippets/content_suggestions_service_factory.cc

 // Copyright 2016 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/ntp_snippets/content_suggestions_service_factory.h"
 
 #include <utility>
 
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
@@ skipping 153 matching lines @@
 
   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;
+  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_decoder_for_fetcher = base::MakeUnique<ImageDecoderImpl>();
+  auto image_fetcher = base::MakeUnique<ImageFetcherImpl>(
+      std::move(image_decoder_for_fetcher), request_context.get());
+  auto image_decoder = base::MakeUnique<ImageDecoderImpl>();
+  auto database =
+      base::MakeUnique<RemoteSuggestionsDatabase>(database_dir, task_runner);
+  auto status_service = base::MakeUnique<RemoteSuggestionsStatusService>(
+      signin_manager, pref_service);
   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));
+      service->category_ranker(), std::move(suggestions_fetcher),
+      std::move(image_fetcher), std::move(image_decoder), std::move(database),
+      std::move(status_service));

[tschumann, 2017/02/01 11:35:37]

I'm having a hard time following the motivation.
Clearly, moving the fetcher into a separate variable makes the code easier to
read. But that's completely independent of using MakeUnique.
For example, I don't see the benefit of moving the other parameters into
variables (as (1) there's another layer of indirection now, (2) they are still
in scope after the call but moved out, which is not an easy state to reason
about.

[Marc Treib, 2017/02/01 12:33:20]

Maybe put all of this into a scope, so the variables won't exist after the call?

[vitaliii, 2017/02/01 13:15:15]

Done.

1) I inlined MakeUnique calls, which have no arguments;

2) For ArticlesProvider I placed all of them into a scope, so (2) is not
applicable anymore. 

As for (1) I still think that this way it is more readable + this discourages
inlining MakeUnique calls with arguments.

 
   PersistentScheduler* scheduler = nullptr;
 #if defined(OS_ANDROID)
   scheduler = NTPSnippetsLauncher::Get();
 #endif  // OS_ANDROID
 
+  auto scheduling_provider_clock = base::MakeUnique<base::DefaultClock>();
   auto scheduling_provider =
       base::MakeUnique<SchedulingRemoteSuggestionsProvider>(
           service, std::move(provider), scheduler, service->user_classifier(),
-          pref_service, base::MakeUnique<base::DefaultClock>());
+          pref_service, std::move(scheduling_provider_clock));
   service->set_remote_suggestions_provider(scheduling_provider.get());
   service->set_remote_suggestions_scheduler(scheduling_provider.get());
   service->RegisterProvider(std::move(scheduling_provider));
 }
 
 void RegisterForeignSessionsProvider(SyncService* sync_service,
                                      ContentSuggestionsService* service,
                                      PrefService* pref_service) {
   std::unique_ptr<TabDelegateSyncAdapter> sync_adapter =
       base::MakeUnique<TabDelegateSyncAdapter>(sync_service);
@@ skipping 45 matching lines @@
   using State = ContentSuggestionsService::State;
   Profile* profile = Profile::FromBrowserContext(context);
   DCHECK(!profile->IsOffTheRecord());
 
   // Create the ContentSuggestionsService.
   SigninManagerBase* signin_manager =
       SigninManagerFactory::GetForProfile(profile);
   HistoryService* history_service = HistoryServiceFactory::GetForProfile(
       profile, ServiceAccessType::EXPLICIT_ACCESS);
   PrefService* pref_service = profile->GetPrefs();
+  auto ranker_clock = base::MakeUnique<base::DefaultClock>();
   std::unique_ptr<CategoryRanker> category_ranker =
-      ntp_snippets::BuildSelectedCategoryRanker(
-          pref_service, base::MakeUnique<base::DefaultClock>());
+      ntp_snippets::BuildSelectedCategoryRanker(pref_service,
+                                                std::move(ranker_clock));
   auto* service =
       new ContentSuggestionsService(State::ENABLED, signin_manager,
           history_service, pref_service, std::move(category_ranker));
 
 #if defined(OS_ANDROID)
   OfflinePageModel* offline_page_model =
       OfflinePageModelFactory::GetForBrowserContext(profile);
   DownloadManager* download_manager =
       content::BrowserContext::GetDownloadManager(profile);
   DownloadService* download_service =
@@ skipping 46 matching lines @@
                             language_model, pref_service, profile);
   }
 
   if (base::FeatureList::IsEnabled(
           ntp_snippets::kForeignSessionsSuggestionsFeature)) {
     RegisterForeignSessionsProvider(sync_service, service, pref_service);
   }
 
   return service;
 }