[Suggestions] Adding a Thumbnail Manager for the Suggestions Service

Will fetch server thumbnails if they are available. Using the BitmapFetcher to obtain and decode images.

I've tested with a subsequent CL that modifies suggestions_source.cc.

Next CLs will implement local caching once the image is fetched.

BUG=None
TEST=ThumbnailManagerTest,ThumbnailManagerBrowserTest

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

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

chrome/browser/search/suggestions/proto/suggestions.proto

@@ -25,4 +25,10 @@ message ChromeSuggestion {
 
   // Title of the suggestion.
   optional string title = 2;
+
+  // The URL of the favicon associated with this page.
+  optional string favicon_url = 3;
+
+  // The URL of the thumbnail associated with this page.
+  optional string thumbnail = 4;
 }

chrome/browser/search/suggestions/suggestions_service.cc

@@ -68,7 +68,8 @@ const char kSuggestionsFieldTrialStateParam[] = "state";
 const char kSuggestionsFieldTrialStateEnabled[] = "enabled";
 
 SuggestionsService::SuggestionsService(Profile* profile)
-    : profile_(profile) {
+    : thumbnail_manager_(new ThumbnailManager(profile)),
+      profile_(profile) {
   // Obtain the URL to use to fetch suggestions data from the Variations param.
   suggestions_url_ = GURL(GetExperimentParam(kSuggestionsFieldTrialURLParam));
 }
@@ -111,6 +112,12 @@ void SuggestionsService::FetchSuggestionsData(
   last_request_started_time_ = base::TimeTicks::Now();
 }
 
+void SuggestionsService::GetPageThumbnail(
+      const GURL& url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+  thumbnail_manager_->GetPageThumbnail(url, callback);
+}
+
 void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   DCHECK_EQ(pending_request_.get(), source);
@@ -154,6 +161,7 @@ void SuggestionsService::OnURLFetchComplete(const net::URLFetcher* source) {
     LogResponseState(RESPONSE_EMPTY);
   } else if (suggestions.ParseFromString(suggestions_data)) {
     LogResponseState(RESPONSE_VALID);
+    thumbnail_manager_->InitializeThumbnailMap(suggestions);
   } else {
     LogResponseState(RESPONSE_INVALID);
   }

chrome/browser/search/suggestions/suggestions_service.h

@@ -13,8 +13,10 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.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"
 #include "url/gurl.h"
 
 class Profile;
@@ -43,6 +45,12 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   // fetch request completes.
   void FetchSuggestionsData(ResponseCallback callback);
 
+  // Retrieves stored thumbnail for website |url| asynchronously. Calls
+  // |callback| with Bitmap pointer if found, and NULL otherwise.
+  void GetPageThumbnail(
+      const GURL& url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
  private:
   FRIEND_TEST_ALL_PREFIXES(SuggestionsServiceTest, FetchSuggestionsData);
 
@@ -68,6 +76,9 @@ class SuggestionsService : public KeyedService, public net::URLFetcherDelegate {
   // Queue of callbacks. These are flushed when fetch request completes.
   std::vector<ResponseCallback> waiting_requestors_;
 
+  // Used to obtain server thumbnails, if available.
+  scoped_ptr<ThumbnailManager> thumbnail_manager_;
+
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(SuggestionsService);

chrome/browser/search/suggestions/thumbnail_manager.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/thumbnail_manager.h"
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/base/load_flags.h"
+
+namespace suggestions {
+
+ThumbnailManager::ThumbnailManager(Profile* profile)
+    : url_request_context_(profile->GetRequestContext()) {}
+
+ThumbnailManager::~ThumbnailManager() {}
+
+ThumbnailManager::ThumbnailRequest::ThumbnailRequest() : fetcher(NULL) {}
+
+ThumbnailManager::ThumbnailRequest::ThumbnailRequest(chrome::BitmapFetcher* f)
+    : fetcher(f) {}
+
+ThumbnailManager::ThumbnailRequest::~ThumbnailRequest() {
+  delete fetcher;
+}
+
+void ThumbnailManager::InitializeThumbnailMap(
+    const SuggestionsProfile& suggestions) {
+  thumbnail_map_.clear();
+  for (int i = 0; i < suggestions.suggestions_size(); ++i) {
+    const ChromeSuggestion& suggestion = suggestions.suggestions(i);
+    if (suggestion.has_thumbnail()) {
+      thumbnail_map_[GURL(suggestion.url())] = GURL(suggestion.thumbnail());
+    }
+  }
+}
+
+void ThumbnailManager::GetPageThumbnail(
+    const GURL& url,
+    base::Callback<void(const GURL&, const SkBitmap*)> callback) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  // If |url| is not found in |thumbnail_map_|, then invoke |callback| with NULL
+  // since there is no associated thumbnail.
+  GURL thumbnail_url;
+  if (!GetThumbnailURL(url, &thumbnail_url)) {
+    callback.Run(url, NULL);
+    return;
+  }
+
+  // Look for a request in progress for |thumbnail_url|.
+  ThumbnailRequestMap::iterator it = pending_requests_.find(thumbnail_url);
+  if (it == pending_requests_.end()) {
+    // |thumbnail_url| is not being fetched, so create a request and initiate
+    // the fetch.
+    ThumbnailRequest request(new chrome::BitmapFetcher(thumbnail_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_requests_[thumbnail_url].swap(&request);
+  } else {
+    // Request in progress. Register as an interested callback.
+    it->second.callbacks.push_back(callback);
+  }
+}
+
+void ThumbnailManager::OnFetchComplete(const GURL thumbnail_url,
+                                       const SkBitmap* bitmap) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  ThumbnailRequestMap::iterator it = pending_requests_.find(thumbnail_url);
+  DCHECK(it != pending_requests_.end());
+
+  ThumbnailRequest* request = &it->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 it = request->callbacks.begin();
+       it != request->callbacks.end(); ++it) {
+    it->Run(request->url, bitmap);
+  }
+  pending_requests_.erase(it);
+}
+
+bool ThumbnailManager::GetThumbnailURL(const GURL& url, GURL* thumbnail_url) {
+  std::map<GURL, GURL>::iterator it = thumbnail_map_.find(url);
+  if (it == thumbnail_map_.end()) return false;  // Not found.
+  *thumbnail_url = it->second;
+  return true;
+}
+
+}  // namespace suggestions

chrome/browser/search/suggestions/thumbnail_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_THUMBNAIL_MANAGER_H_
+#define CHROME_BROWSER_SEARCH_SUGGESTIONS_THUMBNAIL_MANAGER_H_
+
+#include <map>
+#include <utility>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "chrome/browser/bitmap_fetcher.h"
+#include "ui/gfx/image/image_skia.h"
+#include "url/gurl.h"
+
+class Profile;
+
+namespace suggestions {
+
+class SuggestionsProfile;
+
+// A class used to fetch server thumbnails asynchronously.
+class ThumbnailManager : public chrome::BitmapFetcherDelegate {
+ public:
+  explicit ThumbnailManager(Profile* profile);
+  virtual ~ThumbnailManager();
+
+  // Initializes the |thumbnail_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(
+      const GURL& url,
+      base::Callback<void(const GURL&, const SkBitmap*)> callback);
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerTest, InitializeThumbnailMapTest);
+  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerBrowserTest, FetchThumbnails);
+  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerBrowserTest, FetchThumbnailsInvalid);
+  FRIEND_TEST_ALL_PREFIXES(ThumbnailManagerBrowserTest,
+                           FetchThumbnailsMultiple);
+
+  typedef std::vector<base::Callback<void(const GURL&, const SkBitmap*)> >
+      CallbackVector;
+
+  // State related to a thumbnail fetch (associated website url, fetcher,
+  // pending callbacks).
+  struct ThumbnailRequest {
+    ThumbnailRequest();
+    explicit ThumbnailRequest(chrome::BitmapFetcher* f);
+    ~ThumbnailRequest();
+
+    void swap(ThumbnailRequest* other) {
+      std::swap(url, other->url);
+      std::swap(callbacks, other->callbacks);
+      std::swap(fetcher, other->fetcher);
+    }
+
+    GURL url;
+    chrome::BitmapFetcher* fetcher;
+    // Queue for pending callbacks, which may accumulate while the request is in
+    // flight.
+    CallbackVector callbacks;
+  };
+
+  typedef std::map<const GURL, ThumbnailRequest> ThumbnailRequestMap;
+
+  // Inherited from BitmapFetcherDelegate. Runs on the UI thread.
+  virtual void OnFetchComplete(const GURL thumbnail_url,
+                               const SkBitmap* bitmap) OVERRIDE;
+
+  // Looks up thumbnail for |url|. If found, writes the result to
+  // |thumbnail_url| and returns true. Otherwise just returns false.
+  bool GetThumbnailURL(const GURL& url, GURL* thumbnail_url);
+
+  // Used for substituting the request context during testing.
+  void set_request_context(net::URLRequestContextGetter* context) {
+    url_request_context_ = context;
+  }
+
+  // Map from URL to thumbnail URL. Should be kept up to date when a new
+  // SuggestionsProfile is available.
+  std::map<GURL, GURL> thumbnail_map_;
+
+  // Map from each thumbnail URL to the request information (associated website
+  // url, fetcher, pending callbacks).
+  ThumbnailRequestMap pending_requests_;
+
+  net::URLRequestContextGetter* url_request_context_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThumbnailManager);
+};
+
+}  // namespace suggestions
+
+#endif  // CHROME_BROWSER_SEARCH_SUGGESTIONS_THUMBNAIL_MANAGER_H_

chrome/browser/search/suggestions/thumbnail_manager_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 <string>
+
+#include "base/files/file_path.h"
+#include "chrome/browser/search/suggestions/proto/suggestions.pb.h"
+#include "chrome/browser/search/suggestions/thumbnail_manager.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
+#include "net/test/spawned_test_server/spawned_test_server.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/image/image_skia.h"
+#include "url/gurl.h"
+
+namespace {
+
+const char kTestUrl1[] = "http://go.com/";
+const char kTestUrl2[] = "http://goal.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 content::BrowserThread;
+
+class ThumbnailManagerBrowserTest : public InProcessBrowserTest {
+ public:
+  ThumbnailManagerBrowserTest()
+    : num_callback_null_called_(0),
+      num_callback_valid_called_(0),
+      thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
+      test_server_(net::SpawnedTestServer::TYPE_HTTP,
+                   net::SpawnedTestServer::kLocalhost,
+                   base::FilePath(kDocRoot)) {}
+
+  virtual void SetUp() OVERRIDE {
+    ASSERT_TRUE(test_server_.Start());
+
+    context_ = new net::TestURLRequestContextGetter(
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+    context_->AddRef();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    BrowserThread::ReleaseSoon(BrowserThread::IO, FROM_HERE, context_);
+  }
+
+  void OnThumbnailAvailable(const GURL& url, const SkBitmap* bitmap) {
+    if (bitmap) {
+      num_callback_valid_called_++;
+    } else {
+      num_callback_null_called_++;
+    }
+  }
+  int num_callback_null_called_;
+  int num_callback_valid_called_;
+  content::TestBrowserThreadBundle thread_bundle_;
+  net::SpawnedTestServer test_server_;
+  net::TestURLRequestContextGetter* context_;
+};
+
+}  // namespace
+
+namespace suggestions {
+
+IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, FetchThumbnails) {
+  SuggestionsProfile suggestions_profile;
+  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
+  suggestion->set_url(kTestUrl1);
+  suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
+
+  TestingProfile profile;
+  ThumbnailManager thumbnail_manager(&profile);
+  thumbnail_manager.InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager.set_request_context(context_);
+
+  // Fetch existing URL.
+  thumbnail_manager.GetPageThumbnail(
+      GURL(kTestUrl1),
+      base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
+                 base::Unretained(this)));
+
+  content::RunMessageLoop();
+
+  EXPECT_EQ(0, num_callback_null_called_);
+  EXPECT_EQ(1, num_callback_valid_called_);
+
+  // Fetch non-existing URL.
+  thumbnail_manager.GetPageThumbnail(
+      GURL(kTestUrl2),
+      base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
+                 base::Unretained(this)));
+
+  content::RunMessageLoop();
+
+  EXPECT_EQ(1, num_callback_null_called_);
+  EXPECT_EQ(1, num_callback_valid_called_);
+}
+
+IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, FetchThumbnailsMultiple) {
+  SuggestionsProfile suggestions_profile;
+  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
+  suggestion->set_url(kTestUrl1);
+  suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec());
+
+  TestingProfile profile;
+  ThumbnailManager thumbnail_manager(&profile);
+  thumbnail_manager.InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager.set_request_context(context_);
+
+  // Fetch non-existing URL, and add more while request is in flight.
+  for (int i = 0 ; i < 5; i++) {
+    thumbnail_manager.GetPageThumbnail(
+      GURL(kTestUrl1),
+      base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
+                 base::Unretained(this)));
+  }
+
+  content::RunMessageLoop();
+
+  EXPECT_EQ(0, num_callback_null_called_);
+  EXPECT_EQ(5, num_callback_valid_called_);
+}
+
+IN_PROC_BROWSER_TEST_F(ThumbnailManagerBrowserTest, FetchThumbnailsInvalid) {
+  SuggestionsProfile suggestions_profile;
+  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
+  suggestion->set_url(kTestUrl1);
+  suggestion->set_thumbnail(test_server_.GetURL(kInvalidImagePath).spec());
+
+  TestingProfile profile;
+  ThumbnailManager thumbnail_manager(&profile);
+  thumbnail_manager.InitializeThumbnailMap(suggestions_profile);
+  thumbnail_manager.set_request_context(context_);
+
+  // Fetch existing URL that has invalid thumbnail.
+  thumbnail_manager.GetPageThumbnail(
+      GURL(kTestUrl1),
+      base::Bind(&ThumbnailManagerBrowserTest::OnThumbnailAvailable,
+                 base::Unretained(this)));
+
+  content::RunMessageLoop();
+
+  EXPECT_EQ(1, num_callback_null_called_);
+  EXPECT_EQ(0, num_callback_valid_called_);
+}
+
+}  // namespace suggestions

chrome/browser/search/suggestions/thumbnail_manager_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 "chrome/browser/search/suggestions/proto/suggestions.pb.h"
+#include "chrome/browser/search/suggestions/thumbnail_manager.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+const char kTestUrl[] = "http://go.com/";
+const char kTestThumbnailUrl[] = "http://thumb.com/anchor_download_test.png";
+
+class ThumbnailManagerTest : public testing::Test {
+ protected:
+  content::TestBrowserThreadBundle thread_bundle_;
+};
+
+}  // namespace
+
+namespace suggestions {
+
+TEST_F(ThumbnailManagerTest, InitializeThumbnailMapTest) {
+  SuggestionsProfile suggestions_profile;
+  ChromeSuggestion* suggestion = suggestions_profile.add_suggestions();
+  suggestion->set_url(kTestUrl);
+  suggestion->set_thumbnail(kTestThumbnailUrl);
+
+  TestingProfile profile;
+  ThumbnailManager thumbnail_manager(&profile);
+  thumbnail_manager.InitializeThumbnailMap(suggestions_profile);
+
+  GURL output;
+  EXPECT_TRUE(thumbnail_manager.GetThumbnailURL(GURL(kTestUrl), &output));
+  EXPECT_EQ(GURL(kTestThumbnailUrl), output);
+
+  EXPECT_FALSE(thumbnail_manager.GetThumbnailURL(GURL("http://b.com"),
+                                                 &output));
+}
+
+}  // namespace suggestions

chrome/chrome_browser.gypi

@@ -1970,6 +1970,8 @@
         'browser/search/suggestions/suggestions_service_factory.h',
         'browser/search/suggestions/suggestions_source.cc',
         'browser/search/suggestions/suggestions_source.h',
+        'browser/search/suggestions/thumbnail_manager.cc',
+        'browser/search/suggestions/thumbnail_manager.h',
         'browser/search_engines/default_search_manager.cc',
         'browser/search_engines/default_search_manager.h',
         'browser/search_engines/default_search_pref_migration.cc',

chrome/chrome_tests.gypi

@@ -1327,6 +1327,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/thumbnail_manager_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

@@ -1258,6 +1258,7 @@
         'browser/search/search_unittest.cc',
         'browser/search/search_android_unittest.cc',
         'browser/search/suggestions/suggestions_service_unittest.cc',
+        'browser/search/suggestions/thumbnail_manager_unittest.cc',
         'browser/search_engines/default_search_manager_unittest.cc',
         'browser/search_engines/default_search_pref_migration_unittest.cc',
         'browser/search_engines/default_search_policy_handler_unittest.cc',