[Fixed] [Suggestions] Move ImageManager to the component

One (1!) line fix to remove Valgrind issue. Original review https://codereview.chromium.org/543753002/

Keeps the fetcher functionality as ImageFetcherImpl in chrome/browser/search/suggestions.

BUG=387751
TBR=blundell
TEST=ImageManagerTest,ImageFetcherBrowserTest

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

Cr-Commit-Position: refs/heads/master@{#297177}

chrome/browser/search/suggestions/image_fetcher_impl.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/search/suggestions/image_fetcher_impl.h"
+
+#include <string>
+
+#include "content/public/browser/browser_thread.h"
+#include "net/base/load_flags.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace suggestions {
+
+ImageFetcherImpl::ImageFetcherImpl(
+    net::URLRequestContextGetter* url_request_context)
+    : url_request_context_(url_request_context) {}
+
+ImageFetcherImpl::~ImageFetcherImpl() {}
+
+ImageFetcherImpl::ImageRequest::ImageRequest() : fetcher(NULL) {}
+
+ImageFetcherImpl::ImageRequest::ImageRequest(chrome::BitmapFetcher* f)
+    : fetcher(f) {}
+
+ImageFetcherImpl::ImageRequest::~ImageRequest() { delete fetcher; }
+
+void ImageFetcherImpl::SetImageFetcherDelegate(ImageFetcherDelegate* delegate) {
+  DCHECK(delegate);
+  delegate_ = delegate;
+}
+
+void ImageFetcherImpl::StartOrQueueNetworkRequest(
+    const GURL& url, const GURL& image_url,
+    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+  // Before starting to fetch the image. Look for a request in progress for
+  // |image_url|, and queue if appropriate.
+  ImageRequestMap::iterator it = pending_net_requests_.find(image_url);
+  if (it == pending_net_requests_.end()) {
+    // |image_url| is not being fetched, so create a request and initiate
+    // the fetch.
+    ImageRequest request(new chrome::BitmapFetcher(image_url, this));
+    request.url = url;
+    request.callbacks.push_back(callback);
+    request.fetcher->Start(
+        url_request_context_, std::string(),
+        net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
+        net::LOAD_NORMAL);
+    pending_net_requests_[image_url].swap(&request);
+  } else {
+    // Request in progress. Register as an interested callback.
+    it->second.callbacks.push_back(callback);
+  }
+}
+
+void ImageFetcherImpl::OnFetchComplete(const GURL image_url,
+                                       const SkBitmap* bitmap) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  ImageRequestMap::iterator image_iter = pending_net_requests_.find(image_url);
+  DCHECK(image_iter != pending_net_requests_.end());
+
+  ImageRequest* request = &image_iter->second;
+
+  // Here |bitmap| could be NULL or a pointer to a bitmap which is owned by the
+  // BitmapFetcher and which ceases to exist after this function. Pass the
+  // un-owned pointer to the registered callbacks.
+  for (CallbackVector::iterator callback_iter = request->callbacks.begin();
+       callback_iter != request->callbacks.end(); ++callback_iter) {
+    callback_iter->Run(request->url, bitmap);
+  }
+
+  // Inform the ImageFetcherDelegate.
+  if (delegate_) {
+    delegate_->OnImageFetched(request->url, bitmap);
+  }
+
+  // Erase the completed ImageRequest.
+  pending_net_requests_.erase(image_iter);
+}
+
+}  // namespace suggestions

chrome/browser/search/suggestions/image_manager_impl.h → chrome/browser/search/suggestions/image_fetcher_impl.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_IMPL_H_
-#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_IMPL_H_
+#ifndef CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_
+#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_
 
 #include <map>
 #include <utility>
@@ -11,13 +11,8 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
-#include "base/containers/hash_tables.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
-#include "components/leveldb_proto/proto_database.h"
-#include "components/suggestions/image_manager.h"
-#include "components/suggestions/proto/suggestions.pb.h"
+#include "components/suggestions/image_fetcher.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
@@ -27,49 +22,32 @@ class URLRequestContextGetter;
 
 namespace suggestions {
 
-class ImageData;
-class SuggestionsProfile;
-
-// A class used to fetch server images asynchronously and manage the caching
-// layer (both in memory and on disk).
-class ImageManagerImpl : public ImageManager,
+// A class used to fetch server images.
+class ImageFetcherImpl : public ImageFetcher,
                          public chrome::BitmapFetcherDelegate {
  public:
-  typedef std::vector<ImageData> ImageDataVector;
-
-  ImageManagerImpl(
-      net::URLRequestContextGetter* url_request_context,
-      scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database,
-      const base::FilePath& database_dir);
-  virtual ~ImageManagerImpl();
-
-  // Overrides from ImageManager.
-  virtual void Initialize(const SuggestionsProfile& suggestions) OVERRIDE;
-  // Should be called from the UI thread.
-  virtual void GetImageForURL(
-      const GURL& url,
+  explicit ImageFetcherImpl(net::URLRequestContextGetter* url_request_context);
+  virtual ~ImageFetcherImpl();
+
+  virtual void SetImageFetcherDelegate(ImageFetcherDelegate* delegate) OVERRIDE;
+
+  virtual void StartOrQueueNetworkRequest(
+      const GURL& url, const GURL& image_url,
       base::Callback<void(const GURL&, const SkBitmap*)> callback) OVERRIDE;
 
  private:
-  friend class MockImageManagerImpl;
-  friend class ImageManagerImplBrowserTest;
-  FRIEND_TEST_ALL_PREFIXES(ImageManagerImplTest, InitializeTest);
-  FRIEND_TEST_ALL_PREFIXES(ImageManagerImplBrowserTest,
-                           GetImageForURLNetworkCacheHit);
-  FRIEND_TEST_ALL_PREFIXES(ImageManagerImplBrowserTest,
-                           GetImageForURLNetworkCacheNotInitialized);
-
-  // Used for testing.
-  ImageManagerImpl();
+  // Inherited from BitmapFetcherDelegate. Runs on the UI thread.
+  virtual void OnFetchComplete(const GURL image_url,
+                               const SkBitmap* bitmap) OVERRIDE;
 
   typedef std::vector<base::Callback<void(const GURL&, const SkBitmap*)> >
       CallbackVector;
-  typedef base::hash_map<std::string, SkBitmap> ImageMap;
 
   // State related to an image fetch (associated website url, image_url,
   // fetcher, pending callbacks).
   struct ImageRequest {
     ImageRequest();
+    // Struct takes ownership of |f|.
     explicit ImageRequest(chrome::BitmapFetcher* f);
     ~ImageRequest();
 
@@ -90,81 +68,17 @@ class ImageManagerImpl : public ImageManager,
 
   typedef std::map<const GURL, ImageRequest> ImageRequestMap;
 
-  // Looks up image URL for |url|. If found, writes the result to |image_url|
-  // and returns true. Otherwise just returns false.
-  bool GetImageURL(const GURL& url, GURL* image_url);
-
-  void QueueCacheRequest(
-      const GURL& url, const GURL& image_url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback);
-
-  void ServeFromCacheOrNetwork(
-      const GURL& url, const GURL& image_url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback);
-
-  // Will return false if no bitmap was found corresponding to |url|, else
-  // return true and call |callback| with the found bitmap.
-  bool ServeFromCache(
-      const GURL& url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback);
-
-  // Returns null if the |url| had no entry in the cache.
-  SkBitmap* GetBitmapFromCache(const GURL& url);
-
-  void StartOrQueueNetworkRequest(
-      const GURL& url, const GURL& image_url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback);
-
-  // Inherited from BitmapFetcherDelegate. Runs on the UI thread.
-  virtual void OnFetchComplete(const GURL image_url,
-                               const SkBitmap* bitmap) OVERRIDE;
-
-  // Save the image bitmap in the cache and in the database.
-  void SaveImage(const GURL& url, const SkBitmap& bitmap);
-
-  // Database callback methods.
-  // Will initiate loading the entries.
-  void OnDatabaseInit(bool success);
-  // Will transfer the loaded |entries| in memory (|image_map_|).
-  void OnDatabaseLoad(bool success, scoped_ptr<ImageDataVector> entries);
-  void OnDatabaseSave(bool success);
-
-  // Take entries from the database and put them in the local cache.
-  void LoadEntriesInCache(scoped_ptr<ImageDataVector> entries);
-
-  void ServePendingCacheRequests();
-
-  // From SkBitmap to the vector of JPEG-encoded bytes, |dst|. Visible only for
-  // testing.
-  static bool EncodeImage(const SkBitmap& bitmap,
-                          std::vector<unsigned char>* dest);
-
-  // Map from URL to image URL. Should be kept up to date when a new
-  // SuggestionsProfile is available.
-  std::map<GURL, GURL> image_url_map_;
-
   // Map from each image URL to the request information (associated website
   // url, fetcher, pending callbacks).
   ImageRequestMap pending_net_requests_;
 
-  // Map from website URL to request information, used for pending cache
-  // requests while the database hasn't loaded.
-  ImageRequestMap pending_cache_requests_;
-
-  // Holding the bitmaps in memory, keyed by website URL string.
-  ImageMap image_map_;
+  ImageFetcherDelegate* delegate_;
 
   net::URLRequestContextGetter* url_request_context_;
 
-  scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database_;
-
-  bool database_ready_;
-
-  base::WeakPtrFactory<ImageManagerImpl> weak_ptr_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageManagerImpl);
+  DISALLOW_COPY_AND_ASSIGN(ImageFetcherImpl);
 };
 
 }  // namespace suggestions
 
-#endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_IMPL_H_
+#endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_

chrome/browser/search/suggestions/image_fetcher_impl_browsertest.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/search/suggestions/image_fetcher_impl.h"
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/suggestions/image_fetcher_delegate.h"
+#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+class SkBitmap;
+
+namespace suggestions {
+
+namespace {
+
+const char kTestUrl[] = "http://go.com/";
+const char kTestImagePath[] = "files/image_decoding/droids.png";
+const char kInvalidImagePath[] = "files/DOESNOTEXIST";
+
+const base::FilePath::CharType kDocRoot[] =
+    FILE_PATH_LITERAL("chrome/test/data");
+
+class TestImageFetcherDelegate : public ImageFetcherDelegate {
+ public:
+  TestImageFetcherDelegate()
+    : num_delegate_valid_called_(0),
+      num_delegate_null_called_(0) {}
+  virtual ~TestImageFetcherDelegate() {};
+
+  // Perform additional tasks when an image has been fetched.
+  virtual void OnImageFetched(const GURL& url, const SkBitmap* bitmap)
+      OVERRIDE {
+    if (bitmap) {
+      num_delegate_valid_called_++;
+    } else {
+      num_delegate_null_called_++;
+    }
+  };
+
+  int num_delegate_valid_called() { return num_delegate_valid_called_; }
+  int num_delegate_null_called() { return num_delegate_null_called_; }
+
+ private:
+  int num_delegate_valid_called_;
+  int num_delegate_null_called_;
+};
+
+}  // end namespace
+
+class ImageFetcherImplBrowserTest : public InProcessBrowserTest {
+ protected:
+  ImageFetcherImplBrowserTest()
+    : num_callback_valid_called_(0),
+      num_callback_null_called_(0),
+      test_server_(net::SpawnedTestServer::TYPE_HTTP,
+                   net::SpawnedTestServer::kLocalhost,
+                   base::FilePath(kDocRoot)) {}
+
+  virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
+    ASSERT_TRUE(test_server_.Start());
+    InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+  }
+
+  virtual void TearDownInProcessBrowserTestFixture() OVERRIDE {
+    test_server_.Stop();
+  }
+
+  ImageFetcherImpl* CreateImageFetcher() {
+    ImageFetcherImpl* fetcher =
+        new ImageFetcherImpl(browser()->profile()->GetRequestContext());
+    fetcher->SetImageFetcherDelegate(&delegate_);
+    return fetcher;
+  }
+
+  void OnImageAvailable(base::RunLoop* loop,
+                        const GURL& url,
+                        const SkBitmap* bitmap) {
+    if (bitmap) {
+      num_callback_valid_called_++;
+    } else {
+      num_callback_null_called_++;
+    }
+    loop->Quit();
+  }
+
+  void StartOrQueueNetworkRequestHelper(const GURL& image_url) {
+    scoped_ptr<ImageFetcherImpl> image_fetcher_(CreateImageFetcher());
+
+    base::RunLoop run_loop;
+    image_fetcher_->StartOrQueueNetworkRequest(
+        GURL(kTestUrl),
+        image_url,
+        base::Bind(&ImageFetcherImplBrowserTest::OnImageAvailable,
+                   base::Unretained(this), &run_loop));
+    run_loop.Run();
+  }
+
+  int num_callback_valid_called_;
+  int num_callback_null_called_;
+
+  net::SpawnedTestServer test_server_;
+  TestImageFetcherDelegate delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageFetcherImplBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ImageFetcherImplBrowserTest, NormalFetch) {
+  GURL image_url(test_server_.GetURL(kTestImagePath).spec());
+  StartOrQueueNetworkRequestHelper(image_url);
+
+  EXPECT_EQ(1, num_callback_valid_called_);
+  EXPECT_EQ(0, num_callback_null_called_);
+  EXPECT_EQ(1, delegate_.num_delegate_valid_called());
+  EXPECT_EQ(0, delegate_.num_delegate_null_called());
+}
+
+IN_PROC_BROWSER_TEST_F(ImageFetcherImplBrowserTest, MultipleFetch) {
+  GURL image_url(test_server_.GetURL(kTestImagePath).spec());
+
+  for (int i = 0; i < 5; i++) {
+    StartOrQueueNetworkRequestHelper(image_url);
+  }
+
+  EXPECT_EQ(5, num_callback_valid_called_);
+  EXPECT_EQ(0, num_callback_null_called_);
+  EXPECT_EQ(5, delegate_.num_delegate_valid_called());
+  EXPECT_EQ(0, delegate_.num_delegate_null_called());
+}
+
+IN_PROC_BROWSER_TEST_F(ImageFetcherImplBrowserTest, InvalidFetch) {
+  GURL invalid_image_url(test_server_.GetURL(kInvalidImagePath).spec());
+  StartOrQueueNetworkRequestHelper(invalid_image_url);
+
+  EXPECT_EQ(0, num_callback_valid_called_);
+  EXPECT_EQ(1, num_callback_null_called_);
+  EXPECT_EQ(0, delegate_.num_delegate_valid_called());
+  EXPECT_EQ(1, delegate_.num_delegate_null_called());
+}
+
+}  // namespace suggestions

chrome/browser/search/suggestions/image_manager_impl_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 <string>
-
-#include "base/memory/scoped_ptr.h"
-#include "chrome/browser/search/suggestions/image_manager_impl.h"
-#include "chrome/test/base/testing_profile.h"
-#include "components/leveldb_proto/proto_database.h"
-#include "components/leveldb_proto/testing/fake_db.h"
-#include "components/suggestions/proto/suggestions.pb.h"
-#include "content/public/test/test_browser_thread_bundle.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "url/gurl.h"
-
-namespace {
-
-using leveldb_proto::test::FakeDB;
-using suggestions::ImageData;
-using suggestions::ImageManagerImpl;
-
-typedef base::hash_map<std::string, ImageData> EntryMap;
-
-const char kTestUrl[] = "http://go.com/";
-const char kTestImageUrl[] = "http://thumb.com/anchor_download_test.png";
-
-class ImageManagerImplTest : public testing::Test {
- protected:
-  ImageManagerImpl* CreateImageManager(Profile* profile) {
-    FakeDB<ImageData>* fake_db = new FakeDB<ImageData>(&db_model_);
-    return new ImageManagerImpl(
-        profile->GetRequestContext(),
-        scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> >(fake_db),
-        FakeDB<ImageData>::DirectoryForTestDB());
-  }
-
-  content::TestBrowserThreadBundle thread_bundle_;
-  EntryMap db_model_;
-};
-
-}  // namespace
-
-namespace suggestions {
-
-TEST_F(ImageManagerImplTest, InitializeTest) {
-  SuggestionsProfile suggestions_profile;
-  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
-  suggestion->set_url(kTestUrl);
-  suggestion->set_thumbnail(kTestImageUrl);
-
-  TestingProfile profile;
-  scoped_ptr<ImageManagerImpl> image_manager(
-      CreateImageManager(&profile));
-  image_manager->Initialize(suggestions_profile);
-
-  GURL output;
-  EXPECT_TRUE(image_manager->GetImageURL(GURL(kTestUrl), &output));
-  EXPECT_EQ(GURL(kTestImageUrl), output);
-
-  EXPECT_FALSE(image_manager->GetImageURL(GURL("http://b.com"), &output));
-}
-
-}  // namespace suggestions

chrome/browser/search/suggestions/suggestions_service_factory.cc

@@ -8,12 +8,13 @@
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/suggestions/image_manager_impl.h"
+#include "chrome/browser/search/suggestions/image_fetcher_impl.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/leveldb_proto/proto_database.h"
 #include "components/leveldb_proto/proto_database_impl.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/suggestions/blacklist_store.h"
+#include "components/suggestions/image_fetcher.h"
 #include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "components/suggestions/suggestions_service.h"
@@ -66,18 +67,19 @@ KeyedService* SuggestionsServiceFactory::BuildServiceInstanceFor(
       new BlacklistStore(the_profile->GetPrefs()));
 
   scoped_ptr<leveldb_proto::ProtoDatabaseImpl<ImageData> > db(
-      new leveldb_proto::ProtoDatabaseImpl<ImageData>(
-          background_task_runner));
+      new leveldb_proto::ProtoDatabaseImpl<ImageData>(background_task_runner));
 
   base::FilePath database_dir(
       the_profile->GetPath().Append(FILE_PATH_LITERAL("Thumbnails")));
 
-  scoped_ptr<ImageManagerImpl> thumbnail_manager(new ImageManagerImpl(
-      the_profile->GetRequestContext(),
+  scoped_ptr<ImageFetcherImpl> image_fetcher(
+      new ImageFetcherImpl(the_profile->GetRequestContext()));
+  scoped_ptr<ImageManager> thumbnail_manager(new ImageManager(
+      image_fetcher.PassAs<ImageFetcher>(),
       db.PassAs<leveldb_proto::ProtoDatabase<ImageData> >(), database_dir));
   return new SuggestionsService(
       the_profile->GetRequestContext(), suggestions_store.Pass(),
-      thumbnail_manager.PassAs<ImageManager>(), blacklist_store.Pass());
+      thumbnail_manager.Pass(), blacklist_store.Pass());
 }
 
 void SuggestionsServiceFactory::RegisterProfilePrefs(

chrome/chrome_browser.gypi

@@ -1072,8 +1072,8 @@
       'browser/searuch/most_visited_iframe_source.h',
       'browser/search/search.cc',
       'browser/search/search.h',
-      'browser/search/suggestions/image_manager_impl.cc',
-      'browser/search/suggestions/image_manager_impl.h',
+      'browser/search/suggestions/image_fetcher_impl.cc',
+      'browser/search/suggestions/image_fetcher_impl.h',
       'browser/search/suggestions/suggestions_service_factory.cc',
       'browser/search/suggestions/suggestions_service_factory.h',
       'browser/search/suggestions/suggestions_source.cc',

chrome/chrome_tests.gypi

@@ -1377,7 +1377,7 @@
         'browser/safe_browsing/safe_browsing_blocking_page_test.cc',
         'browser/safe_browsing/safe_browsing_service_browsertest.cc',
         'browser/safe_browsing/safe_browsing_test.cc',
-        'browser/search/suggestions/image_manager_impl_browsertest.cc',
+        'browser/search/suggestions/image_fetcher_impl_browsertest.cc',
         'browser/service_process/service_process_control_browsertest.cc',
         'browser/services/gcm/fake_gcm_profile_service.cc',
         'browser/services/gcm/fake_gcm_profile_service.h',

chrome/chrome_tests_unit.gypi

@@ -698,7 +698,6 @@
       'browser/search/most_visited_iframe_source_unittest.cc',
       'browser/search/search_android_unittest.cc',
       'browser/search/search_unittest.cc',
-      'browser/search/suggestions/image_manager_impl_unittest.cc',
       'browser/search_engines/default_search_pref_migration_unittest.cc',
       'browser/search_engines/search_provider_install_data_unittest.cc',
       'browser/search_engines/template_url_scraper_unittest.cc',

components/components_tests.gyp

@@ -219,6 +219,7 @@
             'storage_monitor/storage_monitor_unittest.cc',
             'storage_monitor/storage_monitor_win_unittest.cc',
             'suggestions/blacklist_store_unittest.cc',
+            'suggestions/image_manager_unittest.cc',
             'suggestions/suggestions_service_unittest.cc',
             'suggestions/suggestions_store_unittest.cc',
             'sync_driver/non_ui_data_type_controller_unittest.cc',

components/suggestions.gypi

@@ -19,10 +19,14 @@
         'components.gyp:keyed_service_core',
         'components.gyp:pref_registry',
         'components.gyp:variations',
+        'components.gyp:variations_http_provider',
       ],
       'sources': [
         'suggestions/blacklist_store.cc',
         'suggestions/blacklist_store.h',
+        'suggestions/image_fetcher.h',
+        'suggestions/image_fetcher_delegate.h',
+        'suggestions/image_manager.cc',
         'suggestions/image_manager.h',
         'suggestions/proto/suggestions.proto',
         'suggestions/suggestions_pref_names.cc',

components/suggestions/BUILD.gn

@@ -6,6 +6,9 @@ static_library("suggestions") {
   sources = [
     "blacklist_store.cc",
     "blacklist_store.h",
+    "image_fetcher.h",
+    "image_fetcher_delegate.h",
+    "image_manager.cc",
     "image_manager.h",
     "suggestions_pref_names.cc",
     "suggestions_pref_names.h",

components/suggestions/DEPS

 include_rules = [
   "+components/keyed_service/core",
+  "+components/leveldb_proto",
   "+components/pref_registry",
   "+components/variations",
   "+net",

components/suggestions/image_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 COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_H_
+#define COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_H_
+
+#include "base/callback.h"
+#include "components/suggestions/image_fetcher_delegate.h"
+#include "url/gurl.h"
+
+class SkBitmap;
+
+namespace suggestions {
+
+// A class used to fetch server images.
+class ImageFetcher {
+ public:
+  ImageFetcher() {}
+  virtual ~ImageFetcher() {}
+
+  virtual void SetImageFetcherDelegate(ImageFetcherDelegate* delegate) = 0;
+
+  virtual void StartOrQueueNetworkRequest(
+      const GURL& url, const GURL& image_url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ImageFetcher);
+};
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_H_

components/suggestions/image_fetcher_delegate.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 COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_DELEGATE_H_
+#define COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_DELEGATE_H_
+
+class GURL;
+class SkBitmap;
+
+namespace suggestions {
+
+class ImageFetcherDelegate {
+ public:
+  ImageFetcherDelegate() {}
+
+  // Called when an image was fetched. |url| represents the website for which
+  // the image was fetched. |bitmap| is deleted once out of scope.
+  virtual void OnImageFetched(const GURL& url, const SkBitmap* bitmap) = 0;
+
+ protected:
+  virtual ~ImageFetcherDelegate() {}
+
+  DISALLOW_COPY_AND_ASSIGN(ImageFetcherDelegate);
+};
+
+}  // namespace suggestions
+
+#endif  // COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_DELEGATE_H_

chrome/browser/search/suggestions/image_manager_impl.cc → components/suggestions/image_manager.cc

@@ -2,12 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/search/suggestions/image_manager_impl.h"
+#include "components/suggestions/image_manager.h"
 
-#include "base/memory/ref_counted_memory.h"
-#include "content/public/browser/browser_thread.h"
-#include "net/base/load_flags.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "base/bind.h"
+#include "components/suggestions/image_fetcher.h"
 #include "ui/gfx/codec/jpeg_codec.h"
 
 using leveldb_proto::ProtoDatabase;
@@ -23,30 +21,27 @@ SkBitmap* DecodeImage(const std::vector<unsigned char>& encoded_data) {
 
 namespace suggestions {
 
-ImageManagerImpl::ImageManagerImpl() : weak_ptr_factory_(this) {}
+ImageManager::ImageManager() : weak_ptr_factory_(this) {}
 
-ImageManagerImpl::ImageManagerImpl(
-    net::URLRequestContextGetter* url_request_context,
-    scoped_ptr<ProtoDatabase<ImageData> > database,
-    const base::FilePath& database_dir)
-    : url_request_context_(url_request_context),
+ImageManager::ImageManager(scoped_ptr<ImageFetcher> image_fetcher,
+                           scoped_ptr<ProtoDatabase<ImageData> > database,
+                           const base::FilePath& database_dir)
+    : image_fetcher_(image_fetcher.Pass()),
       database_(database.Pass()),
       database_ready_(false),
       weak_ptr_factory_(this) {
-  database_->Init(database_dir, base::Bind(&ImageManagerImpl::OnDatabaseInit,
+  image_fetcher_->SetImageFetcherDelegate(this);
+  database_->Init(database_dir, base::Bind(&ImageManager::OnDatabaseInit,
                                            weak_ptr_factory_.GetWeakPtr()));
 }
 
-ImageManagerImpl::~ImageManagerImpl() {}
+ImageManager::~ImageManager() {}
 
-ImageManagerImpl::ImageRequest::ImageRequest() : fetcher(NULL) {}
+ImageManager::ImageCacheRequest::ImageCacheRequest() {}
 
-ImageManagerImpl::ImageRequest::ImageRequest(chrome::BitmapFetcher* f)
-    : fetcher(f) {}
+ImageManager::ImageCacheRequest::~ImageCacheRequest() {}
 
-ImageManagerImpl::ImageRequest::~ImageRequest() { delete fetcher; }
-
-void ImageManagerImpl::Initialize(const SuggestionsProfile& suggestions) {
+void ImageManager::Initialize(const SuggestionsProfile& suggestions) {
   image_url_map_.clear();
   for (int i = 0; i < suggestions.suggestions_size(); ++i) {
     const ChromeSuggestion& suggestion = suggestions.suggestions(i);
@@ -56,10 +51,10 @@ void ImageManagerImpl::Initialize(const SuggestionsProfile& suggestions) {
   }
 }
 
-void ImageManagerImpl::GetImageForURL(
+void ImageManager::GetImageForURL(
     const GURL& url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  DCHECK(thread_checker_.CalledOnValidThread());
   // If |url| is not found in |image_url_map_|, then invoke |callback| with
   // NULL since there is no associated image for this |url|.
   GURL image_url;
@@ -79,40 +74,44 @@ void ImageManagerImpl::GetImageForURL(
   ServeFromCacheOrNetwork(url, image_url, callback);
 }
 
-bool ImageManagerImpl::GetImageURL(const GURL& url, GURL* image_url) {
+void ImageManager::OnImageFetched(const GURL& url, const SkBitmap* bitmap) {
+  SaveImage(url, *bitmap);
+}
+
+bool ImageManager::GetImageURL(const GURL& url, GURL* image_url) {
   std::map<GURL, GURL>::iterator it = image_url_map_.find(url);
   if (it == image_url_map_.end()) return false;  // Not found.
   *image_url = it->second;
   return true;
 }
 
-void ImageManagerImpl::QueueCacheRequest(
+void ImageManager::QueueCacheRequest(
     const GURL& url, const GURL& image_url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
   // To be served when the database has loaded.
-  ImageRequestMap::iterator it = pending_cache_requests_.find(url);
+  ImageCacheRequestMap::iterator it = pending_cache_requests_.find(url);
   if (it == pending_cache_requests_.end()) {
-    ImageRequest request(NULL);
+    ImageCacheRequest request;
     request.url = url;
     request.image_url = image_url;
     request.callbacks.push_back(callback);
-    pending_cache_requests_[url].swap(&request);
+    pending_cache_requests_[url] = request;
   } else {
     // Request already queued for this url.
     it->second.callbacks.push_back(callback);
   }
 }
 
-void ImageManagerImpl::ServeFromCacheOrNetwork(
+void ImageManager::ServeFromCacheOrNetwork(
     const GURL& url, const GURL& image_url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
   // If there is a image available in memory, return it.
   if (!ServeFromCache(url, callback)) {
-    StartOrQueueNetworkRequest(url, image_url, callback);
+    image_fetcher_->StartOrQueueNetworkRequest(url, image_url, callback);
   }
 }
 
-bool ImageManagerImpl::ServeFromCache(
+bool ImageManager::ServeFromCache(
     const GURL& url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
   SkBitmap* bitmap = GetBitmapFromCache(url);
@@ -123,7 +122,7 @@ bool ImageManagerImpl::ServeFromCache(
   return false;
 }
 
-SkBitmap* ImageManagerImpl::GetBitmapFromCache(const GURL& url) {
+SkBitmap* ImageManager::GetBitmapFromCache(const GURL& url) {
   ImageMap::iterator image_iter = image_map_.find(url.spec());
   if (image_iter != image_map_.end()) {
     return &image_iter->second;
@@ -131,56 +130,7 @@ SkBitmap* ImageManagerImpl::GetBitmapFromCache(const GURL& url) {
   return NULL;
 }
 
-void ImageManagerImpl::StartOrQueueNetworkRequest(
-    const GURL& url, const GURL& image_url,
-    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
-  // Before starting to fetch the image. Look for a request in progress for
-  // |image_url|, and queue if appropriate.
-  ImageRequestMap::iterator it = pending_net_requests_.find(image_url);
-  if (it == pending_net_requests_.end()) {
-    // |image_url| is not being fetched, so create a request and initiate
-    // the fetch.
-    ImageRequest request(new chrome::BitmapFetcher(image_url, this));
-    request.url = url;
-    request.callbacks.push_back(callback);
-    request.fetcher->Start(
-        url_request_context_, std::string(),
-        net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
-        net::LOAD_NORMAL);
-    pending_net_requests_[image_url].swap(&request);
-  } else {
-    // Request in progress. Register as an interested callback.
-    it->second.callbacks.push_back(callback);
-  }
-}
-
-void ImageManagerImpl::OnFetchComplete(const GURL image_url,
-                                       const SkBitmap* bitmap) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  ImageRequestMap::iterator image_iter = pending_net_requests_.find(image_url);
-  DCHECK(image_iter != pending_net_requests_.end());
-
-  ImageRequest* request = &image_iter->second;
-
-  // Here |bitmap| could be NULL or a pointer to a bitmap which is owned by the
-  // BitmapFetcher and which ceases to exist after this function. Pass the
-  // un-owned pointer to the registered callbacks.
-  for (CallbackVector::iterator callback_iter = request->callbacks.begin();
-       callback_iter != request->callbacks.end(); ++callback_iter) {
-    callback_iter->Run(request->url, bitmap);
-  }
-
-  // Save the bitmap to the in-memory model as well as the database, because it
-  // is now the freshest representation of the image.
-  // TODO(mathp): Handle null (no image found), possible deletion in DB.
-  if (bitmap) SaveImage(request->url, *bitmap);
-
-  // Erase the completed ImageRequest.
-  pending_net_requests_.erase(image_iter);
-}
-
-void ImageManagerImpl::SaveImage(const GURL& url, const SkBitmap& bitmap) {
+void ImageManager::SaveImage(const GURL& url, const SkBitmap& bitmap) {
   // Update the image map.
   image_map_.insert(std::make_pair(url.spec(), bitmap));
 
@@ -200,24 +150,24 @@ void ImageManagerImpl::SaveImage(const GURL& url, const SkBitmap& bitmap) {
         new std::vector<std::string>());
     entries_to_save->push_back(std::make_pair(data.url(), data));
     database_->UpdateEntries(entries_to_save.Pass(), keys_to_remove.Pass(),
-                             base::Bind(&ImageManagerImpl::OnDatabaseSave,
+                             base::Bind(&ImageManager::OnDatabaseSave,
                                         weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
-void ImageManagerImpl::OnDatabaseInit(bool success) {
+void ImageManager::OnDatabaseInit(bool success) {
   if (!success) {
     DVLOG(1) << "Image database init failed.";
     database_.reset();
     ServePendingCacheRequests();
     return;
   }
-  database_->LoadEntries(base::Bind(&ImageManagerImpl::OnDatabaseLoad,
+  database_->LoadEntries(base::Bind(&ImageManager::OnDatabaseLoad,
                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
-void ImageManagerImpl::OnDatabaseLoad(bool success,
-                                      scoped_ptr<ImageDataVector> entries) {
+void ImageManager::OnDatabaseLoad(bool success,
+                                  scoped_ptr<ImageDataVector> entries) {
   if (!success) {
     DVLOG(1) << "Image database load failed.";
     database_.reset();
@@ -230,7 +180,7 @@ void ImageManagerImpl::OnDatabaseLoad(bool success,
   ServePendingCacheRequests();
 }
 
-void ImageManagerImpl::OnDatabaseSave(bool success) {
+void ImageManager::OnDatabaseSave(bool success) {
   if (!success) {
     DVLOG(1) << "Image database save failed.";
     database_.reset();
@@ -238,7 +188,7 @@ void ImageManagerImpl::OnDatabaseSave(bool success) {
   }
 }
 
-void ImageManagerImpl::LoadEntriesInCache(scoped_ptr<ImageDataVector> entries) {
+void ImageManager::LoadEntriesInCache(scoped_ptr<ImageDataVector> entries) {
   for (ImageDataVector::iterator it = entries->begin(); it != entries->end();
        ++it) {
     std::vector<unsigned char> encoded_data(it->data().begin(),
@@ -251,10 +201,10 @@ void ImageManagerImpl::LoadEntriesInCache(scoped_ptr<ImageDataVector> entries) {
   }
 }
 
-void ImageManagerImpl::ServePendingCacheRequests() {
-  for (ImageRequestMap::iterator it = pending_cache_requests_.begin();
+void ImageManager::ServePendingCacheRequests() {
+  for (ImageCacheRequestMap::iterator it = pending_cache_requests_.begin();
        it != pending_cache_requests_.end(); ++it) {
-    const ImageRequest& request = it->second;
+    const ImageCacheRequest& request = it->second;
     for (CallbackVector::const_iterator callback_it = request.callbacks.begin();
          callback_it != request.callbacks.end(); ++callback_it) {
       ServeFromCacheOrNetwork(request.url, request.image_url, *callback_it);
@@ -263,8 +213,8 @@ void ImageManagerImpl::ServePendingCacheRequests() {
 }
 
 // static
-bool ImageManagerImpl::EncodeImage(const SkBitmap& bitmap,
-                                   std::vector<unsigned char>* dest) {
+bool ImageManager::EncodeImage(const SkBitmap& bitmap,
+                               std::vector<unsigned char>* dest) {
   SkAutoLockPixels bitmap_lock(bitmap);
   if (!bitmap.readyToDraw() || bitmap.isNull()) {
     return false;
@@ -272,7 +222,7 @@ bool ImageManagerImpl::EncodeImage(const SkBitmap& bitmap,
   return gfx::JPEGCodec::Encode(
       reinterpret_cast<unsigned char*>(bitmap.getAddr32(0, 0)),
       gfx::JPEGCodec::FORMAT_SkBitmap, bitmap.width(), bitmap.height(),
-      bitmap.width() * bitmap.bytesPerPixel(), 100, dest);
+      bitmap.rowBytes(), 100, dest);
 }
 
 }  // namespace suggestions

components/suggestions/image_manager.h

@@ -5,31 +5,146 @@
 #ifndef COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_
 #define COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_
 
+#include <map>
+#include <string>
+#include <vector>
+
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/containers/hash_tables.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "components/leveldb_proto/proto_database.h"
+#include "components/suggestions/image_fetcher_delegate.h"
 #include "components/suggestions/proto/suggestions.pb.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
+namespace net {
+class URLRequestContextGetter;
+}
+
 namespace suggestions {
 
-// An interface to retrieve images related to a specific URL.
-class ImageManager {
+class ImageData;
+class ImageFetcher;
+class SuggestionsProfile;
+
+// A class used to fetch server images asynchronously and manage the caching
+// layer (both in memory and on disk).
+class ImageManager : public ImageFetcherDelegate {
  public:
-  ImageManager() {}
-  virtual ~ImageManager() {}
+  typedef std::vector<ImageData> ImageDataVector;
+
+  ImageManager(scoped_ptr<ImageFetcher> image_fetcher,
+               scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database,
+               const base::FilePath& database_dir);
+  virtual ~ImageManager();
 
-  // (Re)Initializes states using data received from a SuggestionService. We're
-  // not doing this in the constructor because an instance may be long-lived.
-  virtual void Initialize(const SuggestionsProfile& suggestions) = 0;
+  virtual void Initialize(const SuggestionsProfile& suggestions);
 
-  // Retrieves stored image for website |url| asynchronously. Calls |callback|
-  // with Bitmap pointer if found, and NULL otherwise.
+  // Should be called from the UI thread.
   virtual void GetImageForURL(
       const GURL& url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback) = 0;
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
+ protected:
+  // Perform additional tasks when an image has been fetched.
+  virtual void OnImageFetched(const GURL& url, const SkBitmap* bitmap) OVERRIDE;
 
  private:
+  friend class MockImageManager;
+  friend class ImageManagerTest;
+  FRIEND_TEST_ALL_PREFIXES(ImageManagerTest, InitializeTest);
+  FRIEND_TEST_ALL_PREFIXES(ImageManagerTest, GetImageForURLNetworkCacheHit);
+  FRIEND_TEST_ALL_PREFIXES(ImageManagerTest,
+                           GetImageForURLNetworkCacheNotInitialized);
+
+  // Used for testing.
+  ImageManager();
+
+  typedef std::vector<base::Callback<void(const GURL&, const SkBitmap*)> >
+      CallbackVector;
+  typedef base::hash_map<std::string, SkBitmap> ImageMap;
+
+  // State related to an image fetch (associated website url, image_url,
+  // pending callbacks).
+  struct ImageCacheRequest {
+    ImageCacheRequest();
+    ~ImageCacheRequest();
+
+    GURL url;
+    GURL image_url;
+    // Queue for pending callbacks, which may accumulate while the request is in
+    // flight.
+    CallbackVector callbacks;
+  };
+
+  typedef std::map<const GURL, ImageCacheRequest> ImageCacheRequestMap;
+
+  // Looks up image URL for |url|. If found, writes the result to |image_url|
+  // and returns true. Otherwise just returns false.
+  bool GetImageURL(const GURL& url, GURL* image_url);
+
+  void QueueCacheRequest(
+      const GURL& url, const GURL& image_url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
+  void ServeFromCacheOrNetwork(
+      const GURL& url, const GURL& image_url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
+  // Will return false if no bitmap was found corresponding to |url|, else
+  // return true and call |callback| with the found bitmap.
+  bool ServeFromCache(
+      const GURL& url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
+  // Returns null if the |url| had no entry in the cache.
+  SkBitmap* GetBitmapFromCache(const GURL& url);
+
+  // Save the image bitmap in the cache and in the database.
+  void SaveImage(const GURL& url, const SkBitmap& bitmap);
+
+  // Database callback methods.
+  // Will initiate loading the entries.
+  void OnDatabaseInit(bool success);
+  // Will transfer the loaded |entries| in memory (|image_map_|).
+  void OnDatabaseLoad(bool success, scoped_ptr<ImageDataVector> entries);
+  void OnDatabaseSave(bool success);
+
+  // Take entries from the database and put them in the local cache.
+  void LoadEntriesInCache(scoped_ptr<ImageDataVector> entries);
+
+  void ServePendingCacheRequests();
+
+  // From SkBitmap to the vector of JPEG-encoded bytes, |dst|. Visible only for
+  // testing.
+  static bool EncodeImage(const SkBitmap& bitmap,
+                          std::vector<unsigned char>* dest);
+
+  // Map from URL to image URL. Should be kept up to date when a new
+  // SuggestionsProfile is available.
+  std::map<GURL, GURL> image_url_map_;
+
+  // Map from website URL to request information, used for pending cache
+  // requests while the database hasn't loaded.
+  ImageCacheRequestMap pending_cache_requests_;
+
+  // Holding the bitmaps in memory, keyed by website URL string.
+  ImageMap image_map_;
+
+  scoped_ptr<ImageFetcher> image_fetcher_;
+
+  scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> > database_;
+
+  bool database_ready_;
+
+  base::WeakPtrFactory<ImageManager> weak_ptr_factory_;
+
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(ImageManager);
 };
 

chrome/browser/search/suggestions/image_manager_impl_browsertest.cc → components/suggestions/image_manager_unittest.cc

@@ -6,106 +6,66 @@
 
 #include "base/files/file_path.h"
 #include "base/run_loop.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/search/suggestions/image_manager_impl.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/test/base/in_process_browser_test.h"
 #include "components/leveldb_proto/proto_database.h"
 #include "components/leveldb_proto/testing/fake_db.h"
+#include "components/suggestions/image_fetcher.h"
+#include "components/suggestions/image_fetcher_delegate.h"
+#include "components/suggestions/image_manager.h"
 #include "components/suggestions/proto/suggestions.pb.h"
-#include "content/public/test/test_utils.h"
-#include "net/base/load_flags.h"
-#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
+using ::testing::Return;
+using ::testing::StrictMock;
+using ::testing::_;
+
 namespace suggestions {
 
 const char kTestUrl1[] = "http://go.com/";
 const char kTestUrl2[] = "http://goal.com/";
-const char kTestBitmapUrl[] = "http://test.com";
 const char kTestImagePath[] = "files/image_decoding/droids.png";
 const char kInvalidImagePath[] = "files/DOESNOTEXIST";
 
-const base::FilePath::CharType kDocRoot[] =
-    FILE_PATH_LITERAL("chrome/test/data");
-
-using chrome::BitmapFetcher;
-using content::BrowserThread;
 using leveldb_proto::test::FakeDB;
 using suggestions::ImageData;
-using suggestions::ImageManagerImpl;
+using suggestions::ImageManager;
 
 typedef base::hash_map<std::string, ImageData> EntryMap;
 
 void AddEntry(const ImageData& d, EntryMap* map) { (*map)[d.url()] = d; }
 
-class ImageManagerImplBrowserTest : public InProcessBrowserTest {
+class MockImageFetcher : public suggestions::ImageFetcher {
  public:
-  ImageManagerImplBrowserTest()
-      : num_callback_null_called_(0),
-        num_callback_valid_called_(0),
-        test_server_(net::SpawnedTestServer::TYPE_HTTP,
-                     net::SpawnedTestServer::kLocalhost,
-                     base::FilePath(kDocRoot)) {}
-
-  virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
-    ASSERT_TRUE(test_server_.Start());
-    InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
-  }
+  MockImageFetcher() {}
+  virtual ~MockImageFetcher() {}
+  MOCK_METHOD3(StartOrQueueNetworkRequest,
+               void(const GURL&, const GURL&,
+                    base::Callback<void(const GURL&, const SkBitmap*)>));
+  MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
+};
 
-  virtual void TearDownInProcessBrowserTestFixture() OVERRIDE {
-    test_server_.Stop();
-  }
+class ImageManagerTest : public testing::Test {
+ public:
+  ImageManagerTest()
+      : mock_image_fetcher_(NULL),
+        num_callback_null_called_(0),
+        num_callback_valid_called_(0) {}
 
-  virtual void SetUpOnMainThread() OVERRIDE {
+  virtual void SetUp() OVERRIDE {
     fake_db_ = new FakeDB<ImageData>(&db_model_);
-    image_manager_.reset(CreateImageManagerImpl(fake_db_));
+    image_manager_.reset(CreateImageManager(fake_db_));
   }
 
-  virtual void TearDownOnMainThread() OVERRIDE {
+  virtual void TearDown() OVERRIDE {
     fake_db_ = NULL;
     db_model_.clear();
     image_manager_.reset();
-    test_image_manager_.reset();
   }
 
-  void InitializeTestBitmapData() {
-    FakeDB<ImageData>* test_fake_db = new FakeDB<ImageData>(&db_model_);
-    test_image_manager_.reset(CreateImageManagerImpl(test_fake_db));
-
-    suggestions::SuggestionsProfile suggestions_profile;
-    suggestions::ChromeSuggestion* suggestion =
-        suggestions_profile.add_suggestions();
-    suggestion->set_url(kTestBitmapUrl);
-    suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
-
-    test_image_manager_->Initialize(suggestions_profile);
-
-    // Initialize empty database.
-    test_fake_db->InitCallback(true);
-    test_fake_db->LoadCallback(true);
-
-    base::RunLoop run_loop;
-    // Fetch existing URL.
-    test_image_manager_->GetImageForURL(
-        GURL(kTestBitmapUrl),
-        base::Bind(&ImageManagerImplBrowserTest::OnTestImageAvailable,
-                   base::Unretained(this), &run_loop));
-    run_loop.Run();
-  }
-
-  void OnTestImageAvailable(base::RunLoop* loop, const GURL& url,
-                            const SkBitmap* bitmap) {
-    CHECK(bitmap);
-    // Copy the resource locally.
-    test_bitmap_ = *bitmap;
-    loop->Quit();
-  }
-
-  void InitializeDefaultImageMapAndDatabase(
-      ImageManagerImpl* image_manager, FakeDB<ImageData>* fake_db) {
+  void InitializeDefaultImageMapAndDatabase(ImageManager* image_manager,
+                                            FakeDB<ImageData>* fake_db) {
     CHECK(image_manager);
     CHECK(fake_db);
 
@@ -113,7 +73,7 @@ class ImageManagerImplBrowserTest : public InProcessBrowserTest {
     suggestions::ChromeSuggestion* suggestion =
         suggestions_profile.add_suggestions();
     suggestion->set_url(kTestUrl1);
-    suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
+    suggestion->set_thumbnail(kTestImagePath);
 
     image_manager->Initialize(suggestions_profile);
 
@@ -123,139 +83,99 @@ class ImageManagerImplBrowserTest : public InProcessBrowserTest {
   }
 
   ImageData GetSampleImageData(const std::string& url) {
+    // Create test bitmap.
+    SkBitmap bm;
+    // Being careful with the Bitmap. There are memory-related issue in
+    // crbug.com/101781.
+    bm.allocN32Pixels(4, 4);
+    bm.eraseColor(SK_ColorRED);
     ImageData data;
     data.set_url(url);
     std::vector<unsigned char> encoded;
-    EXPECT_TRUE(ImageManagerImpl::EncodeImage(test_bitmap_, &encoded));
+    EXPECT_TRUE(ImageManager::EncodeImage(bm, &encoded));
     data.set_data(std::string(encoded.begin(), encoded.end()));
     return data;
   }
 
   void OnImageAvailable(base::RunLoop* loop, const GURL& url,
-                            const SkBitmap* bitmap) {
+                        const SkBitmap* bitmap) {
     if (bitmap) {
       num_callback_valid_called_++;
-      std::vector<unsigned char> actual;
-      std::vector<unsigned char> expected;
-      EXPECT_TRUE(ImageManagerImpl::EncodeImage(*bitmap, &actual));
-      EXPECT_TRUE(ImageManagerImpl::EncodeImage(test_bitmap_, &expected));
-      // Check first 100 bytes.
-      std::string actual_str(actual.begin(), actual.begin() + 100);
-      std::string expected_str(expected.begin(), expected.begin() + 100);
-      EXPECT_EQ(expected_str, actual_str);
     } else {
       num_callback_null_called_++;
     }
     loop->Quit();
   }
 
-  ImageManagerImpl* CreateImageManagerImpl(FakeDB<ImageData>* fake_db) {
-    return new ImageManagerImpl(
-        browser()->profile()->GetRequestContext(),
+  ImageManager* CreateImageManager(FakeDB<ImageData>* fake_db) {
+    mock_image_fetcher_ = new StrictMock<MockImageFetcher>();
+    EXPECT_CALL(*mock_image_fetcher_, SetImageFetcherDelegate(_));
+    return new ImageManager(
+        scoped_ptr<ImageFetcher>(mock_image_fetcher_),
         scoped_ptr<leveldb_proto::ProtoDatabase<ImageData> >(fake_db),
         FakeDB<ImageData>::DirectoryForTestDB());
   }
 
   EntryMap db_model_;
-  // Owned by the ImageManagerImpl under test.
+  // Owned by the ImageManager under test.
   FakeDB<ImageData>* fake_db_;
 
-  SkBitmap test_bitmap_;
-  scoped_ptr<ImageManagerImpl> test_image_manager_;
+  MockImageFetcher* mock_image_fetcher_;
 
   int num_callback_null_called_;
   int num_callback_valid_called_;
-  net::SpawnedTestServer test_server_;
   // Under test.
-  scoped_ptr<ImageManagerImpl> image_manager_;
+  scoped_ptr<ImageManager> image_manager_;
 };
 
-IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest, GetImageForURLNetwork) {
-  InitializeTestBitmapData();
-  InitializeDefaultImageMapAndDatabase(image_manager_.get(), fake_db_);
-
-  base::RunLoop run_loop;
-  // Fetch existing URL.
-  image_manager_->GetImageForURL(
-      GURL(kTestUrl1),
-      base::Bind(&ImageManagerImplBrowserTest::OnImageAvailable,
-                 base::Unretained(this), &run_loop));
-  run_loop.Run();
+TEST_F(ImageManagerTest, InitializeTest) {
+  SuggestionsProfile suggestions_profile;
+  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
+  suggestion->set_url(kTestUrl1);
+  suggestion->set_thumbnail(kTestImagePath);
 
-  EXPECT_EQ(0, num_callback_null_called_);
-  EXPECT_EQ(1, num_callback_valid_called_);
+  image_manager_->Initialize(suggestions_profile);
 
-  base::RunLoop run_loop2;
-  // Fetch non-existing URL.
-  image_manager_->GetImageForURL(
-      GURL(kTestUrl2),
-      base::Bind(&ImageManagerImplBrowserTest::OnImageAvailable,
-                 base::Unretained(this), &run_loop2));
-  run_loop2.Run();
+  GURL output;
+  EXPECT_TRUE(image_manager_->GetImageURL(GURL(kTestUrl1), &output));
+  EXPECT_EQ(GURL(kTestImagePath), output);
 
-  EXPECT_EQ(1, num_callback_null_called_);
-  EXPECT_EQ(1, num_callback_valid_called_);
+  EXPECT_FALSE(image_manager_->GetImageURL(GURL("http://b.com"), &output));
 }
 
-IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest,
-                       GetImageForURLNetworkMultiple) {
-  InitializeTestBitmapData();
+TEST_F(ImageManagerTest, GetImageForURLNetwork) {
   InitializeDefaultImageMapAndDatabase(image_manager_.get(), fake_db_);
 
-  // Fetch non-existing URL, and add more while request is in flight.
-  base::RunLoop run_loop;
-  for (int i = 0; i < 5; i++) {
-    // Fetch existing URL.
-    image_manager_->GetImageForURL(
-        GURL(kTestUrl1),
-        base::Bind(&ImageManagerImplBrowserTest::OnImageAvailable,
-                   base::Unretained(this), &run_loop));
-  }
-  run_loop.Run();
-
-  EXPECT_EQ(0, num_callback_null_called_);
-  EXPECT_EQ(5, num_callback_valid_called_);
-}
-
-IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest,
-                       GetImageForURLNetworkInvalid) {
-  SuggestionsProfile suggestions_profile;
-  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
-  suggestion->set_url(kTestUrl1);
-  suggestion->set_thumbnail(test_server_.GetURL(kInvalidImagePath).spec());
-
-  image_manager_->Initialize(suggestions_profile);
-
-  // Database will be initialized and loaded without anything in it.
-  fake_db_->InitCallback(true);
-  fake_db_->LoadCallback(true);
+  // We expect the fetcher to go to network and call the callback.
+  EXPECT_CALL(*mock_image_fetcher_, StartOrQueueNetworkRequest(_, _, _));
 
+  // Fetch existing URL.
   base::RunLoop run_loop;
-  // Fetch existing URL that has invalid image.
-  image_manager_->GetImageForURL(
-      GURL(kTestUrl1),
-      base::Bind(&ImageManagerImplBrowserTest::OnImageAvailable,
-                 base::Unretained(this), &run_loop));
+  image_manager_->GetImageForURL(GURL(kTestUrl1),
+                                 base::Bind(&ImageManagerTest::OnImageAvailable,
+                                            base::Unretained(this), &run_loop));
+
+  // Will not go to network and use the fetcher since URL is invalid.
+  // Fetch non-existing URL.
+  image_manager_->GetImageForURL(GURL(kTestUrl2),
+                                 base::Bind(&ImageManagerTest::OnImageAvailable,
+                                            base::Unretained(this), &run_loop));
   run_loop.Run();
 
   EXPECT_EQ(1, num_callback_null_called_);
-  EXPECT_EQ(0, num_callback_valid_called_);
 }
 
-IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest,
-                       GetImageForURLNetworkCacheHit) {
-  InitializeTestBitmapData();
-
+TEST_F(ImageManagerTest, GetImageForURLNetworkCacheHit) {
   SuggestionsProfile suggestions_profile;
   ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
   suggestion->set_url(kTestUrl1);
   // The URL we set is invalid, to show that it will fail from network.
-  suggestion->set_thumbnail(test_server_.GetURL(kInvalidImagePath).spec());
+  suggestion->set_thumbnail(kInvalidImagePath);
 
-  // Create the ImageManagerImpl with an added entry in the database.
+  // Create the ImageManager with an added entry in the database.
   AddEntry(GetSampleImageData(kTestUrl1), &db_model_);
   FakeDB<ImageData>* fake_db = new FakeDB<ImageData>(&db_model_);
-  image_manager_.reset(CreateImageManagerImpl(fake_db));
+  image_manager_.reset(CreateImageManager(fake_db));
   image_manager_->Initialize(suggestions_profile);
   fake_db->InitCallback(true);
   fake_db->LoadCallback(true);
@@ -264,10 +184,9 @@ IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest,
   EXPECT_FALSE(bitmap->isNull());
 
   base::RunLoop run_loop;
-  image_manager_->GetImageForURL(
-      GURL(kTestUrl1),
-      base::Bind(&ImageManagerImplBrowserTest::OnImageAvailable,
-                 base::Unretained(this), &run_loop));
+  image_manager_->GetImageForURL(GURL(kTestUrl1),
+                                 base::Bind(&ImageManagerTest::OnImageAvailable,
+                                            base::Unretained(this), &run_loop));
   run_loop.Run();
 
   EXPECT_EQ(0, num_callback_null_called_);