Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2793)

Unified Diff: chrome/browser/safe_browsing/permission_reporter_unittest.cc

Issue 2089383005: Add throttling to permission reporter (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@add-hooks-to-permission-layer
Patch Set: Resolve comments Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/safe_browsing/permission_reporter_unittest.cc
diff --git a/chrome/browser/safe_browsing/permission_reporter_unittest.cc b/chrome/browser/safe_browsing/permission_reporter_unittest.cc
index ddb88731cf09d545bfc39415fabc71bcb4e40666..e34149568041e56cf2c45c96b7285cad7e175b51 100644
--- a/chrome/browser/safe_browsing/permission_reporter_unittest.cc
+++ b/chrome/browser/safe_browsing/permission_reporter_unittest.cc
@@ -7,6 +7,8 @@
#include "base/feature_list.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/field_trial.h"
+#include "base/test/simple_test_clock.h"
+#include "base/time/time.h"
#include "chrome/common/safe_browsing/permission_report.pb.h"
#include "components/variations/active_field_trials.h"
#include "content/public/browser/permission_type.h"
@@ -24,13 +26,13 @@ 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 =
- PermissionReport::GEOLOCATION;
-const PermissionReport::Action kDummyPermissionReportAction =
- PermissionReport::GRANTED;
const char kDummyTrialOne[] = "trial one";
const char kDummyGroupOne[] = "group one";
@@ -50,21 +52,32 @@ struct base::Feature kFeatureOffByDefault {
// 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() : net::ReportSender(nullptr, DO_NOT_SEND_COOKIES) {
+ number_of_reports_ = 0;
+ }
+
~MockReportSender() override {}
void Send(const GURL& report_uri, const std::string& report) override {
latest_report_uri_ = report_uri;
latest_report_ = report;
+ number_of_reports_++;
}
const GURL& latest_report_uri() { return latest_report_uri_; }
const std::string& latest_report() { return latest_report_; }
+ int GetAndResetNumberReports() {
kcarattini 2016/07/06 06:28:20 How about "GetAndResetNumberOfReportsSent"?
stefanocs 2016/07/06 07:17:38 Done.
+ int new_reports = number_of_reports_;
+ number_of_reports_ = 0;
+ return new_reports;
+ }
+
private:
GURL latest_report_uri_;
std::string latest_report_;
+ int number_of_reports_;
DISALLOW_COPY_AND_ASSIGN(MockReportSender);
};
@@ -73,29 +86,34 @@ class MockReportSender : public net::ReportSender {
class PermissionReporterTest : public ::testing::Test {
protected:
- PermissionReporterTest()
- : mock_report_sender_(new MockReportSender()),
- permission_reporter_(
- new PermissionReporter(base::WrapUnique(mock_report_sender_))) {}
+ void SetUp() override {
+ mock_report_sender_ = new MockReportSender;
+ clock_ = new base::SimpleTestClock;
+ permission_reporter_.reset(new PermissionReporter(
+ base::WrapUnique(mock_report_sender_), base::WrapUnique(clock_)));
+ }
// 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(kDummyPermissionReportAction, permission_report.action());
- EXPECT_EQ(kDummyOrigin, permission_report.origin());
+ EXPECT_EQ(PermissionReport::GEOLOCATION, permission_report.permission());
+ EXPECT_EQ(PermissionReport::GRANTED, permission_report.action());
+ EXPECT_EQ(kDummyOriginOne, permission_report.origin());
#if defined(OS_ANDROID)
EXPECT_EQ(PermissionReport::ANDROID_PLATFORM,
permission_report.platform_type());
@@ -140,7 +158,7 @@ TEST_F(PermissionReporterTest, SendReportWithFieldTrials) {
EXPECT_TRUE(base::FieldTrialList::IsTrialActive(trial_one->trial_name()));
EXPECT_TRUE(base::FieldTrialList::IsTrialActive(trial_two->trial_name()));
- permission_reporter_->SendReport(GURL(kDummyOrigin), kDummyPermission,
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
kDummyAction);
PermissionReport permission_report;
@@ -162,4 +180,49 @@ TEST_F(PermissionReporterTest, SendReportWithFieldTrials) {
EXPECT_EQ(0U, expected_group_ids.size());
}
+// 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_EQ(0, mock_report_sender_->GetAndResetNumberReports());
+
+ int reports_to_send = kMaximumReportsPerOriginPerPermissionPerMinute;
+ while (reports_to_send--)
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+ kDummyAction);
+ EXPECT_EQ(5, mock_report_sender_->GetAndResetNumberReports());
+
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+ kDummyAction);
+ EXPECT_EQ(0, mock_report_sender_->GetAndResetNumberReports());
+
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionTwo,
+ kDummyAction);
+ EXPECT_EQ(1, mock_report_sender_->GetAndResetNumberReports());
+
+ permission_reporter_->SendReport(GURL(kDummyOriginTwo), kDummyPermissionOne,
+ kDummyAction);
+ EXPECT_EQ(1, mock_report_sender_->GetAndResetNumberReports());
+
+ clock_->Advance(base::TimeDelta::FromMinutes(1));
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+ kDummyAction);
+ EXPECT_EQ(0, mock_report_sender_->GetAndResetNumberReports());
+
+ clock_->Advance(base::TimeDelta::FromMicroseconds(1));
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+ kDummyAction);
+ EXPECT_EQ(1, mock_report_sender_->GetAndResetNumberReports());
+
+ clock_->Advance(base::TimeDelta::FromMinutes(1));
+ reports_to_send = 12;
+ while (reports_to_send--) {
+ clock_->Advance(base::TimeDelta::FromSeconds(5));
+ permission_reporter_->SendReport(GURL(kDummyOriginOne), kDummyPermissionOne,
+ kDummyAction);
+ }
+ EXPECT_EQ(kMaximumReportsPerOriginPerPermissionPerMinute,
+ mock_report_sender_->GetAndResetNumberReports());
+}
+
} // namespace safe_browsing

Powered by Google App Engine
This is Rietveld 408576698