Add request throttler to thumbnail fetching for articles on mobile NTP

components/ntp_snippets/ntp_snippets_service.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/ntp_snippets_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
@@ skipping 192 matching lines @@
       suggestions_service_(suggestions_service),
       application_language_code_(application_language_code),
       scheduler_(scheduler),
       snippets_fetcher_(std::move(snippets_fetcher)),
       image_fetcher_(std::move(image_fetcher)),
       image_decoder_(std::move(image_decoder)),
       database_(std::move(database)),
       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;
   }
 
   database_->SetErrorCallback(base::Bind(&NTPSnippetsService::OnDatabaseError,
                                          base::Unretained(this)));
 
   // We transition to other states while finalizing the initialization, when the
   // database is done loading.
   database_->LoadSnippets(base::Bind(&NTPSnippetsService::OnDatabaseLoaded,
                                      base::Unretained(this)));
 }
 
 NTPSnippetsService::~NTPSnippetsService() {
 }
 
 // static
 void NTPSnippetsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kSnippetHosts);
 
   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() {
   // The scheduler only exists on Android so far, it's null on other platforms.
   if (!scheduler_)
     return;
 
   if (ready()) {
     base::Time now = base::Time::Now();
     scheduler_->Schedule(
@@ skipping 360 matching lines @@
 }
 
 void NTPSnippetsService::FetchSnippetImageFromNetwork(
     const std::string& snippet_id,
     const ImageFetchedCallback& callback) {
   auto it =
       std::find_if(snippets_.begin(), snippets_.end(),
                    [&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);
 }
 
 void NTPSnippetsService::EnterStateEnabled(bool fetch_snippets) {
   if (fetch_snippets)
     FetchSnippets(/*force_request=*/false);
 
   // FetchSnippets should set the status to |AVAILABLE_LOADING| if relevant,
   // otherwise we transition to |AVAILABLE| here.
@@ skipping 128 matching lines @@
 void NTPSnippetsService::UpdateCategoryStatus(CategoryStatus status) {
   if (status == category_status_)
     return;
 
   category_status_ = status;
   observer()->OnCategoryStatusChanged(this, provided_category_,
                                       category_status_);
 }
 
 }  // namespace ntp_snippets