Introduce a request throttler for limiting requests in mobile NTP.

components/ntp_snippets/request_counter.h

Index: components/ntp_snippets/request_counter.h
diff --git a/components/ntp_snippets/request_counter.h b/components/ntp_snippets/request_counter.h
new file mode 100644
index 0000000000000000000000000000000000000000..20115e342ef4e45c01f8c508eb8ba04afef0b34c
--- /dev/null
+++ b/components/ntp_snippets/request_counter.h
@@ -0,0 +1,90 @@
+// 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 base {
+class HistogramBase;
+}  // namespace base
+
+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 RequestType. The request counter is based on:
+//  - daily quota from a variation param "quota_|type|" in the NTPSnippets trial
+//  - pref "ntp.requests_quota.|type|.count" to store the current counter value,
+//  - pref "ntp.requests_quota.|type|.day" to store current day to which the
+//    current counter value applies.
+// Furthermore the counter reports to histograms:
+//  - "NewTabPage.RequestCounter.Total_|type|" - status of each request;
+//  - "NewTabPage.RequestCounter.PerDay_|type|" - the daily number of requests.
+// Thus, when you add a new RequestType of RequestCounter, you must also:
+//  1) define the string value for your |type| in GetRequestTypeAsString(),
+//  2) add constants for days/count prefs, add them to GetCountPref() and
+//     GetDayPref() functions, and register them in RegisterProfilePrefs(),
+//  3) define a new RequestCounterTypes histogram suffix in histogram.xml (with
+//     the same string value as in 1)).
+class RequestCounter {
+ public:
+  // Enumeration listing all current applications of the request counter. Not
+  // used for uma.
+  enum class RequestType {
+    ARTICLE_CONTENT_FETCHER
+  };
+
+  // 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 RequestStatus {

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

I think if you add " : int" here (before the "{"), then you might not need all
the static_casts?

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

No, it does not compile without the static cast.

[Marc Treib, 2016/07/20 12:58:27]

Alright :(  Then no need to add the ": int" I guess

+    FORCED,
+    QUOTA_GRANTED,
+    QUOTA_EXCEEDED,
+    REQUEST_STATUS_MAX

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

This is a COUNT, not a MAX.

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

Done.

+  };
+
+  RequestCounter(PrefService* pref_service,
+                 RequestCounter::RequestType type,

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

nit: "RequestCounter::" not needed

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

Done.

+                 int default_quota);
+
+  // Register profile prefs for all RequestTypes.
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
+  // Returns whether quota is available for another request and reports this
+  // information to UMA.
+  bool GetQuota();

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

Now it sounds like "tell me the quota"...

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

Huh, you are right. I've forgotten about |quota_|.
What about Demand?

[Marc Treib, 2016/07/20 12:58:27]

Yeah, this is much better, thanks!
Maybe add "ForRequest", for some sort of consistency with ReportForcedRequest?

+
+  // Reports a forced request to UMA (without checking the quota).
+  void ReportForcedRequest() const;
+
+ private:
+  void ResetCounterIfDayChanged();
+
+  std::string GetRequestTypeAsString() const;
+  std::string GetDaysPref() const;
+  std::string GetCountPref() const;
+
+  PrefService* pref_service_;
+  RequestCounter::RequestType type_;

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

const, to make it clear?

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

Done.

+
+  int counter_;

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

Do you need to store this? Might as well just make a getter that reads the pref.
Also I'd like a more unambiguous name - request_count_today_ or something like
that?

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

Done.

+  int quota_;
+
+  // The histogram for reporting all requests of the given |type_|.
+  base::HistogramBase* histogram_total_;

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

If you do this for one histogram, then you should do it for other one too IMO,
for consistency.

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

This histogram is used in each request and on two different places in code.
The other is used at most once per day and only on one place in code.
Do you insist on consistency, here?

[Marc Treib, 2016/07/20 12:58:27]

Ah, so it's for performance reasons? Okay, I guess...
I'd still prefer consistency, because I like consistency and it doesn't really
hurt either IMO, but I won't insist.

+
+  DISALLOW_COPY_AND_ASSIGN(RequestCounter);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_REQUEST_COUNTER_H_