[AiS] ImageService

Add a service to request & cache images for Answers in Suggest.

This service allows the UI thread to request images that are fetched
via http(s) and then decoded in a render process, caching the most
recent images to avoid repeated network requests.

BUG=380916

Review URL: https://codereview.chromium.org/319623003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278311 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/android/banners/app_banner_manager.cc

@@ -11,7 +11,7 @@
 #include "chrome/browser/android/banners/app_banner_metrics_ids.h"
 #include "chrome/browser/android/banners/app_banner_settings_helper.h"
 #include "chrome/browser/android/banners/app_banner_utilities.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/android/content_view_core.h"

chrome/browser/android/banners/app_banner_manager.h

@@ -9,7 +9,7 @@
 #include "base/android/jni_weak_ref.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/android/meta_tag_observer.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 
 namespace content {
 struct FrameNavigateParams;

chrome/browser/bitmap_fetcher.h

@@ -2,81 +2,4 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_BITMAP_FETCHER_H_
-#define CHROME_BROWSER_BITMAP_FETCHER_H_
-
-#include "base/memory/scoped_ptr.h"
-#include "chrome/browser/bitmap_fetcher_delegate.h"
-#include "chrome/browser/image_decoder.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request.h"
-#include "third_party/skia/include/core/SkBitmap.h"
-#include "url/gurl.h"
-
-namespace net {
-class URLFetcher;
-class URLRequestContextGetter;
-}  // namespace net
-
-namespace chrome {
-
-// Asynchrounously fetches an image from the given URL and returns the
-// decoded Bitmap to the provided BitmapFetcherDelegate.
-class BitmapFetcher : public net::URLFetcherDelegate,
-                      public ImageDecoder::Delegate {
- public:
-  BitmapFetcher(const GURL& url, BitmapFetcherDelegate* delegate);
-  virtual ~BitmapFetcher();
-
-  const GURL& url() const { return url_; }
-
-  // Start fetching the URL with the fetcher. The delegate is notified
-  // asynchronously when done.  Start may be called more than once in some
-  // cases.  If so, subsequent starts will be ignored since the operation is
-  // already in progress.  Arguments are used to configure the internal fetcher.
-  // Values for |load_flags| are defined in net/base/load_flags.h.  In general,
-  // |net::LOAD_NORMAL| is appropriate.
-  void Start(net::URLRequestContextGetter* request_context,
-             const std::string& referrer,
-             net::URLRequest::ReferrerPolicy referrer_policy,
-             int load_flags);
-
-  // Methods inherited from URLFetcherDelegate
-
-  // This will be called when the URL has been fetched, successfully or not.
-  // Use accessor methods on |source| to get the results.
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
-
-  // This will be called when some part of the response is read. |current|
-  // denotes the number of bytes received up to the call, and |total| is the
-  // expected total size of the response (or -1 if not determined).
-  virtual void OnURLFetchDownloadProgress(const net::URLFetcher* source,
-                                          int64 current,
-                                          int64 total) OVERRIDE;
-
-  // Methods inherited from ImageDecoder::Delegate
-
-  // Called when image is decoded. |decoder| is used to identify the image in
-  // case of decoding several images simultaneously.  This will not be called
-  // on the UI thread.
-  virtual void OnImageDecoded(const ImageDecoder* decoder,
-                              const SkBitmap& decoded_image) OVERRIDE;
-
-  // Called when decoding image failed.
-  virtual void OnDecodeImageFailed(const ImageDecoder* decoder) OVERRIDE;
-
- private:
-  // Alerts the delegate that a failure occurred.
-  void ReportFailure();
-
-  scoped_ptr<net::URLFetcher> url_fetcher_;
-  scoped_refptr<ImageDecoder> image_decoder_;
-  const GURL url_;
-  BitmapFetcherDelegate* const delegate_;
-
-  DISALLOW_COPY_AND_ASSIGN(BitmapFetcher);
-};
-
-}  // namespace chrome
-
-#endif  // CHROME_BROWSER_BITMAP_FETCHER_H_
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"

chrome/browser/bitmap_fetcher.cc → chrome/browser/bitmap_fetcher/bitmap_fetcher.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 
 #include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_fetcher.h"
@@ -11,13 +11,12 @@
 
 namespace chrome {
 
-BitmapFetcher::BitmapFetcher(const GURL& url,
-                             BitmapFetcherDelegate* delegate)
-    : url_(url),
-      delegate_(delegate) {
+BitmapFetcher::BitmapFetcher(const GURL& url, BitmapFetcherDelegate* delegate)
+    : url_(url), delegate_(delegate) {
 }
 
-BitmapFetcher::~BitmapFetcher() {}
+BitmapFetcher::~BitmapFetcher() {
+}
 
 void BitmapFetcher::Start(net::URLRequestContextGetter* request_context,
                           const std::string& referrer,

chrome/browser/bitmap_fetcher/bitmap_fetcher.h

+// Copyright 2014 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_BITMAP_FETCHER_BITMAP_FETCHER_H_
+#define CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/bitmap_fetcher_delegate.h"
+#include "chrome/browser/image_decoder.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "net/url_request/url_request.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "url/gurl.h"
+
+namespace net {
+class URLFetcher;
+class URLRequestContextGetter;
+}  // namespace net
+
+namespace chrome {
+
+// Asynchrounously fetches an image from the given URL and returns the
+// decoded Bitmap to the provided BitmapFetcherDelegate.
+class BitmapFetcher : public net::URLFetcherDelegate,
+                      public ImageDecoder::Delegate {
+ public:
+  BitmapFetcher(const GURL& url, BitmapFetcherDelegate* delegate);
+  virtual ~BitmapFetcher();
+
+  const GURL& url() const { return url_; }
+
+  // Start fetching the URL with the fetcher. The delegate is notified
+  // asynchronously when done.  Start may be called more than once in some
+  // cases.  If so, subsequent starts will be ignored since the operation is
+  // already in progress.  Arguments are used to configure the internal fetcher.
+  // Values for |load_flags| are defined in net/base/load_flags.h.  In general,
+  // |net::LOAD_NORMAL| is appropriate.
+  void Start(net::URLRequestContextGetter* request_context,
+             const std::string& referrer,
+             net::URLRequest::ReferrerPolicy referrer_policy,
+             int load_flags);
+
+  // Methods inherited from URLFetcherDelegate
+
+  // This will be called when the URL has been fetched, successfully or not.
+  // Use accessor methods on |source| to get the results.
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // This will be called when some part of the response is read. |current|
+  // denotes the number of bytes received up to the call, and |total| is the
+  // expected total size of the response (or -1 if not determined).
+  virtual void OnURLFetchDownloadProgress(const net::URLFetcher* source,
+                                          int64 current,
+                                          int64 total) OVERRIDE;
+
+  // Methods inherited from ImageDecoder::Delegate
+
+  // Called when image is decoded. |decoder| is used to identify the image in
+  // case of decoding several images simultaneously.  This will not be called
+  // on the UI thread.
+  virtual void OnImageDecoded(const ImageDecoder* decoder,
+                              const SkBitmap& decoded_image) OVERRIDE;
+
+  // Called when decoding image failed.
+  virtual void OnDecodeImageFailed(const ImageDecoder* decoder) OVERRIDE;
+
+ private:
+  // Alerts the delegate that a failure occurred.
+  void ReportFailure();
+
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+  scoped_refptr<ImageDecoder> image_decoder_;
+  const GURL url_;
+  BitmapFetcherDelegate* const delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcher);
+};
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_H_

chrome/browser/bitmap_fetcher/bitmap_fetcher_service.cc

+// Copyright 2014 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/bitmap_fetcher/bitmap_fetcher_service.h"
+
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
+#include "chrome/browser/profiles/profile.h"
+#include "net/base/load_flags.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace {
+
+const size_t kMaxRequests = 25;  // Maximum number of inflight requests allowed.
+const int kMaxCacheEntries = 5;  // Maximum number of cache entries.
+
+}  // namespace.
+
+class BitmapFetcherRequest {
+ public:
+  BitmapFetcherRequest(BitmapFetcherService::RequestId request_id,
+                       BitmapFetcherService::Observer* observer);
+  ~BitmapFetcherRequest();
+
+  void NotifyImageChanged(const SkBitmap& bitmap);
+  BitmapFetcherService::RequestId request_id() const { return request_id_; }
+
+  // Weak ptr |fetcher| is used to identify associated fetchers.
+  void set_fetcher(const chrome::BitmapFetcher* fetcher) { fetcher_ = fetcher; }
+  const chrome::BitmapFetcher* get_fetcher() const { return fetcher_; }
+
+ private:
+  const BitmapFetcherService::RequestId request_id_;
+  scoped_ptr<BitmapFetcherService::Observer> observer_;
+  const chrome::BitmapFetcher* fetcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcherRequest);
+};
+
+BitmapFetcherRequest::BitmapFetcherRequest(
+    BitmapFetcherService::RequestId request_id,
+    BitmapFetcherService::Observer* observer)
+    : request_id_(request_id), observer_(observer) {
+}
+
+BitmapFetcherRequest::~BitmapFetcherRequest() {
+  SkBitmap empty_bitmap;
+
+  // OnImageChanged with an empty bitmap signifies a completed request.
+  observer_->OnImageChanged(request_id_, empty_bitmap);
+}
+
+void BitmapFetcherRequest::NotifyImageChanged(const SkBitmap& bitmap) {
+  if (!bitmap.empty())
+    observer_->OnImageChanged(request_id_, bitmap);
+}
+
+BitmapFetcherService::CacheEntry::CacheEntry() {
+}
+
+BitmapFetcherService::CacheEntry::~CacheEntry() {
+}
+
+BitmapFetcherService::BitmapFetcherService(content::BrowserContext* context)
+    : cache_(kMaxCacheEntries), current_request_id_(1), context_(context) {
+}
+
+BitmapFetcherService::~BitmapFetcherService() {
+}
+
+void BitmapFetcherService::CancelRequest(int request_id) {
+  ScopedVector<BitmapFetcherRequest>::iterator iter;
+  for (iter = requests_.begin(); iter != requests_.end(); ++iter) {
+    if ((*iter)->request_id() == request_id) {
+      requests_.erase(iter);
+      // Deliberately leave the associated fetcher running to populate cache.
+      return;
+    }
+  }
+}
+
+BitmapFetcherService::RequestId BitmapFetcherService::RequestImage(
+    const GURL& url,
+    Observer* observer) {
+  // Create a new request, assigning next available request ID.
+  ++current_request_id_;
+  if (current_request_id_ == REQUEST_ID_INVALID)
+    ++current_request_id_;
+  int request_id = current_request_id_;
+  scoped_ptr<BitmapFetcherRequest> request(
+      new BitmapFetcherRequest(request_id, observer));
+
+  // Check for existing images first.
+  base::OwningMRUCache<GURL, CacheEntry*>::iterator iter = cache_.Get(url);
+  if (iter != cache_.end()) {
+    BitmapFetcherService::CacheEntry* entry = iter->second;
+    request->NotifyImageChanged(*(entry->bitmap.get()));
+
+    // There is no request ID associated with this - data is already delivered.
+    return REQUEST_ID_INVALID;
+  }
+
+  // Limit number of simultaneous in-flight requests.
+  if (requests_.size() > kMaxRequests)
+    return REQUEST_ID_INVALID;
+
+  // Make sure there's a fetcher for this URL and attach to request.
+  const chrome::BitmapFetcher* fetcher = EnsureFetcherForUrl(url);
+  request->set_fetcher(fetcher);
+
+  requests_.push_back(request.release());
+  return requests_.back()->request_id();
+}
+
+void BitmapFetcherService::Prefetch(const GURL& url) {
+  EnsureFetcherForUrl(url);
+}
+
+chrome::BitmapFetcher* BitmapFetcherService::CreateFetcher(const GURL& url) {
+  chrome::BitmapFetcher* new_fetcher = new chrome::BitmapFetcher(url, this);
+
+  new_fetcher->Start(
+      context_->GetRequestContext(),
+      std::string(),
+      net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
+      net::LOAD_NORMAL);
+  return new_fetcher;
+}
+
+const chrome::BitmapFetcher* BitmapFetcherService::EnsureFetcherForUrl(
+    const GURL& url) {
+  const chrome::BitmapFetcher* fetcher = FindFetcherForUrl(url);
+  if (fetcher)
+    return fetcher;
+
+  chrome::BitmapFetcher* new_fetcher = CreateFetcher(url);
+  active_fetchers_.push_back(new_fetcher);
+  return new_fetcher;
+}
+
+const chrome::BitmapFetcher* BitmapFetcherService::FindFetcherForUrl(
+    const GURL& url) {
+  for (BitmapFetchers::iterator iter = active_fetchers_.begin();
+       iter != active_fetchers_.end();
+       ++iter) {
+    if (url == (*iter)->url())
+      return *iter;
+  }
+  return NULL;
+}
+
+void BitmapFetcherService::RemoveFetcher(const chrome::BitmapFetcher* fetcher) {
+  for (BitmapFetchers::iterator iter = active_fetchers_.begin();
+       iter != active_fetchers_.end();
+       ++iter) {
+    if (fetcher == (*iter)) {
+      active_fetchers_.erase(iter);
+      return;
+    }
+  }
+  NOTREACHED();  // RemoveFetcher should always result in removal.
+}
+
+void BitmapFetcherService::OnFetchComplete(const GURL url,
+                                           const SkBitmap* bitmap) {
+  DCHECK(bitmap);  // can never be NULL, guaranteed by BitmapFetcher.
+
+  const chrome::BitmapFetcher* fetcher = FindFetcherForUrl(url);
+  DCHECK(fetcher);
+
+  // Notify all attached requests of completion.
+  ScopedVector<BitmapFetcherRequest>::iterator iter = requests_.begin();
+  while (iter != requests_.end()) {
+    if ((*iter)->get_fetcher() == fetcher) {
+      (*iter)->NotifyImageChanged(*bitmap);
+      iter = requests_.erase(iter);
+    } else {
+      ++iter;
+    }
+  }
+
+  if (!bitmap->isNull()) {
+    CacheEntry* entry = new CacheEntry;
+    entry->bitmap.reset(new SkBitmap(*bitmap));
+    cache_.Put(fetcher->url(), entry);
+  }
+
+  RemoveFetcher(fetcher);
+}

chrome/browser/bitmap_fetcher/bitmap_fetcher_service.h

+// Copyright 2014 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_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_H_
+#define CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_H_
+
+#include "base/compiler_specific.h"
+#include "base/containers/mru_cache.h"
+#include "base/containers/scoped_ptr_hash_map.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/bitmap_fetcher_delegate.h"
+#include "components/keyed_service/core/keyed_service.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace chrome {
+class BitmapFetcher;
+}  // namespace chrome
+
+class BitmapFetcherRequest;
+class GURL;
+class SkBitmap;
+
+// Service to retrieve images for Answers in Suggest.
+class BitmapFetcherService : public KeyedService,
+                             public chrome::BitmapFetcherDelegate {
+ public:
+  typedef int RequestId;
+  static const RequestId REQUEST_ID_INVALID = 0;
+
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Called whenever the image changes. Called with an empty image if the
+    // fetch failed or the request ended for any reason.
+    virtual void OnImageChanged(RequestId request_id,
+                                const SkBitmap& answers_image) = 0;
+  };
+
+  explicit BitmapFetcherService(content::BrowserContext* context);
+  virtual ~BitmapFetcherService();
+
+  // Cancels a request, if it is still in-flight.
+  void CancelRequest(RequestId requestId);
+
+  // Requests a new image. Will either trigger download or satisfy from cache.
+  // Takes ownership of |observer|. If there are too many outstanding requests,
+  // the request will fail and |observer| will be called to signal failure.
+  // Otherwise, |observer| will be called with either the cached image or the
+  // downloaded one.
+  // NOTE: The observer might be called back synchronously from RequestImage if
+  // the image is already in the cache.
+  RequestId RequestImage(const GURL& url, Observer* observer);
+
+  // Start fetching the image at the given |url|.
+  void Prefetch(const GURL& url);
+
+ protected:
+  // Create a bitmap fetcher for the given |url| and start it. Virtual method
+  // so tests can override this for different behavior.
+  virtual chrome::BitmapFetcher* CreateFetcher(const GURL& url);
+
+ private:
+  friend class BitmapFetcherServiceTest;
+
+  typedef ScopedVector<chrome::BitmapFetcher> BitmapFetchers;
+
+  // Gets the existing fetcher for |url| or constructs a new one if it doesn't
+  // exist.
+  const chrome::BitmapFetcher* EnsureFetcherForUrl(const GURL& url);
+
+  // Find a fetcher with a given |url|. Return NULL if none is found.
+  const chrome::BitmapFetcher* FindFetcherForUrl(const GURL& url);
+
+  // Remove |fetcher| from list of active fetchers. |fetcher| MUST be part of
+  // the list.
+  void RemoveFetcher(const chrome::BitmapFetcher* fetcher);
+
+  // BitmapFetcherDelegate implementation.
+  virtual void OnFetchComplete(const GURL url, const SkBitmap* bitmap) OVERRIDE;
+
+  // Currently active image fetchers.
+  BitmapFetchers active_fetchers_;
+
+  // Currently active requests.
+  ScopedVector<BitmapFetcherRequest> requests_;
+
+  // Cache of retrieved images.
+  struct CacheEntry {
+    CacheEntry();
+    ~CacheEntry();
+
+    scoped_ptr<const SkBitmap> bitmap;
+  };
+  base::OwningMRUCache<GURL, CacheEntry*> cache_;
+
+  // Current request ID to be used.
+  int current_request_id_;
+
+  // Browser context this service is active for.
+  content::BrowserContext* context_;
+
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcherService);
+};
+
+#endif  // CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_H_

chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.cc

+// Copyright 2014 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/bitmap_fetcher/bitmap_fetcher_service_factory.h"
+
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+
+/// Factory
+BitmapFetcherService* BitmapFetcherServiceFactory::GetForBrowserContext(
+    content::BrowserContext* profile) {
+  return static_cast<BitmapFetcherService*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+BitmapFetcherServiceFactory* BitmapFetcherServiceFactory::GetInstance() {
+  return Singleton<BitmapFetcherServiceFactory>::get();
+}
+
+BitmapFetcherServiceFactory::BitmapFetcherServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+          "BitmapFetcherService",
+          BrowserContextDependencyManager::GetInstance()) {
+}
+
+BitmapFetcherServiceFactory::~BitmapFetcherServiceFactory() {
+}
+
+KeyedService* BitmapFetcherServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = static_cast<Profile*>(context);
+  DCHECK(!profile->IsOffTheRecord());
+  return new BitmapFetcherService(profile);
+}

chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h

+// Copyright 2014 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_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_FACTORY_H_
+
+#include "base/memory/singleton.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class BitmapFetcherService;
+
+class BitmapFetcherServiceFactory : BrowserContextKeyedServiceFactory {
+ public:
+  // TODO(groby): Maybe make this GetForProfile?
+  static BitmapFetcherService* GetForBrowserContext(
+      content::BrowserContext* context);
+  static BitmapFetcherServiceFactory* GetInstance();
+
+ private:
+  friend struct DefaultSingletonTraits<BitmapFetcherServiceFactory>;
+
+  BitmapFetcherServiceFactory();
+  virtual ~BitmapFetcherServiceFactory();
+
+  // BrowserContextKeyedServiceFactory:
+  virtual KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const OVERRIDE;
+
+  DISALLOW_COPY_AND_ASSIGN(BitmapFetcherServiceFactory);
+};
+
+#endif  // CHROME_BROWSER_BITMAP_FETCHER_BITMAP_FETCHER_SERVICE_FACTORY_H_

chrome/browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc

+// Copyright 2014 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/bitmap_fetcher/bitmap_fetcher_service.h"
+
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
+#include "chrome/test/base/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class TestObserver : public BitmapFetcherService::Observer {
+ public:
+  explicit TestObserver(BitmapFetcherService::Observer* target)
+      : target_(target) {}
+  virtual ~TestObserver() {}
+
+  virtual void OnImageChanged(BitmapFetcherService::RequestId request_id,
+                              const SkBitmap& answers_image) OVERRIDE {
+    target_->OnImageChanged(request_id, answers_image);
+  }
+
+  BitmapFetcherService::Observer* target_;
+};
+
+class TestService : public BitmapFetcherService {
+ public:
+  explicit TestService(content::BrowserContext* context)
+      : BitmapFetcherService(context) {}
+  virtual ~TestService() {}
+
+  // Create a fetcher, but don't start downloading. That allows side-stepping
+  // the decode step, which requires a utility process.
+  virtual chrome::BitmapFetcher* CreateFetcher(const GURL& url) OVERRIDE {
+    return new chrome::BitmapFetcher(url, this);
+  }
+};
+
+}  // namespace
+
+class BitmapFetcherServiceTest : public testing::Test,
+                                 public BitmapFetcherService::Observer {
+ public:
+  virtual void SetUp() OVERRIDE {
+    service_.reset(new TestService(&profile_));
+    requestsFinished_ = 0;
+    imagesChanged_ = 0;
+    url1_ = GURL("http://example.org/sample-image-1.png");
+    url2_ = GURL("http://example.org/sample-image-2.png");
+  }
+
+  const ScopedVector<BitmapFetcherRequest>& requests() {
+    return service_->requests_;
+  }
+  const ScopedVector<chrome::BitmapFetcher>& active_fetchers() {
+    return service_->active_fetchers_;
+  }
+  size_t cache_size() { return service_->cache_.size(); }
+
+  virtual void OnImageChanged(BitmapFetcherService::RequestId request_id,
+                              const SkBitmap& answers_image) OVERRIDE {
+    if (answers_image.empty())
+      requestsFinished_++;
+    else
+      imagesChanged_++;
+  }
+
+  // Simulate finishing a URL fetch and decode for the given fetcher.
+  void CompleteFetch(const GURL& url) {
+    const chrome::BitmapFetcher* fetcher = service_->FindFetcherForUrl(url);
+    ASSERT_TRUE(NULL != fetcher);
+
+    // Create a non-empty bitmap.
+    SkBitmap image;
+    image.setConfig(SkBitmap::kARGB_8888_Config, 2, 2);
+    image.allocPixels();
+    image.eraseColor(SK_ColorGREEN);
+
+    const_cast<chrome::BitmapFetcher*>(fetcher)->OnImageDecoded(NULL, image);
+  }
+
+  void FailFetch(const GURL& url) {
+    const chrome::BitmapFetcher* fetcher = service_->FindFetcherForUrl(url);
+    ASSERT_TRUE(NULL != fetcher);
+    const_cast<chrome::BitmapFetcher*>(fetcher)
+        ->OnImageDecoded(NULL, SkBitmap());
+  }
+
+ protected:
+  scoped_ptr<BitmapFetcherService> service_;
+
+  int imagesChanged_;
+  int requestsFinished_;
+
+  GURL url1_;
+  GURL url2_;
+
+ private:
+  TestingProfile profile_;
+};
+
+TEST_F(BitmapFetcherServiceTest, CancelInvalidRequest) {
+  service_->CancelRequest(BitmapFetcherService::REQUEST_ID_INVALID);
+  service_->CancelRequest(23);
+}
+
+TEST_F(BitmapFetcherServiceTest, OnlyFirstRequestCreatesFetcher) {
+  EXPECT_EQ(0U, active_fetchers().size());
+
+  service_->RequestImage(url1_, new TestObserver(this));
+  EXPECT_EQ(1U, active_fetchers().size());
+
+  service_->RequestImage(url1_, new TestObserver(this));
+  EXPECT_EQ(1U, active_fetchers().size());
+}
+
+TEST_F(BitmapFetcherServiceTest, CompletedFetchNotifiesAllObservers) {
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  EXPECT_EQ(1U, active_fetchers().size());
+  EXPECT_EQ(4U, requests().size());
+
+  CompleteFetch(url1_);
+  EXPECT_EQ(4, imagesChanged_);
+  EXPECT_EQ(4, requestsFinished_);
+}
+
+TEST_F(BitmapFetcherServiceTest, CancelRequest) {
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  BitmapFetcherService::RequestId requestId =
+      service_->RequestImage(url2_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url1_, new TestObserver(this));
+  EXPECT_EQ(5U, requests().size());
+
+  service_->CancelRequest(requestId);
+  EXPECT_EQ(4U, requests().size());
+
+  CompleteFetch(url2_);
+  EXPECT_EQ(0, imagesChanged_);
+
+  CompleteFetch(url1_);
+  EXPECT_EQ(4, imagesChanged_);
+}
+
+TEST_F(BitmapFetcherServiceTest, FailedRequestsDontEnterCache) {
+  service_->RequestImage(url1_, new TestObserver(this));
+  service_->RequestImage(url2_, new TestObserver(this));
+  EXPECT_EQ(0U, cache_size());
+
+  CompleteFetch(url1_);
+  EXPECT_EQ(1U, cache_size());
+
+  FailFetch(url2_);
+  EXPECT_EQ(1U, cache_size());
+}

chrome/browser/bitmap_fetcher_browsertest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 
 #include "base/compiler_specific.h"
 #include "chrome/browser/profiles/profile.h"

chrome/browser/image_holder.h

@@ -6,7 +6,7 @@
 #define CHROME_BROWSER_IMAGE_HOLDER_H_
 
 #include "base/memory/scoped_vector.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"

chrome/browser/notifications/sync_notifier/synced_notification.h

@@ -14,7 +14,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "sync/api/sync_data.h"
 #include "sync/protocol/sync.pb.h"
 #include "ui/gfx/image/image.h"

chrome/browser/profiles/profile_avatar_downloader.h

@@ -5,7 +5,7 @@
 #define CHROME_BROWSER_PROFILES_PROFILE_AVATAR_DOWNLOADER_H_
 
 #include "base/files/file_path.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 
 class ProfileInfoCache;
 

chrome/browser/search/suggestions/thumbnail_manager.h

@@ -11,7 +11,7 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
-#include "chrome/browser/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 

chrome/chrome_browser.gypi

@@ -232,6 +232,10 @@
         'browser/autocomplete/autocomplete_classifier.h',
         'browser/autocomplete/autocomplete_classifier_factory.cc',
         'browser/autocomplete/autocomplete_classifier_factory.h',
+        'browser/bitmap_fetcher/bitmap_fetcher_service.cc',
+        'browser/bitmap_fetcher/bitmap_fetcher_service.h',
+        'browser/bitmap_fetcher/bitmap_fetcher_service_factory.cc',
+        'browser/bitmap_fetcher/bitmap_fetcher_service_factory.h',
         'browser/autocomplete/autocomplete_controller.cc',
         'browser/autocomplete/autocomplete_controller.h',
         'browser/autocomplete/autocomplete_controller_delegate.h',
@@ -298,8 +302,8 @@
         'browser/background/background_mode_manager_aura.cc',
         'browser/background/background_mode_manager_mac.mm',
         'browser/background/background_mode_manager_win.cc',
-        'browser/bitmap_fetcher.cc',
-        'browser/bitmap_fetcher.h',
+        'browser/bitmap_fetcher/bitmap_fetcher.cc',
+        'browser/bitmap_fetcher/bitmap_fetcher.h',
         'browser/bitmap_fetcher_delegate.h',
         'browser/bookmarks/bookmark_html_writer.cc',
         'browser/bookmarks/bookmark_html_writer.h',

chrome/chrome_tests_unit.gypi

@@ -620,6 +620,7 @@
         'browser/background/background_application_list_model_unittest.cc',
         'browser/background/background_contents_service_unittest.cc',
         'browser/background/background_mode_manager_unittest.cc',
+        'browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc',
         'browser/bookmarks/bookmark_html_writer_unittest.cc',
         'browser/bookmarks/chrome_bookmark_client_unittest.cc',
         'browser/browser_about_handler_unittest.cc',