Add ExploreSitesService to dependency manager.

ExploreSitesService is a lightweight object that lazily initiates heavier resources on demand.
Currently, it is created by UI (NTP page being open), however it starts subsequent
background activities (refreshing the catalog of sites, and collectingusage statistics)
so it should be started even if UI does not require it. This CL adds the service to dependency
manager list so it is created early in the initialization of Profile.

Bug: 905120
Change-Id: I63c30decd2bc9e70729438cc06a596089efcd585
Reviewed-on: https://chromium-review.googlesource.com/c/1334936
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610996}

chrome/browser/android/explore_sites/explore_sites_service_factory.cc

@@ -29,10 +29,27 @@ namespace explore_sites {
 const base::FilePath::CharType kExploreSitesStoreDirname[] =
     FILE_PATH_LITERAL("Explore");
 
+class URLLoaderFactoryGetterImpl
+    : public ExploreSitesServiceImpl::URLLoaderFactoryGetter {
+ public:
+  explicit URLLoaderFactoryGetterImpl(Profile* profile) : profile_(profile) {}
+
+  scoped_refptr<network::SharedURLLoaderFactory> GetFactory() override {
+    return profile_->GetURLLoaderFactory();
+  }
+
+ private:
+  Profile* profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(URLLoaderFactoryGetterImpl);
+};
+
 ExploreSitesServiceFactory::ExploreSitesServiceFactory()
     : BrowserContextKeyedServiceFactory(
           "ExploreSitesService",
-          BrowserContextDependencyManager::GetInstance()) {}
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(HistoryServiceFactory::GetInstance());
+}
 ExploreSitesServiceFactory::~ExploreSitesServiceFactory() = default;
 
 // static
@@ -62,8 +79,8 @@ KeyedService* ExploreSitesServiceFactory::BuildServiceInstanceFor(
       profile->GetPath().Append(kExploreSitesStoreDirname);
   auto explore_sites_store =
       std::make_unique<ExploreSitesStore>(background_task_runner, store_path);
-  scoped_refptr<network::SharedURLLoaderFactory> url_fetcher =
-      profile->GetURLLoaderFactory();
+  auto url_loader_factory_getter =
+      std::make_unique<URLLoaderFactoryGetterImpl>(profile);
   history::HistoryService* history_service =
       HistoryServiceFactory::GetForProfile(profile,
                                            ServiceAccessType::EXPLICIT_ACCESS);
@@ -71,7 +88,7 @@ KeyedService* ExploreSitesServiceFactory::BuildServiceInstanceFor(
       history_service, profile->GetPrefs(), base::DefaultClock::GetInstance());
 
   return new ExploreSitesServiceImpl(std::move(explore_sites_store),
-                                     url_fetcher,
+                                     std::move(url_loader_factory_getter),
                                      std::move(history_stats_reporter));
 }
 }  // namespace explore_sites

chrome/browser/android/explore_sites/explore_sites_service_impl.cc

@@ -33,11 +33,11 @@ namespace explore_sites {
 
 ExploreSitesServiceImpl::ExploreSitesServiceImpl(
     std::unique_ptr<ExploreSitesStore> store,
-    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    std::unique_ptr<URLLoaderFactoryGetter> url_loader_factory_getter,
     std::unique_ptr<HistoryStatisticsReporter> history_statistics_reporter)
     : task_queue_(this),
       explore_sites_store_(std::move(store)),
-      url_loader_factory_(url_loader_factory),
+      url_loader_factory_getter_(std::move(url_loader_factory_getter)),
       history_statistics_reporter_(std::move(history_statistics_reporter)),
       weak_ptr_factory_(this) {
   if (IsExploreSitesEnabled()) {
@@ -213,7 +213,7 @@ void ExploreSitesServiceImpl::GotVersionToStartFetch(
   // Create a fetcher and start fetching the protobuf (async).
   explore_sites_fetcher_ = ExploreSitesFetcher::CreateForGetCatalog(
       is_immediate_fetch, catalog_version, accept_languages,
-      url_loader_factory_,
+      url_loader_factory_getter_->GetFactory(),
       base::BindOnce(&ExploreSitesServiceImpl::OnCatalogFetched,
                      weak_ptr_factory_.GetWeakPtr()));
   explore_sites_fetcher_->Start();

chrome/browser/android/explore_sites/explore_sites_service_impl.h

@@ -24,9 +24,18 @@ namespace explore_sites {
 class ExploreSitesServiceImpl : public ExploreSitesService,
                                 public TaskQueue::Delegate {
  public:
+  // Helper class used to inject a dependency that can later vend a
+  // URLLoaderFactory. URLLoaderFactory fetching requires fully initialized
+  // Profile which is generally available later than service creation time.
+  class URLLoaderFactoryGetter {
+   public:
+    virtual ~URLLoaderFactoryGetter() {}
+    virtual scoped_refptr<network::SharedURLLoaderFactory> GetFactory() = 0;
+  };
+
   ExploreSitesServiceImpl(
       std::unique_ptr<ExploreSitesStore> store,
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      std::unique_ptr<URLLoaderFactoryGetter> url_loader_factory_getter,
       std::unique_ptr<HistoryStatisticsReporter> history_statistics_reporter);
   ~ExploreSitesServiceImpl() override;
 
@@ -93,7 +102,7 @@ class ExploreSitesServiceImpl : public ExploreSitesService,
   // Used to control access to the ExploreSitesStore.
   TaskQueue task_queue_;
   std::unique_ptr<ExploreSitesStore> explore_sites_store_;
-  scoped_refptr<network ::SharedURLLoaderFactory> url_loader_factory_;
+  std::unique_ptr<URLLoaderFactoryGetter> url_loader_factory_getter_;
   std::unique_ptr<ExploreSitesFetcher> explore_sites_fetcher_;
   std::unique_ptr<HistoryStatisticsReporter> history_statistics_reporter_;
   std::vector<BooleanCallback> update_catalog_callbacks_;

chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc

@@ -48,7 +48,9 @@ class ExploreSitesServiceImplTest : public testing::Test {
     auto history_stats_reporter =
         std::make_unique<HistoryStatisticsReporter>(nullptr, nullptr, nullptr);
     service_ = std::make_unique<ExploreSitesServiceImpl>(
-        std::move(store), test_shared_url_loader_factory_,
+        std::move(store),
+        std::make_unique<TestURLLoaderFactoryGetter>(
+            test_shared_url_loader_factory_),
         std::move(history_stats_reporter));
     success_ = false;
     test_data_ = CreateTestDataProto();
@@ -104,6 +106,21 @@ class ExploreSitesServiceImplTest : public testing::Test {
   void ValidateTestCatalog();
 
  private:
+  class TestURLLoaderFactoryGetter
+      : public ExploreSitesServiceImpl::URLLoaderFactoryGetter {
+   public:
+    explicit TestURLLoaderFactoryGetter(
+        scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
+        : url_loader_factory_(url_loader_factory) {}
+    scoped_refptr<network::SharedURLLoaderFactory> GetFactory() override {
+      return url_loader_factory_;
+    }
+
+   private:
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+    DISALLOW_COPY_AND_ASSIGN(TestURLLoaderFactoryGetter);
+  };
+
   std::unique_ptr<explore_sites::ExploreSitesServiceImpl> service_;
   bool success_;
   int callback_count_;

chrome/browser/android/explore_sites/history_statistics_reporter.cc

@@ -41,7 +41,9 @@ HistoryStatisticsReporter::HistoryStatisticsReporter(
       history_service_observer_(this),
       weak_ptr_factory_(this) {}
 
-HistoryStatisticsReporter::~HistoryStatisticsReporter() {}
+HistoryStatisticsReporter::~HistoryStatisticsReporter() {
+  history_service_observer_.RemoveAll();
+}
 
 void HistoryStatisticsReporter::ScheduleReportStatistics() {
   // Only try to report stats once per session.
@@ -71,6 +73,11 @@ void HistoryStatisticsReporter::OnHistoryServiceLoaded(
   ComputeStatistics();
 }
 
+void HistoryStatisticsReporter::HistoryServiceBeingDeleted(
+    history::HistoryService* history_service) {
+  history_service_observer_.RemoveAll();
+}
+
 void HistoryStatisticsReporter::MaybeReportStatistics() {
   if (history_service_->BackendLoaded()) {
     // HistoryService is already loaded. Continue with Initialization.

chrome/browser/android/explore_sites/history_statistics_reporter.h

@@ -33,6 +33,8 @@ class HistoryStatisticsReporter : public history::HistoryServiceObserver {
   // history::HistoryServiceObserver:
   void OnHistoryServiceLoaded(
       history::HistoryService* history_service) override;
+  void HistoryServiceBeingDeleted(
+      history::HistoryService* history_service) override;
 
   // If needed, computes/reports of history statistics. Uses Prefs to only
   // run weekly (approximately).

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -94,6 +94,7 @@
 #include "printing/buildflags/buildflags.h"
 
 #if defined(OS_ANDROID)
+#include "chrome/browser/android/explore_sites/explore_sites_service_factory.h"
 #include "chrome/browser/android/search_permissions/search_permissions_service.h"
 #else
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
@@ -255,6 +256,9 @@ void ChromeBrowserMainExtraPartsProfiles::
 #endif
   EnhancedBookmarkKeyServiceFactory::GetInstance();
 #endif
+#if defined(OS_ANDROID)
+  explore_sites::ExploreSitesServiceFactory::GetInstance();
+#endif
 #if defined(OS_CHROMEOS)
   chromeos::CupsPrintJobManagerFactory::GetInstance();
   chromeos::CupsPrintersManagerFactory::GetInstance();