Add request throttler to thumbnail fetching for articles on mobile NTP

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index d906cbd13fa7bccba9fc12e2c1bf1c0e3e73d9c3..9b543e4961791775b1a052f35e7ba90acd99fe52 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -210,7 +210,10 @@ NTPSnippetsService::NTPSnippetsService(
       snippets_status_service_(std::move(status_service)),
       fetch_after_load_(false),
       provided_category_(
-          category_factory->FromKnownCategory(KnownCategories::ARTICLES)) {
+          category_factory->FromKnownCategory(KnownCategories::ARTICLES)),
+      thumbnail_requests_throttler_(
+          pref_service,
+          RequestThrottler::RequestType::CONTENT_SUGGESTION_THUMBNAIL) {
   if (database_->IsErrorState()) {
     EnterState(State::ERROR_OCCURRED, CategoryStatus::LOADING_ERROR);
     return;
@@ -235,24 +238,24 @@ void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   NTPSnippetsStatusService::RegisterProfilePrefs(registry);
 }
 
-void NTPSnippetsService::FetchSnippets(bool force_request) {
+void NTPSnippetsService::FetchSnippets(bool interactive_request) {
   if (ready())
-    FetchSnippetsFromHosts(GetSuggestionsHosts(), force_request);
+    FetchSnippetsFromHosts(GetSuggestionsHosts(), interactive_request);
   else
     fetch_after_load_ = true;
 }
 
 void NTPSnippetsService::FetchSnippetsFromHosts(
     const std::set<std::string>& hosts,
-    bool force_request) {
+    bool interactive_request) {
   if (!ready())
     return;
 
   if (snippets_.empty())
     UpdateCategoryStatus(CategoryStatus::AVAILABLE_LOADING);
 
-  snippets_fetcher_->FetchSnippetsFromHosts(hosts, application_language_code_,
-                                            kMaxSnippetCount, force_request);
+  snippets_fetcher_->FetchSnippetsFromHosts(
+      hosts, application_language_code_, kMaxSnippetCount, interactive_request);
 }
 
 void NTPSnippetsService::RescheduleFetching() {
@@ -633,12 +636,21 @@ void NTPSnippetsService::FetchSnippetImageFromNetwork(
                    [&snippet_id](const std::unique_ptr<NTPSnippet>& snippet) {
                      return snippet->id() == snippet_id;
                    });
-  if (it == snippets_.end()) {
-    callback.Run(MakeUniqueID(provided_category_, snippet_id), gfx::Image());
+
+  if (it == snippets_.end() ||
+      !thumbnail_requests_throttler_.DemandQuotaForRequest(
+          /*interactive_request=*/true)) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, it != snippets_.end()

[Marc Treib, 2016/08/09 14:04:53]

I don't understand the snippet_id-building here - why do we make a unique ID
only in one case?

[jkrcal, 2016/08/10 10:27:26]

Huh, I just kept the code equivalent :) This was a bug, actually several bugs. 

We need to pass snippet ID in the Image Fetcher because this is the key that we
use to store the raw data in the DB etc. Eventually, the callback should be
called with the suggestion ID - I added a OnSnippetImageDecodedFromNetwork
wrapper function (and changed the order of parameters for all On* functions to
have them consistent).

[Marc Treib, 2016/08/10 11:38:14]

Interesting!
How did the fetched images ever arrive at the UI before, if we called it back
with the wrong ID?!

Might be worth mentioning this fix in the issue description :)

[jkrcal, 2016/08/10 11:57:03]

I've updated the issue. Interestingly, the UI does not care about the id, this
is thrown away in the bridge (did not check how it then works).

[Marc Treib, 2016/08/10 12:04:59]

Ah, it binds a callback directly to the UI element that wants the image, so it
doesn't need the ID to assign it.

[jkrcal, 2016/08/10 16:22:27]

Acknowledged.

[tschumann, 2016/08/11 11:09:40]

Given that was quite a bug, is there a chance to put a test in place which fails
before this fix and passes afterwards?

+                                 ? snippet_id
+                                 : MakeUniqueID(provided_category_, snippet_id),
+                   gfx::Image()));
     return;
   }
 
   const NTPSnippet& snippet = *it->get();
+
   image_fetcher_->StartOrQueueNetworkRequest(
       snippet.id(), snippet.salient_image_url(), callback);
 }