Add request throttler to thumbnail fetching for articles on mobile NTP

components/ntp_snippets/request_throttler.cc

Index: components/ntp_snippets/request_throttler.cc
diff --git a/components/ntp_snippets/request_throttler.cc b/components/ntp_snippets/request_throttler.cc
index 0a6388169265ec3fba373d7546b2143bde812b67..5d6e5ca32638d492484c781e8388584b959bae77 100644
--- a/components/ntp_snippets/request_throttler.cc
+++ b/components/ntp_snippets/request_throttler.cc
@@ -31,13 +31,18 @@ enum class RequestStatus {
   REQUEST_STATUS_COUNT
 };
 
+// Quota value to use if no quota should be applied (by default).
+const int kUnlimitedQuota = INT_MAX;
+
 }  // namespace
 
 struct RequestThrottler::RequestTypeInfo {
     const char* name;
     const char* count_pref;
+    const char* interactive_count_pref;
     const char* day_pref;
     const int default_quota;
+    const int default_interactive_quota;
 };
 
 // When adding a new type here, extend also the "RequestCounterTypes"
@@ -45,7 +50,11 @@ struct RequestThrottler::RequestTypeInfo {
 const RequestThrottler::RequestTypeInfo RequestThrottler::kRequestTypeInfo[] = {
     // RequestCounter::RequestType::CONTENT_SUGGESTION_FETCHER,
     {"SuggestionFetcher", prefs::kSnippetFetcherQuotaCount,
-     prefs::kSnippetFetcherQuotaDay, 50}};
+     prefs::kSnippetFetcherInteractiveQuotaCount,
+     prefs::kSnippetFetcherQuotaDay, 50, kUnlimitedQuota},
+    {"SuggestionThumbnailFetcher", prefs::kSnippetThumbnailsQuotaCount,
+     prefs::kSnippetThumbnailsInteractiveQuotaCount,
+     prefs::kSnippetThumbnailsQuotaDay, kUnlimitedQuota, kUnlimitedQuota}};
 
 RequestThrottler::RequestThrottler(PrefService* pref_service, RequestType type)
     : pref_service_(pref_service),
@@ -62,6 +71,16 @@ RequestThrottler::RequestThrottler(PrefService* pref_service, RequestType type)
     quota_ = type_info_.default_quota;
   }
 
+  std::string interactive_quota = variations::GetVariationParamValue(
+      ntp_snippets::kStudyName,
+      base::StringPrintf("interactive_quota_%s", GetRequestTypeAsString()));
+  if (!base::StringToInt(interactive_quota, &interactive_quota_)) {
+    LOG_IF(WARNING, !interactive_quota.empty())
+        << "Invalid variation parameter for interactive quota for "
+        << GetRequestTypeAsString();
+    interactive_quota_ = type_info_.default_interactive_quota;
+  }
+
   // Since the histogram names are dynamic, we cannot use the standard macros
   // and we need to lookup the histograms, instead.
   int status_count = static_cast<int>(RequestStatus::REQUEST_STATUS_COUNT);
@@ -82,25 +101,27 @@ RequestThrottler::RequestThrottler(PrefService* pref_service, RequestType type)
 void RequestThrottler::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   for (const RequestTypeInfo& info : kRequestTypeInfo) {
     registry->RegisterIntegerPref(info.count_pref, 0);
+    registry->RegisterIntegerPref(info.interactive_count_pref, 0);
     registry->RegisterIntegerPref(info.day_pref, 0);
   }
 }
 
-bool RequestThrottler::DemandQuotaForRequest(bool forced_request) {
+bool RequestThrottler::DemandQuotaForRequest(bool interactive_request) {
   ResetCounterIfDayChanged();
 
-  if (forced_request) {
+  int count = GetCount(interactive_request);
+  if (count < INT_MAX)

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

Do we ever expect to actually hit INT_MAX?! That would correspond to about 46
requests per millisecond.

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

Okay, good point.

+    count++;
+  SetCount(interactive_request, count);
+  bool available = (count <= GetQuota(interactive_request));
+
+  if (interactive_request) {

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

if (interactive_request && available) ?
Or maybe we should just add a new histogram bin? Currently this is a bit
inconsistent...

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

Done. I tried to avoid touching the histograms but it is better to do it :)

     histogram_request_status_->Add(static_cast<int>(RequestStatus::FORCED));
-    return true;
+  } else {
+    histogram_request_status_->Add(
+        static_cast<int>(available ? RequestStatus::QUOTA_GRANTED
+                                   : RequestStatus::QUOTA_EXCEEDED));
   }
-
-  int new_count = GetCount() + 1;
-  SetCount(new_count);
-  bool available = (new_count <= quota_);
-
-  histogram_request_status_->Add(
-      static_cast<int>(available ? RequestStatus::QUOTA_GRANTED
-                                 : RequestStatus::QUOTA_EXCEEDED));
   return available;
 }
 
@@ -112,10 +133,12 @@ void RequestThrottler::ResetCounterIfDayChanged() {
     // The counter is used for the first time in this profile.
     SetDay(now_day);
   } else if (now_day != GetDay()) {
-    // Day has changed - report the number of requests from the previous day.
-    histogram_per_day_->Add(GetCount());
+    // Day has changed - report the number of background requests from the
+    // previous day.

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

Hm. Should we also report the number of interactive requests?

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

Done.

+    histogram_per_day_->Add(GetCount(/*interactive_request=*/false));
     // Reset the counter.
-    SetCount(0);
+    SetCount(/*interactive_request=*/false, 0);
+    SetCount(/*interactive_request=*/true, 0);
     SetDay(now_day);
   }
 }
@@ -124,12 +147,21 @@ const char* RequestThrottler::GetRequestTypeAsString() const {
   return type_info_.name;
 }
 
-int RequestThrottler::GetCount() const {
-  return pref_service_->GetInteger(type_info_.count_pref);
+int RequestThrottler::GetQuota(bool interactive_request) const {
+  return interactive_request ? interactive_quota_ : quota_;
+}
+
+int RequestThrottler::GetCount(bool interactive_request) const {
+  return interactive_request
+             ? pref_service_->GetInteger(type_info_.interactive_count_pref)
+             : pref_service_->GetInteger(type_info_.count_pref);

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

You could move the ? into the GetInteger()

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

Done.

 }
 
-void RequestThrottler::SetCount(int count) {
-  pref_service_->SetInteger(type_info_.count_pref, count);
+void RequestThrottler::SetCount(bool interactive_request, int count) {
+  pref_service_->SetInteger(interactive_request
+                                ? type_info_.interactive_count_pref
+                                : type_info_.count_pref,
+                            count);
 }
 
 int RequestThrottler::GetDay() const {