Introduce a request throttler for limiting requests in mobile NTP.

components/ntp_snippets/request_counter.cc

Index: components/ntp_snippets/request_counter.cc
diff --git a/components/ntp_snippets/request_counter.cc b/components/ntp_snippets/request_counter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8013fb7ffd4ef67d0cc67dcfce070c608bff8d15
--- /dev/null
+++ b/components/ntp_snippets/request_counter.cc
@@ -0,0 +1,123 @@
+// Copyright 2016 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/request_counter.h"
+
+#include <vector>
+
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/stringprintf.h"
+#include "base/time/time.h"
+#include "components/ntp_snippets/ntp_snippets_constants.h"
+#include "components/ntp_snippets/pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/variations/variations_associated_data.h"
+
+namespace {
+
+const char kQuotaParamPrefix[] = "quota_";
+const char kUMAHistogramTotalFormat[] = "NewTabPage.RequestCounter.%s.Total";
+const char kUMAHistogramPerUserFormat[] =
+    "NewTabPage.RequestCounter.%s.PerUser";
+
+}  // namespace
+
+namespace ntp_snippets {
+
+RequestCounter::RequestCounter(PrefService* pref_service,
+                               const std::string& id,
+                               int default_quota)
+    : pref_service_(pref_service), id_(id), counter_(-1) {
+  DCHECK(!id_.empty());
+
+  std::string quota = variations::GetVariationParamValue(
+      ntp_snippets::kStudyName, kQuotaParamPrefix + id_);
+  if (!base::StringToInt(quota, &quota_)) {

[Marc Treib, 2016/07/19 09:06:17]

Maybe warn if the param is non-empty, but fails to parse?

[jkrcal, 2016/07/20 09:40:40]

Done.

[Marc Treib, 2016/07/20 10:19:05]

This should be LOG, not DCHECK - getting invalid input is not a bug in Chrome :)
(There's LOG_IF for this purpose!)

[jkrcal, 2016/07/20 12:09:27]

Done.

+    quota_ = default_quota;
+  }
+
+  // Create the CamelCase version of |id_|, i.e. "sample_id" -> "SampleId"
+  std::vector<std::string> words = base::SplitString(
+      id_, "_", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  for (std::string& word : words) {
+    word[0] = toupper(word[0]);
+    id_camel_case_ += word;
+  }
+}
+
+// static
+void RequestCounter::RegisterProfilePrefs(PrefRegistrySimple* registry,
+                                          const std::string& id) {
+  registry->RegisterIntegerPref(GetCountPref(id), 0);
+  registry->RegisterIntegerPref(GetDaysPref(id), 0);
+}
+
+bool RequestCounter::IsQuotaAvailable() {
+  ResetCounterIfDayChanged();
+
+  counter_++;
+  pref_service_->SetInteger(GetCountPref(id_), counter_);
+
+  bool available = (counter_ <= quota_);
+  UMA_HISTOGRAM_ENUMERATION(
+      GetTotalHistogramName(),

[Marc Treib, 2016/07/19 09:06:17]

This doesn't work - the UMA_HISTOGRAM macros require that the histogram name is
a constant. Example of what to do instead:
https://cs.chromium.org/chromium/src/components/ntp_tiles/most_visited_sites....

[jkrcal, 2016/07/20 09:40:40]

Done.

+      static_cast<int>(available ? RequestType::QUOTA_GRANTED
+                                 : RequestType::QUOTA_EXCEEDED),
+      static_cast<int>(RequestType::REQUEST_TYPE_MAX));
+
+  return available;
+}
+
+void RequestCounter::ReportForcedRequest() {
+  UMA_HISTOGRAM_ENUMERATION(GetTotalHistogramName(),
+                            static_cast<int>(RequestType::FORCED),
+                            static_cast<int>(RequestType::REQUEST_TYPE_MAX));
+}
+
+void RequestCounter::ResetCounterIfDayChanged() {
+  // Load the counter from pref, if not done yet. Loading is not done in the
+  // constructor to simplify code in unittests.
+  if (counter_ == -1)
+    counter_ = pref_service_->GetInteger(GetCountPref(id_));

[Marc Treib, 2016/07/19 09:06:17]

I assume the problem is that the perf service is null in tests? If so, I'd
prefer to keep the initialization in the ctor, behind an "if (prefs_)".
(Alternatively, IIRC it's not very hard to pass in a fake pref service.)

[jkrcal, 2016/07/20 09:40:39]

Done.

+
+  int now_day =
+      base::TimeDelta::FromMilliseconds(base::Time::Now().ToJavaTime())

[Marc Treib, 2016/07/19 09:06:17]

Why not just Now().InDays()?

[jkrcal, 2016/07/20 09:40:40]

Found something that looks much better to me.

+          .InDays();
+  std::string day_pref_key = GetDaysPref(id_);
+
+  if (!pref_service_->HasPrefPath(day_pref_key)) {
+    // The counter starts from 0 for the first time in this profile.
+    pref_service_->SetInteger(day_pref_key, now_day);
+  } else if (now_day != pref_service_->GetInteger(day_pref_key)) {
+    // Day has changed - report the no of requests from the previous day.
+    UMA_HISTOGRAM_COUNTS_100(GetPerUserHistogramName(), counter_);
+    pref_service_->SetInteger(day_pref_key, now_day);
+    counter_ = 0;
+  }
+}
+
+std::string RequestCounter::GetTotalHistogramName() {
+  return base::StringPrintf(kUMAHistogramTotalFormat, id_camel_case_.c_str());
+}
+
+std::string RequestCounter::GetPerUserHistogramName() {
+  return base::StringPrintf(kUMAHistogramPerUserFormat, id_camel_case_.c_str());
+}
+
+// static
+std::string RequestCounter::GetDaysPref(const std::string& id) {
+  return base::StringPrintf(
+      ntp_snippets::prefs::kSnippetQuotaDayFormat, id.c_str());
+}
+
+// static
+std::string RequestCounter::GetCountPref(const std::string& id) {
+  return base::StringPrintf(
+      ntp_snippets::prefs::kSnippetQuotaCountFormat, id.c_str());
+}
+
+}  // namespace ntp_snippets