Refactoring: Moves image fetching and caching into CachedImageFetcher

components/ntp_snippets/remote/remote_suggestions_provider.cc

 // Copyright 2015 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 "components/ntp_snippets/remote/remote_suggestions_provider.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/task_runner_util.h"
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "components/ntp_snippets/features.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
@@ skipping 182 matching lines @@
 void CallWithEmptyResults(const FetchDoneCallback& callback,
                           const Status& status) {
   if (callback.is_null()) {
     return;
   }
   callback.Run(status, std::vector<ContentSuggestion>());
 }
 
 }  // namespace
 
+CachedImageFetcher::CachedImageFetcher(
+    std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
+    std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
+    PrefService* pref_service,
+    RemoteSuggestionsDatabase* database)
+    : image_fetcher_(std::move(image_fetcher)),
+      image_decoder_(std::move(image_decoder)),
+      database_(database),
+      thumbnail_requests_throttler_(
+          pref_service,
+          RequestThrottler::RequestType::CONTENT_SUGGESTION_THUMBNAIL) {
+  // |image_fetcher_| can be null in tests.
+  if (image_fetcher_) {
+    image_fetcher_->SetImageFetcherDelegate(this);
+    image_fetcher_->SetDataUseServiceName(
+        data_use_measurement::DataUseUserData::NTP_SNIPPETS);
+  }
+}
+
+CachedImageFetcher::~CachedImageFetcher() {}
+
+void CachedImageFetcher::FetchSuggestionImage(
+    const ContentSuggestion::ID& suggestion_id,
+    const GURL& url,
+    const ImageFetchedCallback& callback) {
+  database_->LoadImage(
+      suggestion_id.id_within_category(),
+      base::Bind(&CachedImageFetcher::OnSnippetImageFetchedFromDatabase,
+                 base::Unretained(this), callback, suggestion_id, url));
+}
+
+// This function gets only called for caching the image data received from the
+// network. The actual decoding is done in OnSnippetImageDecodedFromDatabase().
+void CachedImageFetcher::OnImageDataFetched(
+    const std::string& id_within_category,
+    const std::string& image_data) {
+  if (image_data.empty()) {
+    return;
+  }
+  database_->SaveImage(id_within_category, image_data);
+}
+
+void CachedImageFetcher::OnImageDecodingDone(
+    const ImageFetchedCallback& callback,
+    const std::string& id_within_category,
+    const gfx::Image& image) {
+  callback.Run(image);
+}
+
+void CachedImageFetcher::OnSnippetImageFetchedFromDatabase(
+    const ImageFetchedCallback& callback,
+    const ContentSuggestion::ID& suggestion_id,
+    const GURL& url,
+    std::string data) {  // SnippetImageCallback requires nonconst reference.
+  // |image_decoder_| is null in tests.
+  if (image_decoder_ && !data.empty()) {
+    image_decoder_->DecodeImage(
+        data, base::Bind(
+                  &CachedImageFetcher::OnSnippetImageDecodedFromDatabase,
+                  base::Unretained(this), callback, suggestion_id, url));
+    return;
+  }
+  // Fetching from the DB failed; start a network fetch.
+  FetchSnippetImageFromNetwork(suggestion_id, url, callback);
+}
+
+void CachedImageFetcher::OnSnippetImageDecodedFromDatabase(
+    const ImageFetchedCallback& callback,
+    const ContentSuggestion::ID& suggestion_id,
+    const GURL& url,
+    const gfx::Image& image) {
+  if (!image.IsEmpty()) {
+    callback.Run(image);
+    return;
+  }
+  // If decoding the image failed, delete the DB entry.
+  database_->DeleteImage(suggestion_id.id_within_category());
+  FetchSnippetImageFromNetwork(suggestion_id, url, callback);
+}
+
+void CachedImageFetcher::FetchSnippetImageFromNetwork(
+    const ContentSuggestion::ID& suggestion_id,
+    const GURL& url,
+    const ImageFetchedCallback& callback) {
+  if (url.is_empty() ||
+      !thumbnail_requests_throttler_.DemandQuotaForRequest(
+          /*interactive_request=*/true)) {
+    // Return an empty image. Directly, this is never synchronous with the
+    // original FetchSuggestionImage() call - an asynchronous database query has
+    // happened in the meantime.
+    callback.Run(gfx::Image());
+    return;
+  }
+
+  image_fetcher_->StartOrQueueNetworkRequest(
+      suggestion_id.id_within_category(), url,
+      base::Bind(&CachedImageFetcher::OnImageDecodingDone,
+                 base::Unretained(this), callback));
+}
+
 RemoteSuggestionsProvider::RemoteSuggestionsProvider(
     Observer* observer,
     CategoryFactory* category_factory,
     PrefService* pref_service,
     const std::string& application_language_code,
     const UserClassifier* user_classifier,
     NTPSnippetsScheduler* scheduler,
     std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
     std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
     std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
     std::unique_ptr<RemoteSuggestionsDatabase> database,
     std::unique_ptr<RemoteSuggestionsStatusService> status_service)
     : ContentSuggestionsProvider(observer, category_factory),
       state_(State::NOT_INITED),
       pref_service_(pref_service),
       articles_category_(
           category_factory->FromKnownCategory(KnownCategories::ARTICLES)),
       application_language_code_(application_language_code),
       user_classifier_(user_classifier),
       scheduler_(scheduler),
       snippets_fetcher_(std::move(snippets_fetcher)),
-      image_fetcher_(std::move(image_fetcher)),
-      image_decoder_(std::move(image_decoder)),
       database_(std::move(database)),
+      image_fetcher_(std::move(image_fetcher),
+                     std::move(image_decoder),
+                     pref_service,
+                     database_.get()),
       status_service_(std::move(status_service)),
       fetch_when_ready_(false),
       nuke_when_initialized_(false),
-      thumbnail_requests_throttler_(
-          pref_service,
-          RequestThrottler::RequestType::CONTENT_SUGGESTION_THUMBNAIL),
       tick_clock_(base::MakeUnique<base::DefaultTickClock>()) {
   pref_service_->ClearPref(kDeprecatedSnippetHostsPref);
 
   RestoreCategoriesFromPrefs();
   // The articles category always exists. Add it if we didn't get it from prefs.
   // TODO(treib): Rethink this.
   category_contents_.insert(
       std::make_pair(articles_category_,
                      CategoryContent(BuildArticleCategoryInfo(base::nullopt))));
   // Tell the observer about all the categories.
@@ skipping 178 matching lines @@
     return;
   }
 
   auto content_it = category_contents_.find(suggestion_id.category());
   DCHECK(content_it != category_contents_.end());
   CategoryContent* content = &content_it->second;
   DismissSuggestionFromCategoryContent(content,
                                        suggestion_id.id_within_category());
 }
 
-void RemoteSuggestionsProvider::FetchSuggestionImage(
-    const ContentSuggestion::ID& suggestion_id,
-    const ImageFetchedCallback& callback) {
-  database_->LoadImage(
-      suggestion_id.id_within_category(),
-      base::Bind(&RemoteSuggestionsProvider::OnSnippetImageFetchedFromDatabase,
-                 base::Unretained(this), callback, suggestion_id));
-}
-
 void RemoteSuggestionsProvider::ClearHistory(
     base::Time begin,
     base::Time end,
     const base::Callback<bool(const GURL& url)>& filter) {
   // Both time range and the filter are ignored and all suggestions are removed,
   // because it is not known which history entries were used for the suggestions
   // personalization.
   if (!ready()) {
     nuke_when_initialized_ = true;
   } else {
@@ skipping 78 matching lines @@
   const CategoryContent& content =
       category_contents_.at(suggestion_id.category());
   const NTPSnippet* snippet =
       content.FindSnippet(suggestion_id.id_within_category());
   if (!snippet) {
     return GURL();
   }
   return snippet->salient_image_url();
 }
 
-// image_fetcher::ImageFetcherDelegate implementation.
-void RemoteSuggestionsProvider::OnImageDataFetched(
-    const std::string& id_within_category,
-    const std::string& image_data) {
-  if (image_data.empty()) {
-    return;
-  }
-
-  // Only save the image if the corresponding snippet still exists.
-  bool found = false;
-  for (const auto& entry : category_contents_) {
-    if (entry.second.FindSnippet(id_within_category)) {
-      found = true;
-      break;
-    }
-  }
-  if (!found) {
-    return;
-  }
-
-  // Only cache the data in the DB, the actual serving is done in the callback
-  // provided to |image_fetcher_| (OnSnippetImageDecodedFromNetwork()).
-  database_->SaveImage(id_within_category, image_data);
-}
-
 void RemoteSuggestionsProvider::OnDatabaseLoaded(
     NTPSnippet::PtrVector snippets) {
   if (state_ == State::ERROR_OCCURRED) {
     return;
   }
   DCHECK(state_ == State::NOT_INITED);
   DCHECK(base::ContainsKey(category_contents_, articles_category_));
 
   base::TimeDelta database_load_time =
       base::TimeTicks::Now() - database_load_start_;
@@ skipping 345 matching lines @@
     }
   }
 
   for (Category category : categories_to_erase) {
     category_contents_.erase(category);
   }
 
   StoreCategoriesToPrefs();
 }
 
-void RemoteSuggestionsProvider::OnSnippetImageFetchedFromDatabase(
-    const ImageFetchedCallback& callback,
-    const ContentSuggestion::ID& suggestion_id,
-    std::string data) {  // SnippetImageCallback requires nonconst reference.
-  // |image_decoder_| is null in tests.
-  if (image_decoder_ && !data.empty()) {
-    image_decoder_->DecodeImage(
-        data, base::Bind(
-                  &RemoteSuggestionsProvider::OnSnippetImageDecodedFromDatabase,
-                  base::Unretained(this), callback, suggestion_id));
-    return;
-  }
-
-  // Fetching from the DB failed; start a network fetch.
-  FetchSnippetImageFromNetwork(suggestion_id, callback);
-}
-
-void RemoteSuggestionsProvider::OnSnippetImageDecodedFromDatabase(
-    const ImageFetchedCallback& callback,
-    const ContentSuggestion::ID& suggestion_id,
-    const gfx::Image& image) {
-  if (!image.IsEmpty()) {
-    callback.Run(image);
-    return;
-  }
-
-  // If decoding the image failed, delete the DB entry.
-  database_->DeleteImage(suggestion_id.id_within_category());
-
-  FetchSnippetImageFromNetwork(suggestion_id, callback);
-}
-
-void RemoteSuggestionsProvider::FetchSnippetImageFromNetwork(
+void RemoteSuggestionsProvider::FetchSuggestionImage(
     const ContentSuggestion::ID& suggestion_id,
     const ImageFetchedCallback& callback) {
   if (!base::ContainsKey(category_contents_, suggestion_id.category())) {
-    OnSnippetImageDecodedFromNetwork(
-        callback, suggestion_id.id_within_category(), gfx::Image());
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+     FROM_HERE, base::Bind(callback, gfx::Image()));

[Marc Treib, 2016/12/14 09:02:15]

nit: Wrong indentation.

     return;
   }
-
   GURL image_url = FindSnippetImageUrl(suggestion_id);
-
-  if (image_url.is_empty() ||
-      !thumbnail_requests_throttler_.DemandQuotaForRequest(
-          /*interactive_request=*/true)) {
-    // Return an empty image. Directly, this is never synchronous with the
-    // original FetchSuggestionImage() call - an asynchronous database query has
-    // happened in the meantime.
-    OnSnippetImageDecodedFromNetwork(
-        callback, suggestion_id.id_within_category(), gfx::Image());
+  if (image_url.is_empty()) {
+    // As we don't know the corresponding snippet anymore, we don't expect to
+    // find it in the database (and also can't fetch it remotely). Cut the
+    // lookup short and return directly.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(callback, gfx::Image()));
     return;
   }
-
-  image_fetcher_->StartOrQueueNetworkRequest(
-      suggestion_id.id_within_category(), image_url,
-      base::Bind(&RemoteSuggestionsProvider::OnSnippetImageDecodedFromNetwork,
-                 base::Unretained(this), callback));
-}
-
-void RemoteSuggestionsProvider::OnSnippetImageDecodedFromNetwork(
-    const ImageFetchedCallback& callback,
-    const std::string& id_within_category,
-    const gfx::Image& image) {
-  callback.Run(image);
+  image_fetcher_.FetchSuggestionImage(suggestion_id, image_url, callback);
 }
 
 void RemoteSuggestionsProvider::EnterStateReady() {
   if (nuke_when_initialized_) {
     NukeAllSnippets();
     nuke_when_initialized_ = false;
   }
 
   auto article_category_it = category_contents_.find(articles_category_);
   DCHECK(article_category_it != category_contents_.end());
@@ skipping 28 matching lines @@
 }
 
 void RemoteSuggestionsProvider::FinishInitialization() {
   if (nuke_when_initialized_) {
     // We nuke here in addition to EnterStateReady, so that it happens even if
     // we enter the DISABLED state below.
     NukeAllSnippets();
     nuke_when_initialized_ = false;
   }
 
-  // |image_fetcher_| can be null in tests.
-  if (image_fetcher_) {
-    image_fetcher_->SetImageFetcherDelegate(this);
-    image_fetcher_->SetDataUseServiceName(
-        data_use_measurement::DataUseUserData::NTP_SNIPPETS);
-  }
-
   // Note: Initializing the status service will run the callback right away with
   // the current state.
   status_service_->Init(base::Bind(&RemoteSuggestionsProvider::OnStatusChanged,
                                    base::Unretained(this)));
 
   // Always notify here even if we got nothing from the database, because we
   // don't know how long the fetch will take or if it will even complete.
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
@@ skipping 266 matching lines @@
 RemoteSuggestionsProvider::CategoryContent::CategoryContent(CategoryContent&&) =
     default;
 
 RemoteSuggestionsProvider::CategoryContent::~CategoryContent() = default;
 
 RemoteSuggestionsProvider::CategoryContent&
 RemoteSuggestionsProvider::CategoryContent::operator=(CategoryContent&&) =
     default;
 
 }  // namespace ntp_snippets