[IC] Implement service for CachedImageFetcher

Bug: 872370
Change-Id: Idda57db7658d52b09224686f3f4925e6f57d7113
Reviewed-on: https://chromium-review.googlesource.com/c/1247402Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600088}

chrome/browser/BUILD.gn

@@ -244,6 +244,8 @@ jumbo_split_static_library("browser") {
     "browsing_data/site_data_size_collector.h",
     "cache_stats_recorder.cc",
     "cache_stats_recorder.h",
+    "cached_image_fetcher/cached_image_fetcher_service_factory.cc",
+    "cached_image_fetcher/cached_image_fetcher_service_factory.h",
     "chooser_controller/chooser_controller.cc",
     "chooser_controller/chooser_controller.h",
     "chrome_browser_application_mac.h",
@@ -1781,6 +1783,7 @@ jumbo_split_static_library("browser") {
     "//components/history/content/browser",
     "//components/history/core/browser",
     "//components/history/core/common",
+    "//components/image_fetcher/core",
     "//components/infobars/core",
     "//components/invalidation/impl",
     "//components/keyed_service/content",

chrome/browser/cached_image_fetcher/cached_image_fetcher_service_factory.cc

+// Copyright 2018 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/cached_image_fetcher/cached_image_fetcher_service_factory.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/files/file_path.h"
+#include "base/sequenced_task_runner.h"
+#include "base/task/post_task.h"
+#include "base/time/default_clock.h"
+#include "build/build_config.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/suggestions/image_decoder_impl.h"
+#include "components/image_fetcher/core/cached_image_fetcher_service.h"
+#include "components/image_fetcher/core/storage/image_cache.h"
+#include "components/image_fetcher/core/storage/image_data_store_disk.h"
+#include "components/image_fetcher/core/storage/image_metadata_store_leveldb.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+
+#if defined(OS_ANDROID)
+#include "base/path_service.h"
+#endif
+
+namespace image_fetcher {
+
+namespace {
+
+// The path under the browser context's data directory which the image_cache
+// will be stored.
+const base::FilePath::CharType kImageCacheSubdir[] =
+    FILE_PATH_LITERAL("image_cache");
+
+std::unique_ptr<ImageDecoder> CreateImageDecoderImpl() {
+  return std::make_unique<suggestions::ImageDecoderImpl>();
+}
+
+}  // namespace
+
+CachedImageFetcherService*
+CachedImageFetcherServiceFactory::GetForBrowserContext(
+    content::BrowserContext* context) {
+  return static_cast<CachedImageFetcherService*>(
+      GetInstance()->GetServiceForBrowserContext(context, true));
+}
+
+CachedImageFetcherServiceFactory*
+CachedImageFetcherServiceFactory::GetInstance() {
+  return base::Singleton<CachedImageFetcherServiceFactory>::get();
+}
+
+CachedImageFetcherServiceFactory::CachedImageFetcherServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+          "CachedImageFetcherService",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+CachedImageFetcherServiceFactory::~CachedImageFetcherServiceFactory() = default;
+
+KeyedService* CachedImageFetcherServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  base::FilePath cache_path;
+#if defined(OS_ANDROID)
+  // On Android, get a special cache directory that is cleared under pressure.
+  // The subdirectory under needs to be registered file_paths.xml as well.
+  if (base::PathService::Get(base::DIR_CACHE, &cache_path)) {
+    cache_path = cache_path.Append(kImageCacheSubdir);
+  }
+#else
+  // On other platforms, GetCachePath can be cleared by the user.
+  cache_path = context->GetCachePath().Append(kImageCacheSubdir);
+#endif
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner =
+      base::CreateSequencedTaskRunnerWithTraits(
+          {base::MayBlock(), base::TaskPriority::USER_VISIBLE});
+  base::DefaultClock* clock = base::DefaultClock::GetInstance();
+
+  auto metadata_store = std::make_unique<ImageMetadataStoreLevelDB>(
+      cache_path, task_runner, clock);
+  auto data_store =
+      std::make_unique<ImageDataStoreDisk>(cache_path, task_runner);
+
+  Profile* profile = Profile::FromBrowserContext(context);
+  // TODO(wylieb): Start in read-only mode if user is incognito.
+  scoped_refptr<ImageCache> image_cache = base::MakeRefCounted<ImageCache>(
+      std::move(data_store), std::move(metadata_store), profile->GetPrefs(),
+      clock, task_runner);
+
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
+      content::BrowserContext::GetDefaultStoragePartition(profile)
+          ->GetURLLoaderFactoryForBrowserProcess();
+
+  return new CachedImageFetcherService(
+      base::BindRepeating(CreateImageDecoderImpl),
+      std::move(url_loader_factory), std::move(image_cache));
+}
+
+content::BrowserContext*
+CachedImageFetcherServiceFactory::GetBrowserContextToUse(
+    content::BrowserContext* context) const {
+  // Return different BrowserContexts for regular/incognito.
+  return context;
+}
+
+}  // namespace image_fetcher

chrome/browser/cached_image_fetcher/cached_image_fetcher_service_factory.h

+// Copyright 2018 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_CACHED_IMAGE_FETCHER_CACHED_IMAGE_FETCHER_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_CACHED_IMAGE_FETCHER_CACHED_IMAGE_FETCHER_SERVICE_FACTORY_H_
+
+#include "base/macros.h"
+#include "base/memory/singleton.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace image_fetcher {
+
+class CachedImageFetcherService;
+
+// Factory to create one CachedImageFetcherService per browser context.
+class CachedImageFetcherServiceFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static CachedImageFetcherService* GetForBrowserContext(
+      content::BrowserContext* context);
+  static CachedImageFetcherServiceFactory* GetInstance();
+
+ private:
+  friend struct base::DefaultSingletonTraits<CachedImageFetcherServiceFactory>;
+
+  CachedImageFetcherServiceFactory();
+  ~CachedImageFetcherServiceFactory() override;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+  content::BrowserContext* GetBrowserContextToUse(
+      content::BrowserContext* context) const override;
+
+  DISALLOW_COPY_AND_ASSIGN(CachedImageFetcherServiceFactory);
+};
+
+}  // namespace image_fetcher
+
+#endif  // CHROME_BROWSER_CACHED_IMAGE_FETCHER_CACHED_IMAGE_FETCHER_SERVICE_FACTORY_H_

components/image_fetcher/core/BUILD.gn

@@ -6,6 +6,8 @@ static_library("core") {
   sources = [
     "cached_image_fetcher.cc",
     "cached_image_fetcher.h",
+    "cached_image_fetcher_service.cc",
+    "cached_image_fetcher_service.h",
     "image_data_fetcher.cc",
     "image_data_fetcher.h",
     "image_decoder.h",
@@ -22,6 +24,7 @@ static_library("core") {
   public_deps = [
     "//base",
     "//components/data_use_measurement/core",
+    "//components/keyed_service/core",
     "//net",
     "//services/network/public/cpp",
     "//ui/gfx",

components/image_fetcher/core/DEPS

 include_rules = [
+  "+components/keyed_service",
   "+components/leveldb_proto",
   "+components/prefs",
   "+ui/gfx/codec",

components/image_fetcher/core/cached_image_fetcher.cc

@@ -55,9 +55,9 @@ bool EncodeSkBitmapToPNG(const SkBitmap& bitmap,
 
 CachedImageFetcher::CachedImageFetcher(
     std::unique_ptr<ImageFetcher> image_fetcher,
-    std::unique_ptr<ImageCache> image_cache)
+    scoped_refptr<ImageCache> image_cache)
     : image_fetcher_(std::move(image_fetcher)),
-      image_cache_(std::move(image_cache)),
+      image_cache_(image_cache),
       weak_ptr_factory_(this) {
   DCHECK(image_fetcher_);
   DCHECK(image_cache_);

components/image_fetcher/core/cached_image_fetcher.h

@@ -36,7 +36,7 @@ struct RequestMetadata;
 class CachedImageFetcher : public ImageFetcher {
  public:
   CachedImageFetcher(std::unique_ptr<ImageFetcher> image_fetcher,
-                     std::unique_ptr<ImageCache> image_cache);
+                     scoped_refptr<ImageCache> image_cache);
   ~CachedImageFetcher() override;
 
   // ImageFetcher:
@@ -89,8 +89,11 @@ class CachedImageFetcher : public ImageFetcher {
   void OnImageDecodedFromNetwork(const GURL& image_url,
                                  const gfx::Image& image);
 
+  // ImageFetcher has some state that's stored, so it's owned by
+  // CachedImageFetcher.
   std::unique_ptr<ImageFetcher> image_fetcher_;
-  std::unique_ptr<ImageCache> image_cache_;
+
+  scoped_refptr<ImageCache> image_cache_;
 
   gfx::Size desired_image_frame_size_;
 

components/image_fetcher/core/cached_image_fetcher_service.cc

+// Copyright 2018 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 "components/image_fetcher/core/cached_image_fetcher_service.h"
+
+#include <utility>
+
+#include "base/time/clock.h"
+#include "components/image_fetcher/core/cached_image_fetcher.h"
+#include "components/image_fetcher/core/image_decoder.h"
+#include "components/image_fetcher/core/image_fetcher_impl.h"
+#include "components/image_fetcher/core/storage/image_cache.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+
+namespace image_fetcher {
+
+CachedImageFetcherService::CachedImageFetcherService(
+    CreateImageDecoderCallback create_image_decoder_fn,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    scoped_refptr<ImageCache> image_cache)
+    : create_image_decoder_callback_(create_image_decoder_fn),
+      url_loader_factory_(url_loader_factory),
+      image_cache_(image_cache) {}
+
+CachedImageFetcherService::~CachedImageFetcherService() = default;
+
+// TODO(wylieb): Store CachedImageFetcher once it's stateless.
+std::unique_ptr<CachedImageFetcher>
+CachedImageFetcherService::CreateCachedImageFetcher() {
+  return std::make_unique<CachedImageFetcher>(
+      std::make_unique<ImageFetcherImpl>(create_image_decoder_callback_.Run(),
+                                         url_loader_factory_),
+      image_cache_);
+}
+
+}  // namespace image_fetcher

components/image_fetcher/core/cached_image_fetcher_service.h

+// Copyright 2018 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 COMPONENTS_IMAGE_FETCHER_CORE_CACHED_IMAGE_FETCHER_SERVICE_H_
+#define COMPONENTS_IMAGE_FETCHER_CORE_CACHED_IMAGE_FETCHER_SERVICE_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
+namespace image_fetcher {
+
+class CachedImageFetcher;
+class ImageCache;
+class ImageDecoder;
+
+using CreateImageDecoderCallback =
+    base::RepeatingCallback<std::unique_ptr<ImageDecoder>()>;
+
+// Keyed service responsible for managing the lifetime of CachedImageFetcher.
+// Persists the ImageCache, and uses it to create instances of the
+// CachedImageFethcer.
+class CachedImageFetcherService : public KeyedService {
+ public:
+  explicit CachedImageFetcherService(
+      CreateImageDecoderCallback create_image_decoder_callback,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      scoped_refptr<ImageCache> image_cache);
+  ~CachedImageFetcherService() override;
+
+  // Create an instance of CachedImageFetcher based on the ImageCache.
+  std::unique_ptr<CachedImageFetcher> CreateCachedImageFetcher();
+
+ private:
+  CreateImageDecoderCallback create_image_decoder_callback_;
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  scoped_refptr<ImageCache> image_cache_;
+
+  DISALLOW_COPY_AND_ASSIGN(CachedImageFetcherService);
+};
+
+}  // namespace image_fetcher
+
+#endif  // COMPONENTS_IMAGE_FETCHER_CORE_CACHED_IMAGE_FETCHER_SERVICE_H_

components/image_fetcher/core/cached_image_fetcher_unittest.cc

@@ -12,6 +12,7 @@
 
 #include "base/bind.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted.h"
 #include "base/test/mock_callback.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/test/simple_test_clock.h"
@@ -72,10 +73,9 @@ class ComponentizedCachedImageFetcherTest : public testing::Test {
         data_dir_.GetPath(), base::SequencedTaskRunnerHandle::Get());
 
     ImageCache::RegisterProfilePrefs(test_prefs_.registry());
-    auto image_cache = std::make_unique<ImageCache>(
+    image_cache_ = base::MakeRefCounted<ImageCache>(
         std::move(data_store), std::move(metadata_store), &test_prefs_, &clock_,
         base::SequencedTaskRunnerHandle::Get());
-    image_cache_ = image_cache.get();
 
     image_cache_->SaveImage(kImageUrl.spec(), kImageData);
     RunUntilIdle();
@@ -93,7 +93,7 @@ class ComponentizedCachedImageFetcherTest : public testing::Test {
     cached_image_fetcher_ = std::make_unique<CachedImageFetcher>(
         std::make_unique<image_fetcher::ImageFetcherImpl>(std::move(decoder),
                                                           shared_factory_),
-        std::move(image_cache));
+        image_cache_);
 
     RunUntilIdle();
   }
@@ -103,7 +103,7 @@ class ComponentizedCachedImageFetcherTest : public testing::Test {
   CachedImageFetcher* cached_image_fetcher() {
     return cached_image_fetcher_.get();
   }
-  ImageCache* image_cache() { return image_cache_; }
+  scoped_refptr<ImageCache> image_cache() { return image_cache_; }
   FakeImageDecoder* image_decoder() { return fake_image_decoder_; }
   network::TestURLLoaderFactory* test_url_loader_factory() {
     return &test_url_loader_factory_;
@@ -117,7 +117,7 @@ class ComponentizedCachedImageFetcherTest : public testing::Test {
   scoped_refptr<network::SharedURLLoaderFactory> shared_factory_;
   FakeImageDecoder* fake_image_decoder_;
 
-  ImageCache* image_cache_;
+  scoped_refptr<ImageCache> image_cache_;
   base::SimpleTestClock clock_;
   TestingPrefServiceSimple test_prefs_;
   base::ScopedTempDir data_dir_;

components/image_fetcher/core/storage/image_cache.h

@@ -7,7 +7,9 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "components/image_fetcher/core/storage/image_store_types.h"
 
@@ -21,12 +23,13 @@ class SequencedTaskRunner;
 
 namespace image_fetcher {
 
+class ImageCache;
 class ImageDataStore;
 class ImageMetadataStore;
 
 // Persist image meta/data via the given implementations of ImageDataStore and
 // ImageMetadataStore.
-class ImageCache {
+class ImageCache : public base::RefCounted<ImageCache> {
  public:
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
@@ -35,7 +38,6 @@ class ImageCache {
              PrefService* pref_service,
              base::Clock* clock,
              scoped_refptr<base::SequencedTaskRunner> task_runner);
-  ~ImageCache();
 
   // Adds or updates the image data for the |url|. If the class hasn't been
   // initialized yet, the call is queued.
@@ -50,6 +52,8 @@ class ImageCache {
 
  private:
   friend class ImageCacheTest;
+  friend class base::RefCounted<ImageCache>;
+  ~ImageCache();
 
   // Queue or start |request| depending if the cache is initialized.
   void QueueOrStartRequest(base::OnceClosure request);

components/image_fetcher/core/storage/image_cache_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "components/image_fetcher/core/storage/image_cache.h"
 
+#include <map>
+#include <utility>
+
 #include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
@@ -49,7 +52,7 @@ class ImageCacheTest : public testing::Test {
     data_store_ = data_store.get();
 
     ImageCache::RegisterProfilePrefs(test_prefs_.registry());
-    image_cache_ = std::make_unique<ImageCache>(
+    image_cache_ = base::MakeRefCounted<ImageCache>(
         std::move(data_store), std::move(metadata_store), &test_prefs_, &clock_,
         base::SequencedTaskRunnerHandle::Get());
   }
@@ -116,7 +119,7 @@ class ImageCacheTest : public testing::Test {
   MOCK_METHOD1(DataCallback, void(std::string));
 
  private:
-  std::unique_ptr<ImageCache> image_cache_;
+  scoped_refptr<ImageCache> image_cache_;
   ImageMetadataStoreLevelDB* metadata_store_;
   ImageDataStoreDisk* data_store_;
   base::SimpleTestClock clock_;