[Suggestions] Make ThumbnailManager implement ImageManager

In a componentized world, ThumbnailManager will stay in Chrome and ImageManager will be the interface on which SuggestionsService will depend on.

BUG=387751
TBR=jam
TEST=Suggestions*,ThumbnailManager*

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

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

chrome/browser/search/suggestions/image_manager.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_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_
+#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
+#include "ui/gfx/image/image_skia.h"
+#include "url/gurl.h"
+
+namespace suggestions {
+
+// An interface to retrieve images related to a specific URL.
+class ImageManager {
+ public:
+  ImageManager() {}
+  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;
+
+  // Retrieves stored image for website |url| asynchronously. Calls |callback|
+  // with Bitmap pointer if found, and NULL otherwise.
+  virtual void GetImageForURL(
+      const GURL& url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ImageManager);
+};
+
+}  // namespace suggestions
+
+#endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_H_

chrome/browser/search/suggestions/suggestions_service.cc

@@ -119,7 +119,7 @@ std::string GetBlacklistUrlPrefix() {
 SuggestionsService::SuggestionsService(
     net::URLRequestContextGetter* url_request_context,
     scoped_ptr<SuggestionsStore> suggestions_store,
-    scoped_ptr<ThumbnailManager> thumbnail_manager,
+    scoped_ptr<ImageManager> thumbnail_manager,
     scoped_ptr<BlacklistStore> blacklist_store)
     : suggestions_store_(suggestions_store.Pass()),
       blacklist_store_(blacklist_store.Pass()),
@@ -187,7 +187,7 @@ void SuggestionsService::FetchSuggestionsDataNoTimeout(
 void SuggestionsService::GetPageThumbnail(
     const GURL& url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
-  thumbnail_manager_->GetPageThumbnail(url, callback);
+  thumbnail_manager_->GetImageForURL(url, callback);
 }
 
 void SuggestionsService::BlacklistURL(
@@ -315,7 +315,7 @@ void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
     suggestions_store_->ClearSuggestions();
   } else if (suggestions.ParseFromString(suggestions_data)) {
     LogResponseState(RESPONSE_VALID);
-    thumbnail_manager_->InitializeThumbnailMap(suggestions);
+    thumbnail_manager_->Initialize(suggestions);
     suggestions_store_->StoreSuggestions(suggestions);
   } else {
     LogResponseState(RESPONSE_INVALID);

chrome/browser/search/suggestions/suggestions_service.h

@@ -15,8 +15,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
+#include "chrome/browser/search/suggestions/image_manager.h"
 #include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
-#include "chrome/browser/search/suggestions/thumbnail_manager.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "ui/gfx/image/image_skia.h"
@@ -51,7 +51,7 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
 
   SuggestionsService(net::URLRequestContextGetter* url_request_context,
                      scoped_ptr<SuggestionsStore> suggestions_store,
-                     scoped_ptr<ThumbnailManager> thumbnail_manager,
+                     scoped_ptr<ImageManager> thumbnail_manager,
                      scoped_ptr<BlacklistStore> blacklist_store);
   virtual ~SuggestionsService();
 
@@ -163,7 +163,7 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   std::vector<ResponseCallback> waiting_requestors_;
 
   // Used to obtain server thumbnails, if available.
-  scoped_ptr<ThumbnailManager> thumbnail_manager_;
+  scoped_ptr<ImageManager> thumbnail_manager_;
 
   net::URLRequestContextGetter* url_request_context_;
 

chrome/browser/search/suggestions/suggestions_service_factory.cc

@@ -4,13 +4,16 @@
 
 #include "chrome/browser/search/suggestions/suggestions_service_factory.h"
 
+#include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/suggestions/blacklist_store.h"
+#include "chrome/browser/search/suggestions/image_manager.h"
 #include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
 #include "chrome/browser/search/suggestions/suggestions_service.h"
 #include "chrome/browser/search/suggestions/suggestions_store.h"
+#include "chrome/browser/search/suggestions/thumbnail_manager.h"
 #include "chrome/common/pref_names.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/leveldb_proto/proto_database.h"
@@ -73,7 +76,7 @@ KeyedService* SuggestionsServiceFactory::BuildServiceInstanceFor(
       db.PassAs<leveldb_proto::ProtoDatabase<ThumbnailData> >(), database_dir));
   return new SuggestionsService(
       the_profile->GetRequestContext(), suggestions_store.Pass(),
-      thumbnail_manager.Pass(), blacklist_store.Pass());
+      thumbnail_manager.PassAs<ImageManager>(), blacklist_store.Pass());
 }
 
 void SuggestionsServiceFactory::RegisterProfilePrefs(

chrome/browser/search/suggestions/suggestions_service_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/search/suggestions/blacklist_store.h"
+#include "chrome/browser/search/suggestions/image_manager.h"
 #include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
 #include "chrome/browser/search/suggestions/suggestions_service_factory.h"
 #include "chrome/browser/search/suggestions/suggestions_store.h"
@@ -32,6 +33,7 @@ using testing::DoAll;
 using ::testing::Eq;
 using ::testing::Return;
 using testing::SetArgPointee;
+using ::testing::NiceMock;
 using ::testing::StrictMock;
 using ::testing::_;
 
@@ -100,12 +102,12 @@ class MockSuggestionsStore : public suggestions::SuggestionsStore {
   MOCK_METHOD0(ClearSuggestions, void());
 };
 
-class MockThumbnailManager : public suggestions::ThumbnailManager {
+class MockImageManager : public suggestions::ImageManager {
  public:
-  MockThumbnailManager() {}
-  virtual ~MockThumbnailManager() {}
-  MOCK_METHOD1(InitializeThumbnailMap, void(const SuggestionsProfile&));
-  MOCK_METHOD2(GetPageThumbnail,
+  MockImageManager() {}
+  virtual ~MockImageManager() {}
+  MOCK_METHOD1(Initialize, void(const SuggestionsProfile&));
+  MOCK_METHOD2(GetImageForURL,
                void(const GURL&,
                     base::Callback<void(const GURL&, const SkBitmap*)>));
 };
@@ -184,11 +186,11 @@ class SuggestionsServiceTest : public testing::Test {
   // SuggestionsStore in |mock_suggestions_store_|.
   SuggestionsService* CreateSuggestionsServiceWithMocks() {
     mock_suggestions_store_ = new StrictMock<MockSuggestionsStore>();
-    mock_thumbnail_manager_ = new StrictMock<MockThumbnailManager>();
+    mock_thumbnail_manager_ = new NiceMock<MockImageManager>();
     mock_blacklist_store_ = new MockBlacklistStore();
     return new SuggestionsService(
         request_context_, scoped_ptr<SuggestionsStore>(mock_suggestions_store_),
-        scoped_ptr<ThumbnailManager>(mock_thumbnail_manager_),
+        scoped_ptr<ImageManager>(mock_thumbnail_manager_),
         scoped_ptr<BlacklistStore>(mock_blacklist_store_));
   }
 
@@ -248,7 +250,7 @@ class SuggestionsServiceTest : public testing::Test {
   net::FakeURLFetcherFactory factory_;
   // Only used if the SuggestionsService is built with mocks. Not owned.
   MockSuggestionsStore* mock_suggestions_store_;
-  MockThumbnailManager* mock_thumbnail_manager_;
+  MockImageManager* mock_thumbnail_manager_;
   MockBlacklistStore* mock_blacklist_store_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_;
 

chrome/browser/search/suggestions/thumbnail_manager.cc

@@ -18,7 +18,8 @@ namespace {
 SkBitmap* DecodeThumbnail(const std::vector<unsigned char>& encoded_data) {
   return gfx::JPEGCodec::Decode(&encoded_data[0], encoded_data.size());
 }
-}
+
+}  // namespace
 
 namespace suggestions {
 
@@ -45,8 +46,7 @@ ThumbnailManager::ThumbnailRequest::ThumbnailRequest(chrome::BitmapFetcher* f)
 
 ThumbnailManager::ThumbnailRequest::~ThumbnailRequest() { delete fetcher; }
 
-void ThumbnailManager::InitializeThumbnailMap(
-    const SuggestionsProfile& suggestions) {
+void ThumbnailManager::Initialize(const SuggestionsProfile& suggestions) {
   thumbnail_url_map_.clear();
   for (int i = 0; i < suggestions.suggestions_size(); ++i) {
     const ChromeSuggestion& suggestion = suggestions.suggestions(i);
@@ -56,7 +56,7 @@ void ThumbnailManager::InitializeThumbnailMap(
   }
 }
 
-void ThumbnailManager::GetPageThumbnail(
+void ThumbnailManager::GetImageForURL(
     const GURL& url,
     base::Callback<void(const GURL&, const SkBitmap*)> callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
@@ -68,7 +68,7 @@ void ThumbnailManager::GetPageThumbnail(
     return;
   }
 
-  // |database_| can be null if something went wrong in initialization.
+  // |database_| can be NULL if something went wrong in initialization.
   if (database_.get() && !database_ready_) {
     // Once database is initialized, it will serve pending requests from either
     // cache or network.

chrome/browser/search/suggestions/thumbnail_manager.h

@@ -15,6 +15,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
+#include "chrome/browser/search/suggestions/image_manager.h"
 #include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
 #include "components/leveldb_proto/proto_database.h"
 #include "ui/gfx/image/image_skia.h"
@@ -31,7 +32,8 @@ class ThumbnailData;
 
 // A class used to fetch server thumbnails asynchronously and manage the caching
 // layer (both in memory and on disk).
-class ThumbnailManager : public chrome::BitmapFetcherDelegate {
+class ThumbnailManager : public ImageManager,
+                         public chrome::BitmapFetcherDelegate {
  public:
   typedef std::vector<ThumbnailData> ThumbnailVector;
 
@@ -41,25 +43,21 @@ class ThumbnailManager : public chrome::BitmapFetcherDelegate {
       const base::FilePath& database_dir);
   virtual ~ThumbnailManager();
 
-  // Initializes the |thumbnail_url_map_| with the proper mapping from website
-  // URL to thumbnail URL.
-  void InitializeThumbnailMap(const SuggestionsProfile& suggestions);
-
-  // Retrieves stored thumbnail for website |url| asynchronously. Calls
-  // |callback| with Bitmap pointer if found, and NULL otherwise. Should be
-  // called from the UI thread.
-  void GetPageThumbnail(
+  // Overrides from ImageManager.
+  virtual void Initialize(const SuggestionsProfile& suggestions) OVERRIDE;
+  // Should be called from the UI thread.
+  virtual void GetImageForURL(
       const GURL& url,
-      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+      base::Callback<void(const GURL&, const SkBitmap*)> callback) OVERRIDE;
 
  private:
   friend class MockThumbnailManager;
   friend class ThumbnailManagerBrowserTest;
-  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerTest, InitializeThumbnailMapTest);
+  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerTest, InitializeTest);
   FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerBrowserTest,
-                           GetPageThumbnailNetworkCacheHit);
+                           GetImageForURLNetworkCacheHit);
   FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerBrowserTest,
-                           GetPageThumbnailNetworkCacheNotInitialized);
+                           GetImageForURLNetworkCacheNotInitialized);
 
   // Used for testing.
   ThumbnailManager();

chrome/browser/search/suggestions/thumbnail_manager_browsertest.cc

@@ -81,7 +81,7 @@ class ThumbnailManagerBrowserTest : public InProcessBrowserTest {
     suggestion->set_url(kTestBitmapUrl);
     suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
 
-    test_thumbnail_manager_->InitializeThumbnailMap(suggestions_profile);
+    test_thumbnail_manager_->Initialize(suggestions_profile);
 
     // Initialize empty database.
     test_fake_db->InitCallback(true);
@@ -89,7 +89,7 @@ class ThumbnailManagerBrowserTest : public InProcessBrowserTest {
 
     base::RunLoop run_loop;
     // Fetch existing URL.
-    test_thumbnail_manager_->GetPageThumbnail(
+    test_thumbnail_manager_->GetImageForURL(
         GURL(kTestBitmapUrl),
         base::Bind(&ThumbnailManagerBrowserTest::OnTestThumbnailAvailable,
                    base::Unretained(this), &run_loop));
@@ -115,7 +115,7 @@ class ThumbnailManagerBrowserTest : public InProcessBrowserTest {
     suggestion->set_url(kTestUrl1);
     suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
 
-    thumbnail_manager->InitializeThumbnailMap(suggestions_profile);
+    thumbnail_manager->Initialize(suggestions_profile);
 
     // Initialize empty database.
     fake_db->InitCallback(true);
@@ -170,12 +170,12 @@ class ThumbnailManagerBrowserTest : public InProcessBrowserTest {
   scoped_ptr<ThumbnailManager> thumbnail_manager_;
 };
 
-IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, GetPageThumbnailNetwork) {
+IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, GetImageForURLNetwork) {
   InitializeDefaultThumbnailMapAndDatabase(thumbnail_manager_.get(), fake_db_);
 
   base::RunLoop run_loop;
   // Fetch existing URL.
-  thumbnail_manager_->GetPageThumbnail(
+  thumbnail_manager_->GetImageForURL(
       GURL(kTestUrl1),
       base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
                  base::Unretained(this), &run_loop));
@@ -186,7 +186,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, GetPageThumbnailNetwork) {
 
   base::RunLoop run_loop2;
   // Fetch non-existing URL.
-  thumbnail_manager_->GetPageThumbnail(
+  thumbnail_manager_->GetImageForURL(
       GURL(kTestUrl2),
       base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
                  base::Unretained(this), &run_loop2));
@@ -197,14 +197,14 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, GetPageThumbnailNetwork) {
 }
 
 IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
-                       GetPageThumbnailNetworkMultiple) {
+                       GetImageForURLNetworkMultiple) {
   InitializeDefaultThumbnailMapAndDatabase(thumbnail_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.
-    thumbnail_manager_->GetPageThumbnail(
+    thumbnail_manager_->GetImageForURL(
         GURL(kTestUrl1),
         base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
                    base::Unretained(this), &run_loop));
@@ -216,13 +216,13 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
-                       GetPageThumbnailNetworkInvalid) {
+                       GetImageForURLNetworkInvalid) {
   SuggestionsProfile suggestions_profile;
   ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
   suggestion->set_url(kTestUrl1);
   suggestion->set_thumbnail(test_server_.GetURL(kInvalidImagePath).spec());
 
-  thumbnail_manager_->InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager_->Initialize(suggestions_profile);
 
   // Database will be initialized and loaded without anything in it.
   fake_db_->InitCallback(true);
@@ -230,7 +230,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
 
   base::RunLoop run_loop;
   // Fetch existing URL that has invalid thumbnail.
-  thumbnail_manager_->GetPageThumbnail(
+  thumbnail_manager_->GetImageForURL(
       GURL(kTestUrl1),
       base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
                  base::Unretained(this), &run_loop));
@@ -241,7 +241,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
-                       GetPageThumbnailNetworkCacheHit) {
+                       GetImageForURLNetworkCacheHit) {
   InitializeTestBitmapData();
 
   SuggestionsProfile suggestions_profile;
@@ -254,7 +254,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
   AddEntry(GetSampleThumbnailData(kTestUrl1), &db_model_);
   FakeDB<ThumbnailData>* fake_db = new FakeDB<ThumbnailData>(&db_model_);
   thumbnail_manager_.reset(CreateThumbnailManager(fake_db));
-  thumbnail_manager_->InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager_->Initialize(suggestions_profile);
   fake_db->InitCallback(true);
   fake_db->LoadCallback(true);
   // Expect something in the cache.
@@ -262,7 +262,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest,
   EXPECT_FALSE(bitmap->isNull());
 
   base::RunLoop run_loop;
-  thumbnail_manager_->GetPageThumbnail(
+  thumbnail_manager_->GetImageForURL(
       GURL(kTestUrl1),
       base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
                  base::Unretained(this), &run_loop));

chrome/browser/search/suggestions/thumbnail_manager_unittest.cc

@@ -43,7 +43,7 @@ class ThumbnailManagerTest : public testing::Test {
 
 namespace suggestions {
 
-TEST_F(ThumbnailManagerTest, InitializeThumbnailMapTest) {
+TEST_F(ThumbnailManagerTest, InitializeTest) {
   SuggestionsProfile suggestions_profile;
   ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
   suggestion->set_url(kTestUrl);
@@ -52,7 +52,7 @@ TEST_F(ThumbnailManagerTest, InitializeThumbnailMapTest) {
   TestingProfile profile;
   scoped_ptr<ThumbnailManager> thumbnail_manager(
       CreateThumbnailManager(&profile));
-  thumbnail_manager->InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager->Initialize(suggestions_profile);
 
   GURL output;
   EXPECT_TRUE(thumbnail_manager->GetThumbnailURL(GURL(kTestUrl), &output));

chrome/chrome_browser.gypi

@@ -1137,6 +1137,7 @@
       'browser/search/search.h',
       'browser/search/suggestions/blacklist_store.cc',
       'browser/search/suggestions/blacklist_store.h',
+      'browser/search/suggestions/image_manager.h',
       'browser/search/suggestions/suggestions_service.cc',
       'browser/search/suggestions/suggestions_service.h',
       'browser/search/suggestions/suggestions_service_factory.cc',