Add throttling to permission reporter

chrome/browser/safe_browsing/permission_reporter_unittest.cc

 // 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 "chrome/browser/safe_browsing/permission_reporter.h"
 
 #include "base/memory/ptr_util.h"
+#include "base/test/simple_test_clock.h"
+#include "base/time/time.h"
 #include "chrome/common/safe_browsing/permission_report.pb.h"
 #include "content/public/browser/permission_type.h"
 #include "net/url_request/report_sender.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::PermissionType;
 
 namespace safe_browsing {
 
 namespace {
 // URL to upload permission action reports.
 const char kPermissionActionReportingUploadUrl[] =
     "http://safebrowsing.googleusercontent.com/safebrowsing/clientreport/"
     "permission-action";
 
-const char kDummyOrigin[] = "http://example.test/";
-const PermissionType kDummyPermission = PermissionType::GEOLOCATION;
+const int kMaximumReportsPerOriginPerPermissionPerMinute = 5;
+
+const char kDummyOriginOne[] = "http://example.test/";
+const char kDummyOriginTwo[] = "http://example2.test/";
+const PermissionType kDummyPermissionOne = PermissionType::GEOLOCATION;
+const PermissionType kDummyPermissionTwo = PermissionType::NOTIFICATIONS;
 const PermissionAction kDummyAction = GRANTED;
-const PermissionReport::PermissionType kDummyPermissionReportPermission =
+const PermissionReport::PermissionType kDummyPermissionReportPermissionOne =

[raymes, 2016/06/29 07:09:17]

nit: maybe we just inline this? Sometimes it's a bit easier to read than having
a bunch of constants all defined at the top.

[stefanocs, 2016/06/30 00:24:51]

Done.

     PermissionReport::GEOLOCATION;
 const PermissionReport::Action kDummyPermissionReportAction =

[raymes, 2016/06/29 07:09:17]

nit: same here.

[stefanocs, 2016/06/30 00:24:51]

Done.

     PermissionReport::GRANTED;
 
 // A mock ReportSender that keeps track of the last report sent.
 class MockReportSender : public net::ReportSender {
  public:
   MockReportSender() : net::ReportSender(nullptr, DO_NOT_SEND_COOKIES) {}
   ~MockReportSender() override {}
 
   void Send(const GURL& report_uri, const std::string& report) override {
     latest_report_uri_ = report_uri;
     latest_report_ = report;
   }
 
   const GURL& latest_report_uri() { return latest_report_uri_; }
 
   const std::string& latest_report() { return latest_report_; }
 
  private:
   GURL latest_report_uri_;
   std::string latest_report_;
 
   DISALLOW_COPY_AND_ASSIGN(MockReportSender);
 };
 
 }  // namespace
 
 class PermissionReporterTest : public ::testing::Test {
  protected:
   PermissionReporterTest()
-      : mock_report_sender_(new MockReportSender()),
+      : mock_report_sender_(new MockReportSender),
+        clock_(new base::SimpleTestClock),
         permission_reporter_(
-            new PermissionReporter(base::WrapUnique(mock_report_sender_))) {}
+            new PermissionReporter(base::WrapUnique(mock_report_sender_),
+                                   base::WrapUnique(clock_))) {}
+
+  bool IsAllowedToSend(content::PermissionType permission, const GURL& origin) {
+    return permission_reporter_->IsAllowedToSend(permission, origin);
+  }
 
   // Owned by |permission_reporter_|.
   MockReportSender* mock_report_sender_;
 
+  // Owned by |permission_reporter_|.
+  base::SimpleTestClock* clock_;
+
   std::unique_ptr<PermissionReporter> permission_reporter_;
 };
 
 // Test that PermissionReporter::SendReport sends a serialized report string to
 // SafeBrowsing CSD servers.
 TEST_F(PermissionReporterTest, SendReport) {
-  permission_reporter_->SendReport(GURL(kDummyOrigin), kDummyPermission,
+  permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
                                    kDummyAction);
 
   PermissionReport permission_report;
   ASSERT_TRUE(
       permission_report.ParseFromString(mock_report_sender_->latest_report()));
-  EXPECT_EQ(kDummyPermissionReportPermission, permission_report.permission());
+  EXPECT_EQ(kDummyPermissionReportPermissionOne,
+            permission_report.permission());
   EXPECT_EQ(kDummyPermissionReportAction, permission_report.action());
-  EXPECT_EQ(kDummyOrigin, permission_report.origin());
+  EXPECT_EQ(kDummyOriginOne, permission_report.origin());
 
   EXPECT_EQ(GURL(kPermissionActionReportingUploadUrl),
             mock_report_sender_->latest_report_uri());
 }
 
+// Test that PermissionReporter::IsAllowedToSend returns true only when the
+// number of reports sent in the last one minute per origin per permission is
+// under a threshold.
+TEST_F(PermissionReporterTest, IsAllowedToSend) {
+  EXPECT_TRUE(IsAllowedToSend(kDummyPermissionOne, GURL(kDummyOriginOne)));
+
+  int reports_to_send = kMaximumReportsPerOriginPerPermissionPerMinute;
+  while (reports_to_send--)
+    permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+                                     kDummyAction);
+
+  EXPECT_FALSE(IsAllowedToSend(kDummyPermissionOne, GURL(kDummyOriginOne)));
+  EXPECT_TRUE(IsAllowedToSend(kDummyPermissionTwo, GURL(kDummyOriginOne)));
+  EXPECT_TRUE(IsAllowedToSend(kDummyPermissionOne, GURL(kDummyOriginTwo)));
+
+  clock_->Advance(base::TimeDelta::FromMinutes(1));
+  EXPECT_FALSE(IsAllowedToSend(kDummyPermissionOne, GURL(kDummyOriginOne)));
+
+  clock_->Advance(base::TimeDelta::FromMicroseconds(1));
+  EXPECT_TRUE(IsAllowedToSend(kDummyPermissionOne, GURL(kDummyOriginOne)));

[raymes, 2016/06/29 07:09:17]

Instead of calling IsAllowedToSend directly, I think it would be better to test
the public interface by just sending reports. We can use the MockReportSender to
check if the reports actually got sent (we don't need to check their contents
though).

[stefanocs, 2016/06/30 00:24:51]

Done.

+}
+
 }  // namespace safe_browsing