Make DownloadOfflineContentProvider a simple KeyedService.

Create DownloadOfflineContentProviderFactory, a
SimpleKeyedServiceFactory that creates and maintains ownership of
DownloadOfflineContentProviders associated to ProfileKeys.

This gets rid of Android-specific globals and platform-specific
ownership in DownloadCoreServiceImpl.

Change-Id: Iabee9d98f961849ebdfb35c678cf625db605705f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1664995
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671383}

chrome/browser/BUILD.gn

@@ -412,6 +412,8 @@ jumbo_split_static_library("browser") {
     "download/download_manager_utils.h",
     "download/download_offline_content_provider.cc",
     "download/download_offline_content_provider.h",
+    "download/download_offline_content_provider_factory.cc",
+    "download/download_offline_content_provider_factory.h",
     "download/download_permission_request.cc",
     "download/download_permission_request.h",
     "download/download_prefs.cc",

chrome/browser/android/download/download_controller.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/android/profile_key_util.h"
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/download/download_offline_content_provider.h"
+#include "chrome/browser/download/download_offline_content_provider_factory.h"
 #include "chrome/browser/download/download_stats.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/offline_pages/android/offline_page_bridge.h"
@@ -394,7 +395,7 @@ void DownloadController::OnDownloadStarted(
   ProfileKey* profile_key =
       profile ? profile->GetProfileKey() : ::android::GetMainProfileKey();
 
-  DownloadUtils::GetDownloadOfflineContentProvider(profile_key)
+  DownloadOfflineContentProviderFactory::GetForKey(profile_key)
       ->OnDownloadStarted(download_item);
 
   OnDownloadUpdated(download_item);

chrome/browser/android/download/download_utils.cc

@@ -10,13 +10,10 @@
 #include "base/metrics/field_trial_params.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/android/chrome_feature_list.h"
-#include "chrome/browser/download/download_offline_content_provider.h"
 #include "chrome/browser/download/offline_item_utils.h"
-#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/download/public/common/download_utils.h"
-#include "components/offline_items_collection/core/offline_content_aggregator.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/download_item_utils.h"
 #include "jni/DownloadUtils_jni.h"
@@ -27,8 +24,6 @@ using base::android::ConvertUTF16ToJavaString;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaParamRef;
 using base::android::ScopedJavaLocalRef;
-using OfflineContentAggregator =
-    offline_items_collection::OfflineContentAggregator;
 
 namespace {
 // If received bytes is more than the size limit and resumption will restart
@@ -36,9 +31,6 @@ namespace {
 int kDefaultAutoResumptionSizeLimit = 10 * 1024 * 1024;  // 10 MB
 const char kAutoResumptionSizeLimitParamName[] = "AutoResumptionSizeLimit";
 
-static DownloadOfflineContentProvider* g_download_provider = nullptr;
-static DownloadOfflineContentProvider* g_download_provider_incognito = nullptr;
-
 }  // namespace
 
 static ScopedJavaLocalRef<jstring> JNI_DownloadUtils_GetFailStateMessage(
@@ -113,30 +105,3 @@ std::string DownloadUtils::RemapGenericMimeType(const std::string& mime_type,
       ConvertUTF8ToJavaString(env, file_name));
   return ConvertJavaStringToUTF8(env, j_remapped_mime_type);
 }
-
-// static
-DownloadOfflineContentProvider*
-DownloadUtils::GetDownloadOfflineContentProvider(ProfileKey* profile_key) {
-  DCHECK(profile_key);
-
-  OfflineContentAggregator* aggregator =
-      OfflineContentAggregatorFactory::GetForKey(profile_key);
-
-  // Only create the provider if it is needed for the given |browser_context|.
-  if (!profile_key->IsOffTheRecord() && !g_download_provider) {
-    std::string name_space = OfflineContentAggregator::CreateUniqueNameSpace(
-        OfflineItemUtils::GetDownloadNamespacePrefix(false), false);
-    g_download_provider =
-        new DownloadOfflineContentProvider(aggregator, name_space);
-  }
-
-  if (profile_key->IsOffTheRecord() && !g_download_provider_incognito) {
-    std::string name_space = OfflineContentAggregator::CreateUniqueNameSpace(
-        OfflineItemUtils::GetDownloadNamespacePrefix(true), true);
-    g_download_provider_incognito =
-        new DownloadOfflineContentProvider(aggregator, name_space);
-  }
-
-  return profile_key->IsOffTheRecord() ? g_download_provider_incognito
-                                       : g_download_provider;
-}

chrome/browser/android/download/download_utils.h

@@ -13,9 +13,7 @@ namespace download {
 class DownloadItem;
 }
 
-class DownloadOfflineContentProvider;
 class GURL;
-class ProfileKey;
 
 // Native side of DownloadUtils.java.
 class DownloadUtils {
@@ -26,8 +24,6 @@ class DownloadUtils {
   static std::string RemapGenericMimeType(const std::string& mime_type,
                                           const GURL& url,
                                           const std::string& file_name);
-  static DownloadOfflineContentProvider* GetDownloadOfflineContentProvider(
-      ProfileKey* profile_key);
 };
 
 #endif  // CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_UTILS_H_

chrome/browser/download/download_core_service_impl.cc

@@ -10,16 +10,14 @@
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_offline_content_provider.h"
+#include "chrome/browser/download/download_offline_content_provider_factory.h"
 #include "chrome/browser/download/download_status_updater.h"
 #include "chrome/browser/download/download_ui_controller.h"
-#include "chrome/browser/download/offline_item_utils.h"
 #include "chrome/browser/download/simple_download_manager_coordinator_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
-#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/download/public/common/simple_download_manager_coordinator.h"
 #include "components/history/core/browser/history_service.h"
-#include "components/offline_items_collection/core/offline_content_aggregator.h"
 #include "content/public/browser/download_manager.h"
 #include "extensions/buildflags/buildflags.h"
 
@@ -74,7 +72,9 @@ DownloadCoreServiceImpl::GetDownloadManagerDelegate() {
                      new DownloadHistory::HistoryAdapter(history))));
   }
 
-  auto* download_provider = CreateDownloadOfflineContentProvider();
+  DownloadOfflineContentProvider* download_provider =
+      DownloadOfflineContentProviderFactory::GetForKey(
+          profile_->GetProfileKey());
   download_provider->SetSimpleDownloadManagerCoordinator(coordinator);
 
   // Pass an empty delegate when constructing the DownloadUIController. The
@@ -95,23 +95,6 @@ DownloadCoreServiceImpl::GetDownloadManagerDelegate() {
   return manager_delegate_.get();
 }
 
-DownloadOfflineContentProvider*
-DownloadCoreServiceImpl::CreateDownloadOfflineContentProvider() {
-#if defined(OS_ANDROID)
-  return DownloadUtils::GetDownloadOfflineContentProvider(
-      profile_->GetProfileKey());
-#else
-  download_provider_.reset(new DownloadOfflineContentProvider(
-      OfflineContentAggregatorFactory::GetForKey(
-          profile_->GetOriginalProfile()->GetProfileKey()),
-      offline_items_collection::OfflineContentAggregator::CreateUniqueNameSpace(
-          OfflineItemUtils::GetDownloadNamespacePrefix(
-              profile_->IsOffTheRecord()),
-          profile_->IsOffTheRecord())));
-  return download_provider_.get();
-#endif
-}
-
 DownloadHistory* DownloadCoreServiceImpl::GetDownloadHistory() {
   if (!download_manager_created_) {
     GetDownloadManagerDelegate();

chrome/browser/download/download_core_service_impl.h

@@ -19,7 +19,6 @@
 
 class ChromeDownloadManagerDelegate;
 class DownloadHistory;
-class DownloadOfflineContentProvider;
 class DownloadUIController;
 class ExtensionDownloadsEventRouter;
 class Profile;
@@ -55,8 +54,6 @@ class DownloadCoreServiceImpl : public DownloadCoreService {
   void Shutdown() override;
 
  private:
-  DownloadOfflineContentProvider* CreateDownloadOfflineContentProvider();
-
   bool download_manager_created_;
   Profile* profile_;
 
@@ -67,10 +64,6 @@ class DownloadCoreServiceImpl : public DownloadCoreService {
 
   std::unique_ptr<DownloadHistory> download_history_;
 
-  // The download provider is the responsible for supplying offline items to the
-  // UI.
-  std::unique_ptr<DownloadOfflineContentProvider> download_provider_;
-
   // The UI controller is responsible for observing the download manager and
   // notifying the UI of any new downloads. Its lifetime matches that of the
   // associated download manager.

chrome/browser/download/download_offline_content_provider.h

@@ -7,11 +7,13 @@
 
 #include <memory>
 #include <set>
+#include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "components/download/public/common/download_item.h"
 #include "components/download/public/common/simple_download_manager_coordinator.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "components/offline_items_collection/core/offline_content_aggregator.h"
 #include "components/offline_items_collection/core/offline_content_provider.h"
 
@@ -32,7 +34,8 @@ class SkBitmap;
 // SimpleDownloadManagerCoordinator and notifies UI about updates about various
 // downloads.
 class DownloadOfflineContentProvider
-    : public OfflineContentProvider,
+    : public KeyedService,
+      public OfflineContentProvider,
       public DownloadItem::Observer,
       public SimpleDownloadManagerCoordinator::Observer {
  public:

chrome/browser/download/download_offline_content_provider_factory.cc

+// Copyright 2019 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/download/download_offline_content_provider_factory.h"
+
+#include <string>
+
+#include "base/memory/singleton.h"
+#include "build/build_config.h"
+#include "chrome/browser/download/download_offline_content_provider.h"
+#include "chrome/browser/download/offline_item_utils.h"
+#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
+#include "chrome/browser/profiles/profile_key.h"
+#include "components/keyed_service/core/simple_dependency_manager.h"
+#include "components/offline_items_collection/core/offline_content_aggregator.h"
+
+// static
+DownloadOfflineContentProviderFactory*
+DownloadOfflineContentProviderFactory::GetInstance() {
+  return base::Singleton<DownloadOfflineContentProviderFactory>::get();
+}
+
+// static
+DownloadOfflineContentProvider*
+DownloadOfflineContentProviderFactory::GetForKey(SimpleFactoryKey* key) {
+  return static_cast<DownloadOfflineContentProvider*>(
+      GetInstance()->GetServiceForKey(key, true));
+}
+
+DownloadOfflineContentProviderFactory::DownloadOfflineContentProviderFactory()
+    : SimpleKeyedServiceFactory("DownloadOfflineContentProvider",
+                                SimpleDependencyManager::GetInstance()) {}
+
+DownloadOfflineContentProviderFactory::
+    ~DownloadOfflineContentProviderFactory() = default;
+
+std::unique_ptr<KeyedService>
+DownloadOfflineContentProviderFactory::BuildServiceInstanceFor(
+    SimpleFactoryKey* key) const {
+  OfflineContentAggregator* aggregator =
+      OfflineContentAggregatorFactory::GetForKey(key);
+  std::string name_space = OfflineContentAggregator::CreateUniqueNameSpace(
+      OfflineItemUtils::GetDownloadNamespacePrefix(key->IsOffTheRecord()),
+      key->IsOffTheRecord());
+  return std::make_unique<DownloadOfflineContentProvider>(aggregator,
+                                                          name_space);
+}
+
+SimpleFactoryKey* DownloadOfflineContentProviderFactory::GetKeyToUse(
+    SimpleFactoryKey* key) const {
+  return key;
+}

chrome/browser/download/download_offline_content_provider_factory.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_OFFLINE_CONTENT_PROVIDER_FACTORY_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_OFFLINE_CONTENT_PROVIDER_FACTORY_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "components/keyed_service/core/simple_keyed_service_factory.h"
+
+class DownloadOfflineContentProvider;
+class SimpleFactoryKey;
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}  // namespace base
+
+// This class builds and associates DownloadOfflineContentProvider with their
+// Profiles, represented by SimpleFactoryKeys.
+class DownloadOfflineContentProviderFactory : public SimpleKeyedServiceFactory {
+ public:
+  // Returns a singleton instance of an DownloadOfflineContentProviderFactory.
+  static DownloadOfflineContentProviderFactory* GetInstance();
+
+  // Returns the DownloadOfflineContentProvider associated with |key| or creates
+  // and associates one if it doesn't exist.
+  static DownloadOfflineContentProvider* GetForKey(SimpleFactoryKey* key);
+
+ private:
+  friend struct base::DefaultSingletonTraits<
+      DownloadOfflineContentProviderFactory>;
+
+  DownloadOfflineContentProviderFactory();
+  ~DownloadOfflineContentProviderFactory() override;
+
+  // SimpleKeyedServiceFactory implementation.
+  std::unique_ptr<KeyedService> BuildServiceInstanceFor(
+      SimpleFactoryKey* key) const override;
+  SimpleFactoryKey* GetKeyToUse(SimpleFactoryKey* key) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadOfflineContentProviderFactory);
+};
+
+#endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_OFFLINE_CONTENT_PROVIDER_FACTORY_H_