Introduce a request throttler for limiting requests in mobile NTP.

components/ntp_snippets/request_counter.h

+// 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.
+
+#ifndef COMPONENTS_NTP_SNIPPETS_REQUEST_COUNTER_H_
+#define COMPONENTS_NTP_SNIPPETS_REQUEST_COUNTER_H_
+
+#include <string>
+
+#include "base/macros.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace ntp_snippets {
+
+// Counts requests to external services, compares them to a daily quota, reports
+// them to UMA. In the application code, create one local instance for each type
+// of requests, identified by the |id| string. The request counter is based on:
+//  - daily quota from a variation param "quota_|id|" in the NTPSnippets trial;
+//  - prefs "ntp.requests_quota.|id|.day" and "ntp.requests_quota.|id|.count".
+// Furthermore the counter reports
+//  - all requests to the "NewTabPage.RequestsTotal.|Id|" histogram
+//  - the daily number of requests per user in the
+//  "NewTabPage.RequestsPerUser.|Id|" histogram

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

RequestsPerUser and RequestTotal would only be different if there are multiple
profiles?
Also, these don't match the actual UMA names in the .cc file.

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

Clarified.

+//  (note that your "example_id" is autocapitalized to "ExampleId")

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

Why?

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

Removed.

+// Thus, when you add an application of RequestCounter, you must also:
+//  - add a static function that calls RequestCounter::RegisterPrefs with the
+//  same |id| and call this function from browser_prefs.cc.
+//  - define the two histograms in histogram.xml (see previous applications).

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

Hmmmm, I don't really like this. So, there's a central list of "allowed" IDs
(histograms.xml), but the actual names in code are scattered all over the place,
and even re-formatted automatically. That will make it super hard to search for
"where is this histogram coming from".
Alternative suggestion: Have a central list (enum) of counters here, similar to
what components/data_use_measurement does:
https://cs.chromium.org/chromium/src/components/data_use_measurement/core/dat...

WDYT?

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

histograms.xml has a concept of "histogram suffixes", would that work here? (I
think the "suffix" can also be in the middle of the name)

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

Done.

+class RequestCounter {
+ public:
+  // Enumeration listing all possible outcomes for fetch attempts. Used for UMA
+  // histogram, so do not change existing values. Insert new values at the end,
+  // and update the histogram definition.
+  enum class RequestType {
+    FORCED,
+    QUOTA_GRANTED,
+    QUOTA_EXCEEDED,
+    REQUEST_TYPE_MAX
+  };
+
+  // The |id| passed in here must be unique application-wide.
+  RequestCounter(PrefService* pref_service,
+                 const std::string& id,
+                 int default_quota);
+
+  // Must be called with the same |id| as used in the constructor before any
+  // calls to IsQuotaAvailable.
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry,
+                                   const std::string& id);
+
+  // Returns whether quota is available for another request and reports this
+  // information to UMA.
+  bool IsQuotaAvailable();

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

The name sounds like this is a pure getter that doesn't change anything. Maybe
"RequestQuota", to make clearer what actually happens? (Though "Request" is also
not that clear...)

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

Changed to GetQuota() which avoids overloading the "Request" word.

+
+  // Reports a forced request to UMA (without checking the quota).
+  void ReportForcedRequest();
+
+ private:
+  PrefService* pref_service_;
+  std::string id_;
+  std::string id_camel_case_;
+
+  int counter_;
+  int quota_;
+
+  void ResetCounterIfDayChanged();

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

Methods come before data members.

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

Done.

+
+  static std::string GetDaysPref(const std::string& id);
+  static std::string GetCountPref(const std::string& id);
+
+  std::string GetTotalHistogramName();
+  std::string GetPerUserHistogramName();
+
+  DISALLOW_COPY_AND_ASSIGN(RequestCounter);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_REQUEST_COUNTER_H_