diff --git a/chrome/browser/search/suggestions/image_fetcher_impl.cc b/chrome/browser/search/suggestions/image_fetcher_impl.cc deleted file mode 100644 index 949f6d30028e..000000000000 --- a/chrome/browser/search/suggestions/image_fetcher_impl.cc +++ /dev/null @@ -1,82 +0,0 @@ -// 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 - -#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 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 diff --git a/chrome/browser/search/suggestions/image_fetcher_impl.h b/chrome/browser/search/suggestions/image_fetcher_impl.h deleted file mode 100644 index 4d79a6cd892b..000000000000 --- a/chrome/browser/search/suggestions/image_fetcher_impl.h +++ /dev/null @@ -1,84 +0,0 @@ -// 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_FETCHER_IMPL_H_ -#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_ - -#include -#include -#include - -#include "base/basictypes.h" -#include "base/callback.h" -#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h" -#include "components/suggestions/image_fetcher.h" -#include "ui/gfx/image/image_skia.h" -#include "url/gurl.h" - -namespace net { -class URLRequestContextGetter; -} - -namespace suggestions { - -// A class used to fetch server images. -class ImageFetcherImpl : public ImageFetcher, - public chrome::BitmapFetcherDelegate { - public: - 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 callback) OVERRIDE; - - private: - // Inherited from BitmapFetcherDelegate. Runs on the UI thread. - virtual void OnFetchComplete(const GURL image_url, - const SkBitmap* bitmap) OVERRIDE; - - typedef std::vector > - CallbackVector; - - // 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(); - - void swap(ImageRequest* other) { - std::swap(url, other->url); - std::swap(image_url, other->image_url); - std::swap(callbacks, other->callbacks); - std::swap(fetcher, other->fetcher); - } - - GURL url; - GURL image_url; - chrome::BitmapFetcher* fetcher; - // Queue for pending callbacks, which may accumulate while the request is in - // flight. - CallbackVector callbacks; - }; - - typedef std::map ImageRequestMap; - - // Map from each image URL to the request information (associated website - // url, fetcher, pending callbacks). - ImageRequestMap pending_net_requests_; - - ImageFetcherDelegate* delegate_; - - net::URLRequestContextGetter* url_request_context_; - - DISALLOW_COPY_AND_ASSIGN(ImageFetcherImpl); -}; - -} // namespace suggestions - -#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_FETCHER_IMPL_H_ diff --git a/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc b/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc deleted file mode 100644 index 8b4fcfda2f48..000000000000 --- a/chrome/browser/search/suggestions/image_fetcher_impl_browsertest.cc +++ /dev/null @@ -1,149 +0,0 @@ -// 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 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 diff --git a/components/suggestions/image_manager.cc b/chrome/browser/search/suggestions/image_manager_impl.cc similarity index 54% rename from components/suggestions/image_manager.cc rename to chrome/browser/search/suggestions/image_manager_impl.cc index 197c11fc6bd3..3e0ce3b67bfe 100644 --- a/components/suggestions/image_manager.cc +++ b/chrome/browser/search/suggestions/image_manager_impl.cc @@ -2,10 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/suggestions/image_manager.h" +#include "chrome/browser/search/suggestions/image_manager_impl.h" -#include "base/bind.h" -#include "components/suggestions/image_fetcher.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 "ui/gfx/codec/jpeg_codec.h" using leveldb_proto::ProtoDatabase; @@ -21,27 +23,30 @@ SkBitmap* DecodeImage(const std::vector& encoded_data) { namespace suggestions { -ImageManager::ImageManager() : weak_ptr_factory_(this) {} +ImageManagerImpl::ImageManagerImpl() : weak_ptr_factory_(this) {} -ImageManager::ImageManager(scoped_ptr image_fetcher, - scoped_ptr > database, - const base::FilePath& database_dir) - : image_fetcher_(image_fetcher.Pass()), +ImageManagerImpl::ImageManagerImpl( + net::URLRequestContextGetter* url_request_context, + scoped_ptr > database, + const base::FilePath& database_dir) + : url_request_context_(url_request_context), database_(database.Pass()), database_ready_(false), weak_ptr_factory_(this) { - image_fetcher_->SetImageFetcherDelegate(this); - database_->Init(database_dir, base::Bind(&ImageManager::OnDatabaseInit, + database_->Init(database_dir, base::Bind(&ImageManagerImpl::OnDatabaseInit, weak_ptr_factory_.GetWeakPtr())); } -ImageManager::~ImageManager() {} +ImageManagerImpl::~ImageManagerImpl() {} -ImageManager::ImageCacheRequest::ImageCacheRequest() {} +ImageManagerImpl::ImageRequest::ImageRequest() : fetcher(NULL) {} -ImageManager::ImageCacheRequest::~ImageCacheRequest() {} +ImageManagerImpl::ImageRequest::ImageRequest(chrome::BitmapFetcher* f) + : fetcher(f) {} -void ImageManager::Initialize(const SuggestionsProfile& suggestions) { +ImageManagerImpl::ImageRequest::~ImageRequest() { delete fetcher; } + +void ImageManagerImpl::Initialize(const SuggestionsProfile& suggestions) { image_url_map_.clear(); for (int i = 0; i < suggestions.suggestions_size(); ++i) { const ChromeSuggestion& suggestion = suggestions.suggestions(i); @@ -51,10 +56,10 @@ void ImageManager::Initialize(const SuggestionsProfile& suggestions) { } } -void ImageManager::GetImageForURL( +void ImageManagerImpl::GetImageForURL( const GURL& url, base::Callback callback) { - DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); // 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; @@ -74,44 +79,40 @@ void ImageManager::GetImageForURL( ServeFromCacheOrNetwork(url, image_url, callback); } -void ImageManager::OnImageFetched(const GURL& url, const SkBitmap* bitmap) { - SaveImage(url, *bitmap); -} - -bool ImageManager::GetImageURL(const GURL& url, GURL* image_url) { +bool ImageManagerImpl::GetImageURL(const GURL& url, GURL* image_url) { std::map::iterator it = image_url_map_.find(url); if (it == image_url_map_.end()) return false; // Not found. *image_url = it->second; return true; } -void ImageManager::QueueCacheRequest( +void ImageManagerImpl::QueueCacheRequest( const GURL& url, const GURL& image_url, base::Callback callback) { // To be served when the database has loaded. - ImageCacheRequestMap::iterator it = pending_cache_requests_.find(url); + ImageRequestMap::iterator it = pending_cache_requests_.find(url); if (it == pending_cache_requests_.end()) { - ImageCacheRequest request; + ImageRequest request(NULL); request.url = url; request.image_url = image_url; request.callbacks.push_back(callback); - pending_cache_requests_[url] = request; + pending_cache_requests_[url].swap(&request); } else { // Request already queued for this url. it->second.callbacks.push_back(callback); } } -void ImageManager::ServeFromCacheOrNetwork( +void ImageManagerImpl::ServeFromCacheOrNetwork( const GURL& url, const GURL& image_url, base::Callback callback) { // If there is a image available in memory, return it. if (!ServeFromCache(url, callback)) { - image_fetcher_->StartOrQueueNetworkRequest(url, image_url, callback); + StartOrQueueNetworkRequest(url, image_url, callback); } } -bool ImageManager::ServeFromCache( +bool ImageManagerImpl::ServeFromCache( const GURL& url, base::Callback callback) { SkBitmap* bitmap = GetBitmapFromCache(url); @@ -122,7 +123,7 @@ bool ImageManager::ServeFromCache( return false; } -SkBitmap* ImageManager::GetBitmapFromCache(const GURL& url) { +SkBitmap* ImageManagerImpl::GetBitmapFromCache(const GURL& url) { ImageMap::iterator image_iter = image_map_.find(url.spec()); if (image_iter != image_map_.end()) { return &image_iter->second; @@ -130,7 +131,56 @@ SkBitmap* ImageManager::GetBitmapFromCache(const GURL& url) { return NULL; } -void ImageManager::SaveImage(const GURL& url, const SkBitmap& bitmap) { +void ImageManagerImpl::StartOrQueueNetworkRequest( + const GURL& url, const GURL& image_url, + base::Callback 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) { // Update the image map. image_map_.insert(std::make_pair(url.spec(), bitmap)); @@ -150,24 +200,24 @@ void ImageManager::SaveImage(const GURL& url, const SkBitmap& bitmap) { new std::vector()); entries_to_save->push_back(std::make_pair(data.url(), data)); database_->UpdateEntries(entries_to_save.Pass(), keys_to_remove.Pass(), - base::Bind(&ImageManager::OnDatabaseSave, + base::Bind(&ImageManagerImpl::OnDatabaseSave, weak_ptr_factory_.GetWeakPtr())); } } -void ImageManager::OnDatabaseInit(bool success) { +void ImageManagerImpl::OnDatabaseInit(bool success) { if (!success) { DVLOG(1) << "Image database init failed."; database_.reset(); ServePendingCacheRequests(); return; } - database_->LoadEntries(base::Bind(&ImageManager::OnDatabaseLoad, + database_->LoadEntries(base::Bind(&ImageManagerImpl::OnDatabaseLoad, weak_ptr_factory_.GetWeakPtr())); } -void ImageManager::OnDatabaseLoad(bool success, - scoped_ptr entries) { +void ImageManagerImpl::OnDatabaseLoad(bool success, + scoped_ptr entries) { if (!success) { DVLOG(1) << "Image database load failed."; database_.reset(); @@ -180,7 +230,7 @@ void ImageManager::OnDatabaseLoad(bool success, ServePendingCacheRequests(); } -void ImageManager::OnDatabaseSave(bool success) { +void ImageManagerImpl::OnDatabaseSave(bool success) { if (!success) { DVLOG(1) << "Image database save failed."; database_.reset(); @@ -188,7 +238,7 @@ void ImageManager::OnDatabaseSave(bool success) { } } -void ImageManager::LoadEntriesInCache(scoped_ptr entries) { +void ImageManagerImpl::LoadEntriesInCache(scoped_ptr entries) { for (ImageDataVector::iterator it = entries->begin(); it != entries->end(); ++it) { std::vector encoded_data(it->data().begin(), @@ -201,10 +251,10 @@ void ImageManager::LoadEntriesInCache(scoped_ptr entries) { } } -void ImageManager::ServePendingCacheRequests() { - for (ImageCacheRequestMap::iterator it = pending_cache_requests_.begin(); +void ImageManagerImpl::ServePendingCacheRequests() { + for (ImageRequestMap::iterator it = pending_cache_requests_.begin(); it != pending_cache_requests_.end(); ++it) { - const ImageCacheRequest& request = it->second; + const ImageRequest& 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); @@ -213,8 +263,8 @@ void ImageManager::ServePendingCacheRequests() { } // static -bool ImageManager::EncodeImage(const SkBitmap& bitmap, - std::vector* dest) { +bool ImageManagerImpl::EncodeImage(const SkBitmap& bitmap, + std::vector* dest) { SkAutoLockPixels bitmap_lock(bitmap); if (!bitmap.readyToDraw() || bitmap.isNull()) { return false; diff --git a/chrome/browser/search/suggestions/image_manager_impl.h b/chrome/browser/search/suggestions/image_manager_impl.h new file mode 100644 index 000000000000..df8d3abd027c --- /dev/null +++ b/chrome/browser/search/suggestions/image_manager_impl.h @@ -0,0 +1,170 @@ +// 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_IMPL_H_ +#define CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_IMPL_H_ + +#include +#include +#include + +#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 "ui/gfx/image/image_skia.h" +#include "url/gurl.h" + +namespace net { +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, + public chrome::BitmapFetcherDelegate { + public: + typedef std::vector ImageDataVector; + + ImageManagerImpl( + net::URLRequestContextGetter* url_request_context, + scoped_ptr > 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, + base::Callback 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(); + + typedef std::vector > + CallbackVector; + typedef base::hash_map ImageMap; + + // State related to an image fetch (associated website url, image_url, + // fetcher, pending callbacks). + struct ImageRequest { + ImageRequest(); + explicit ImageRequest(chrome::BitmapFetcher* f); + ~ImageRequest(); + + void swap(ImageRequest* other) { + std::swap(url, other->url); + std::swap(image_url, other->image_url); + std::swap(callbacks, other->callbacks); + std::swap(fetcher, other->fetcher); + } + + GURL url; + GURL image_url; + chrome::BitmapFetcher* fetcher; + // Queue for pending callbacks, which may accumulate while the request is in + // flight. + CallbackVector callbacks; + }; + + typedef std::map 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 callback); + + void ServeFromCacheOrNetwork( + const GURL& url, const GURL& image_url, + base::Callback 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 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 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 entries); + void OnDatabaseSave(bool success); + + // Take entries from the database and put them in the local cache. + void LoadEntriesInCache(scoped_ptr 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* dest); + + // Map from URL to image URL. Should be kept up to date when a new + // SuggestionsProfile is available. + std::map 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_; + + net::URLRequestContextGetter* url_request_context_; + + scoped_ptr > database_; + + bool database_ready_; + + base::WeakPtrFactory weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(ImageManagerImpl); +}; + +} // namespace suggestions + +#endif // CHROME_BROWSER_SEARCH_SUGGESTIONS_IMAGE_MANAGER_IMPL_H_ diff --git a/chrome/browser/search/suggestions/image_manager_impl_browsertest.cc b/chrome/browser/search/suggestions/image_manager_impl_browsertest.cc new file mode 100644 index 000000000000..fcbfb405d063 --- /dev/null +++ b/chrome/browser/search/suggestions/image_manager_impl_browsertest.cc @@ -0,0 +1,277 @@ +// 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 + +#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/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/gtest/include/gtest/gtest.h" +#include "ui/gfx/image/image_skia.h" +#include "url/gurl.h" + +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; + +typedef base::hash_map EntryMap; + +void AddEntry(const ImageData& d, EntryMap* map) { (*map)[d.url()] = d; } + +class ImageManagerImplBrowserTest : public InProcessBrowserTest { + 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(); + } + + virtual void TearDownInProcessBrowserTestFixture() OVERRIDE { + test_server_.Stop(); + } + + virtual void SetUpOnMainThread() OVERRIDE { + fake_db_ = new FakeDB(&db_model_); + image_manager_.reset(CreateImageManagerImpl(fake_db_)); + } + + virtual void TearDownOnMainThread() OVERRIDE { + fake_db_ = NULL; + db_model_.clear(); + image_manager_.reset(); + test_image_manager_.reset(); + } + + void InitializeTestBitmapData() { + FakeDB* test_fake_db = new FakeDB(&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* fake_db) { + CHECK(image_manager); + CHECK(fake_db); + + suggestions::SuggestionsProfile suggestions_profile; + suggestions::ChromeSuggestion* suggestion = + suggestions_profile.add_suggestions(); + suggestion->set_url(kTestUrl1); + suggestion->set_thumbnail(test_server_.GetURL(kTestImagePath).spec()); + + image_manager->Initialize(suggestions_profile); + + // Initialize empty database. + fake_db->InitCallback(true); + fake_db->LoadCallback(true); + } + + ImageData GetSampleImageData(const std::string& url) { + ImageData data; + data.set_url(url); + std::vector encoded; + EXPECT_TRUE(ImageManagerImpl::EncodeImage(test_bitmap_, &encoded)); + data.set_data(std::string(encoded.begin(), encoded.end())); + return data; + } + + void OnImageAvailable(base::RunLoop* loop, const GURL& url, + const SkBitmap* bitmap) { + if (bitmap) { + num_callback_valid_called_++; + std::vector actual; + std::vector 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* fake_db) { + return new ImageManagerImpl( + browser()->profile()->GetRequestContext(), + scoped_ptr >(fake_db), + FakeDB::DirectoryForTestDB()); + } + + EntryMap db_model_; + // Owned by the ImageManagerImpl under test. + FakeDB* fake_db_; + + SkBitmap test_bitmap_; + scoped_ptr test_image_manager_; + + int num_callback_null_called_; + int num_callback_valid_called_; + net::SpawnedTestServer test_server_; + // Under test. + scoped_ptr 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(); + + EXPECT_EQ(0, num_callback_null_called_); + EXPECT_EQ(1, num_callback_valid_called_); + + 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(); + + EXPECT_EQ(1, num_callback_null_called_); + EXPECT_EQ(1, num_callback_valid_called_); +} + +IN_PROC_BROWSER_TEST_F(ImageManagerImplBrowserTest, + GetImageForURLNetworkMultiple) { + InitializeTestBitmapData(); + 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); + + 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)); + 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(); + + 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()); + + // Create the ImageManagerImpl with an added entry in the database. + AddEntry(GetSampleImageData(kTestUrl1), &db_model_); + FakeDB* fake_db = new FakeDB(&db_model_); + image_manager_.reset(CreateImageManagerImpl(fake_db)); + image_manager_->Initialize(suggestions_profile); + fake_db->InitCallback(true); + fake_db->LoadCallback(true); + // Expect something in the cache. + SkBitmap* bitmap = image_manager_->GetBitmapFromCache(GURL(kTestUrl1)); + EXPECT_FALSE(bitmap->isNull()); + + base::RunLoop run_loop; + 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(1, num_callback_valid_called_); +} + +} // namespace suggestions diff --git a/chrome/browser/search/suggestions/image_manager_impl_unittest.cc b/chrome/browser/search/suggestions/image_manager_impl_unittest.cc new file mode 100644 index 000000000000..bf99191d1794 --- /dev/null +++ b/chrome/browser/search/suggestions/image_manager_impl_unittest.cc @@ -0,0 +1,64 @@ +// 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 + +#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 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* fake_db = new FakeDB(&db_model_); + return new ImageManagerImpl( + profile->GetRequestContext(), + scoped_ptr >(fake_db), + FakeDB::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 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 diff --git a/chrome/browser/search/suggestions/suggestions_service_factory.cc b/chrome/browser/search/suggestions/suggestions_service_factory.cc index 4a98018a2374..e16b07043fdd 100644 --- a/chrome/browser/search/suggestions/suggestions_service_factory.cc +++ b/chrome/browser/search/suggestions/suggestions_service_factory.cc @@ -8,13 +8,12 @@ #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_fetcher_impl.h" +#include "chrome/browser/search/suggestions/image_manager_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" @@ -67,19 +66,18 @@ KeyedService* SuggestionsServiceFactory::BuildServiceInstanceFor( new BlacklistStore(the_profile->GetPrefs())); scoped_ptr > db( - new leveldb_proto::ProtoDatabaseImpl(background_task_runner)); + new leveldb_proto::ProtoDatabaseImpl( + background_task_runner)); base::FilePath database_dir( the_profile->GetPath().Append(FILE_PATH_LITERAL("Thumbnails"))); - scoped_ptr image_fetcher( - new ImageFetcherImpl(the_profile->GetRequestContext())); - scoped_ptr thumbnail_manager(new ImageManager( - image_fetcher.PassAs(), + scoped_ptr thumbnail_manager(new ImageManagerImpl( + the_profile->GetRequestContext(), db.PassAs >(), database_dir)); return new SuggestionsService( the_profile->GetRequestContext(), suggestions_store.Pass(), - thumbnail_manager.Pass(), blacklist_store.Pass()); + thumbnail_manager.PassAs(), blacklist_store.Pass()); } void SuggestionsServiceFactory::RegisterProfilePrefs( diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 1ff559e23aac..58082c5936cd 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1072,8 +1072,8 @@ 'browser/search/most_visited_iframe_source.h', 'browser/search/search.cc', 'browser/search/search.h', - 'browser/search/suggestions/image_fetcher_impl.cc', - 'browser/search/suggestions/image_fetcher_impl.h', + 'browser/search/suggestions/image_manager_impl.cc', + 'browser/search/suggestions/image_manager_impl.h', 'browser/search/suggestions/suggestions_service_factory.cc', 'browser/search/suggestions/suggestions_service_factory.h', 'browser/search/suggestions/suggestions_source.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 6e4c4618cbe9..efa2a15859db 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1376,7 +1376,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_fetcher_impl_browsertest.cc', + 'browser/search/suggestions/image_manager_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', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 1e213e2a75ba..f52eae0392c7 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -697,6 +697,7 @@ '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', diff --git a/components/components_tests.gyp b/components/components_tests.gyp index c031b25e6310..2fd337db4227 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -217,7 +217,6 @@ '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', diff --git a/components/suggestions.gypi b/components/suggestions.gypi index e03dc2199ea2..f4820ec79edf 100644 --- a/components/suggestions.gypi +++ b/components/suggestions.gypi @@ -19,14 +19,10 @@ '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', diff --git a/components/suggestions/BUILD.gn b/components/suggestions/BUILD.gn index 12c9576a2a21..d505c402a2f4 100644 --- a/components/suggestions/BUILD.gn +++ b/components/suggestions/BUILD.gn @@ -6,9 +6,6 @@ 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", diff --git a/components/suggestions/DEPS b/components/suggestions/DEPS index 557fff63b1ea..e3304f39f065 100644 --- a/components/suggestions/DEPS +++ b/components/suggestions/DEPS @@ -1,6 +1,5 @@ include_rules = [ "+components/keyed_service/core", - "+components/leveldb_proto", "+components/pref_registry", "+components/variations", "+net", diff --git a/components/suggestions/image_fetcher.h b/components/suggestions/image_fetcher.h deleted file mode 100644 index 52fcff4f67da..000000000000 --- a/components/suggestions/image_fetcher.h +++ /dev/null @@ -1,34 +0,0 @@ -// 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 callback) = 0; - - private: - DISALLOW_COPY_AND_ASSIGN(ImageFetcher); -}; - -} // namespace suggestions - -#endif // COMPONENTS_SUGGESTIONS_IMAGE_FETCHER_H_ diff --git a/components/suggestions/image_fetcher_delegate.h b/components/suggestions/image_fetcher_delegate.h deleted file mode 100644 index f2335efe0262..000000000000 --- a/components/suggestions/image_fetcher_delegate.h +++ /dev/null @@ -1,29 +0,0 @@ -// 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_ diff --git a/components/suggestions/image_manager.h b/components/suggestions/image_manager.h index e538a6b917e2..8c09fdfce416 100644 --- a/components/suggestions/image_manager.h +++ b/components/suggestions/image_manager.h @@ -5,146 +5,31 @@ #ifndef COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_ #define COMPONENTS_SUGGESTIONS_IMAGE_MANAGER_H_ -#include -#include -#include - #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 { -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 { +// An interface to retrieve images related to a specific URL. +class ImageManager { public: - typedef std::vector ImageDataVector; - - ImageManager(scoped_ptr image_fetcher, - scoped_ptr > database, - const base::FilePath& database_dir); - virtual ~ImageManager(); + ImageManager() {} + virtual ~ImageManager() {} - virtual void Initialize(const SuggestionsProfile& suggestions); + // (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; - // Should be called from the UI thread. + // 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 callback); - - protected: - // Perform additional tasks when an image has been fetched. - virtual void OnImageFetched(const GURL& url, const SkBitmap* bitmap) OVERRIDE; + base::Callback callback) = 0; 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 > - CallbackVector; - typedef base::hash_map 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 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 callback); - - void ServeFromCacheOrNetwork( - const GURL& url, const GURL& image_url, - base::Callback 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 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 entries); - void OnDatabaseSave(bool success); - - // Take entries from the database and put them in the local cache. - void LoadEntriesInCache(scoped_ptr 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* dest); - - // Map from URL to image URL. Should be kept up to date when a new - // SuggestionsProfile is available. - std::map 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 image_fetcher_; - - scoped_ptr > database_; - - bool database_ready_; - - base::WeakPtrFactory weak_ptr_factory_; - - base::ThreadChecker thread_checker_; - DISALLOW_COPY_AND_ASSIGN(ImageManager); }; diff --git a/components/suggestions/image_manager_unittest.cc b/components/suggestions/image_manager_unittest.cc deleted file mode 100644 index 757b810f52a8..000000000000 --- a/components/suggestions/image_manager_unittest.cc +++ /dev/null @@ -1,193 +0,0 @@ -// 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 - -#include "base/files/file_path.h" -#include "base/run_loop.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 "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 kTestImagePath[] = "files/image_decoding/droids.png"; -const char kInvalidImagePath[] = "files/DOESNOTEXIST"; - -using leveldb_proto::test::FakeDB; -using suggestions::ImageData; -using suggestions::ImageManager; - -typedef base::hash_map EntryMap; - -void AddEntry(const ImageData& d, EntryMap* map) { (*map)[d.url()] = d; } - -class MockImageFetcher : public suggestions::ImageFetcher { - public: - MockImageFetcher() {} - virtual ~MockImageFetcher() {} - MOCK_METHOD3(StartOrQueueNetworkRequest, - void(const GURL&, const GURL&, - base::Callback)); - MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*)); -}; - -class ImageManagerTest : public testing::Test { - public: - ImageManagerTest() - : mock_image_fetcher_(NULL), - num_callback_null_called_(0), - num_callback_valid_called_(0) {} - - virtual void SetUp() OVERRIDE { - fake_db_ = new FakeDB(&db_model_); - image_manager_.reset(CreateImageManager(fake_db_)); - } - - virtual void TearDown() OVERRIDE { - fake_db_ = NULL; - db_model_.clear(); - image_manager_.reset(); - } - - void InitializeDefaultImageMapAndDatabase(ImageManager* image_manager, - FakeDB* fake_db) { - CHECK(image_manager); - CHECK(fake_db); - - suggestions::SuggestionsProfile suggestions_profile; - suggestions::ChromeSuggestion* suggestion = - suggestions_profile.add_suggestions(); - suggestion->set_url(kTestUrl1); - suggestion->set_thumbnail(kTestImagePath); - - image_manager->Initialize(suggestions_profile); - - // Initialize empty database. - fake_db->InitCallback(true); - fake_db->LoadCallback(true); - } - - ImageData GetSampleImageData(const std::string& url) { - // Create test bitmap. - SkBitmap bm; - bm.allocN32Pixels(2, 2); - ImageData data; - data.set_url(url); - std::vector 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) { - if (bitmap) { - num_callback_valid_called_++; - } else { - num_callback_null_called_++; - } - loop->Quit(); - } - - ImageManager* CreateImageManager(FakeDB* fake_db) { - mock_image_fetcher_ = new StrictMock(); - EXPECT_CALL(*mock_image_fetcher_, SetImageFetcherDelegate(_)); - return new ImageManager( - scoped_ptr(mock_image_fetcher_), - scoped_ptr >(fake_db), - FakeDB::DirectoryForTestDB()); - } - - EntryMap db_model_; - // Owned by the ImageManager under test. - FakeDB* fake_db_; - - MockImageFetcher* mock_image_fetcher_; - - int num_callback_null_called_; - int num_callback_valid_called_; - // Under test. - scoped_ptr image_manager_; -}; - -TEST_F(ImageManagerTest, InitializeTest) { - SuggestionsProfile suggestions_profile; - ChromeSuggestion* suggestion = suggestions_profile.add_suggestions(); - suggestion->set_url(kTestUrl1); - suggestion->set_thumbnail(kTestImagePath); - - image_manager_->Initialize(suggestions_profile); - - GURL output; - EXPECT_TRUE(image_manager_->GetImageURL(GURL(kTestUrl1), &output)); - EXPECT_EQ(GURL(kTestImagePath), output); - - EXPECT_FALSE(image_manager_->GetImageURL(GURL("http://b.com"), &output)); -} - -TEST_F(ImageManagerTest, GetImageForURLNetwork) { - InitializeDefaultImageMapAndDatabase(image_manager_.get(), fake_db_); - - // 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; - 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_); -} - -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(kInvalidImagePath); - - // Create the ImageManager with an added entry in the database. - AddEntry(GetSampleImageData(kTestUrl1), &db_model_); - FakeDB* fake_db = new FakeDB(&db_model_); - image_manager_.reset(CreateImageManager(fake_db)); - image_manager_->Initialize(suggestions_profile); - fake_db->InitCallback(true); - fake_db->LoadCallback(true); - // Expect something in the cache. - SkBitmap* bitmap = image_manager_->GetBitmapFromCache(GURL(kTestUrl1)); - EXPECT_FALSE(bitmap->isNull()); - - base::RunLoop 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_); - EXPECT_EQ(1, num_callback_valid_called_); -} - -} // namespace suggestions