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

Unified Diff: chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc

Issue 2678253002: Add metrics for Safe Browsing blacklist response. (Closed)
Patch Set: Fixing diff. Created 3 years, 10 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/permissions/permission_decision_auto_blocker_unittest.cc
diff --git a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
index e1ddba59ba1235f5fb21aaeaf13f7feb2ea5a214..8f64ca6475a32c5b8e6923ede91cb591110b1f12 100644
--- a/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
+++ b/chrome/browser/permissions/permission_decision_auto_blocker_unittest.cc
@@ -8,9 +8,11 @@
#include "base/bind.h"
#include "base/run_loop.h"
+#include "base/test/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_clock.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/permissions/permission_uma_util.h"
dominickn 2017/02/07 05:36:54 Not sure if this is used?
meredithl 2017/02/07 06:27:29 The enums for SafeBrowsingResponse are defined in
dominickn 2017/02/07 22:28:25 Acknowledged.
#include "chrome/browser/permissions/permission_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -254,6 +256,7 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, RemoveCountsByUrl) {
// Test that an origin that has been blacklisted for a permission is embargoed.
TEST_F(PermissionDecisionAutoBlockerUnitTest, TestUpdateEmbargoBlacklist) {
GURL url("https://www.google.com");
+ base::HistogramTester histograms;
scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
new MockSafeBrowsingDatabaseManager(true /* perform_callback */);
@@ -264,6 +267,8 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestUpdateEmbargoBlacklist) {
UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url);
EXPECT_TRUE(last_embargoed_status());
+ histograms.ExpectUniqueSample("Permissions.AutoBlocker.SafeBrowsingResponse",
+ SafeBrowsingResponse::BLACKLISTED, 1);
}
// Check that IsUnderEmbargo returns the correct value when the embargo is set
@@ -312,6 +317,7 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, CheckEmbargoStatus) {
TEST_F(PermissionDecisionAutoBlockerUnitTest, TestDismissEmbargoBackoff) {
GURL url("https://www.google.com");
clock()->SetNow(base::Time::Now());
+ base::HistogramTester histograms;
dominickn 2017/02/07 05:36:54 This is currently unused?
meredithl 2017/02/07 06:27:29 Done.
// Record some dismisses.
EXPECT_FALSE(autoblocker()->RecordDismissAndEmbargo(
@@ -346,6 +352,12 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestDismissEmbargoBackoff) {
clock()->Advance(base::TimeDelta::FromDays(8));
EXPECT_FALSE(
autoblocker()->IsUnderEmbargo(content::PermissionType::GEOLOCATION, url));
+
+ // Record another dismiss, subsequent requests should be autoblocked again.
+ EXPECT_TRUE(autoblocker()->RecordDismissAndEmbargo(
+ url, content::PermissionType::GEOLOCATION));
+ EXPECT_TRUE(
+ autoblocker()->IsUnderEmbargo(content::PermissionType::GEOLOCATION, url));
}
// Test the logic for a combination of blacklisting and dismissal embargo.
@@ -377,6 +389,7 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestExpiredBlacklistEmbargo) {
TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingTimeout) {
GURL url("https://www.google.com");
clock()->SetNow(base::Time::Now());
+ base::HistogramTester histograms;
scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
new MockSafeBrowsingDatabaseManager(false /* perform_callback */);
@@ -389,6 +402,8 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingTimeout) {
EXPECT_FALSE(last_embargoed_status());
EXPECT_FALSE(
autoblocker()->IsUnderEmbargo(content::PermissionType::GEOLOCATION, url));
+ histograms.ExpectUniqueSample("Permissions.AutoBlocker.SafeBrowsingResponse",
+ SafeBrowsingResponse::TIMEOUT, 1);
db_manager->SetPerformCallback(true);
SetSafeBrowsingDatabaseManagerAndTimeoutForTesting(db_manager,
2000 /* timeout in ms */);
@@ -396,6 +411,8 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingTimeout) {
clock()->Advance(base::TimeDelta::FromDays(1));
UpdateEmbargoedStatus(content::PermissionType::GEOLOCATION, url);
EXPECT_TRUE(last_embargoed_status());
+ histograms.ExpectTotalCount("Permissions.AutoBlocker.SafeBrowsingResponse",
dominickn 2017/02/07 05:36:54 Can you ExpectBucketCount the specific bucket that
meredithl 2017/02/07 06:27:29 Done.
+ 2);
clock()->Advance(base::TimeDelta::FromDays(1));
EXPECT_TRUE(
@@ -473,3 +490,21 @@ TEST_F(PermissionDecisionAutoBlockerUnitTest,
EXPECT_EQ(50, autoblocker()->GetIgnoreCount(
url, content::PermissionType::GEOLOCATION));
}
+
+TEST_F(PermissionDecisionAutoBlockerUnitTest, TestSafeBrowsingResponse) {
+ GURL url("https://www.google.com");
+ clock()->SetNow(base::Time::Now());
+ base::HistogramTester histograms;
+
+ scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
+ new MockSafeBrowsingDatabaseManager(true /* perform_callback */);
+ std::set<std::string> blacklisted_permissions{"GEOLOCATION"};
+ db_manager->BlacklistUrlPermissions(url, blacklisted_permissions);
+ SetSafeBrowsingDatabaseManagerAndTimeoutForTesting(db_manager,
+ 0 /* timeout in ms */);
+
+ UpdateEmbargoedStatus(content::PermissionType::NOTIFICATIONS, url);
+ EXPECT_FALSE(last_embargoed_status());
+ histograms.ExpectUniqueSample("Permissions.AutoBlocker.SafeBrowsingResponse",
+ SafeBrowsingResponse::NOT_BLACKLISTED, 1);
+}

Powered by Google App Engine
This is Rietveld 408576698