Skip to content

GitLab

T
tangled
Commit 7856c1e2 authored by treib's avatar treib Committed by Commit bot
Browse files
Options

Supervised users: Add histogram to record filtering results 

This is in addition to the SafeSites histogram.

Also, cleanup some unused histograms in the URL filter.

Review URL: https://codereview.chromium.org/1043113002

Cr-Commit-Position: refs/heads/master@{#324238}
parent de318388
Hide whitespace changes
Inline Side-by-side
Showing with 124 additions and 32 deletions
chrome/browser/supervised_user/supervised_user_resource_throttle.cc
View file @ 7856c1e2
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
#include "chrome/browser/supervised_user/supervised_user_resource_throttle.h" #include "chrome/browser/supervised_user/supervised_user_resource_throttle.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/metrics/histogram.h" #include "base/metrics/sparse_histogram.h"
#include "chrome/browser/supervised_user/supervised_user_interstitial.h" #include "chrome/browser/supervised_user/supervised_user_interstitial.h"
#include "chrome/browser/supervised_user/supervised_user_navigation_observer.h" #include "chrome/browser/supervised_user/supervised_user_navigation_observer.h"
#include "chrome/browser/supervised_user/supervised_user_url_filter.h" #include "chrome/browser/supervised_user/supervised_user_url_filter.h"
...@@ -28,14 +28,16 @@ enum { ...@@ -28,14 +28,16 @@ enum {
FILTERING_BEHAVIOR_ALLOW_UNCERTAIN, FILTERING_BEHAVIOR_ALLOW_UNCERTAIN,
FILTERING_BEHAVIOR_BLOCK_BLACKLIST, FILTERING_BEHAVIOR_BLOCK_BLACKLIST,
FILTERING_BEHAVIOR_BLOCK_SAFESITES, FILTERING_BEHAVIOR_BLOCK_SAFESITES,
FILTERING_BEHAVIOR_MAX = FILTERING_BEHAVIOR_BLOCK_SAFESITES FILTERING_BEHAVIOR_BLOCK_MANUAL,
FILTERING_BEHAVIOR_BLOCK_DEFAULT,
FILTERING_BEHAVIOR_MAX = FILTERING_BEHAVIOR_BLOCK_DEFAULT
}; };
const int kHistogramFilteringBehaviorSpacing = 100; const int kHistogramFilteringBehaviorSpacing = 100;
const int kHistogramPageTransitionMaxKnownValue = const int kHistogramPageTransitionMaxKnownValue =
static_cast<int>(ui::PAGE_TRANSITION_KEYWORD_GENERATED); static_cast<int>(ui::PAGE_TRANSITION_KEYWORD_GENERATED);
const int kHistogramPageTransitionFallbackValue = const int kHistogramPageTransitionFallbackValue =
kHistogramFilteringBehaviorSpacing - 1; kHistogramFilteringBehaviorSpacing - 1;
const int kHistogramMax = 500; const int kHistogramMax = 700;
static_assert(kHistogramPageTransitionMaxKnownValue < static_assert(kHistogramPageTransitionMaxKnownValue <
kHistogramPageTransitionFallbackValue, kHistogramPageTransitionFallbackValue,
...@@ -54,12 +56,17 @@ int GetHistogramValueForFilteringBehavior( ...@@ -54,12 +56,17 @@ int GetHistogramValueForFilteringBehavior(
return uncertain ? FILTERING_BEHAVIOR_ALLOW_UNCERTAIN return uncertain ? FILTERING_BEHAVIOR_ALLOW_UNCERTAIN
: FILTERING_BEHAVIOR_ALLOW; : FILTERING_BEHAVIOR_ALLOW;
case SupervisedUserURLFilter::BLOCK: case SupervisedUserURLFilter::BLOCK:
if (reason == SupervisedUserURLFilter::BLACKLIST) switch (reason) {
return FILTERING_BEHAVIOR_BLOCK_BLACKLIST; case SupervisedUserURLFilter::BLACKLIST:
else if (reason == SupervisedUserURLFilter::ASYNC_CHECKER) return FILTERING_BEHAVIOR_BLOCK_BLACKLIST;
return FILTERING_BEHAVIOR_BLOCK_SAFESITES; case SupervisedUserURLFilter::ASYNC_CHECKER:
// Fall through. return FILTERING_BEHAVIOR_BLOCK_SAFESITES;
default: case SupervisedUserURLFilter::MANUAL:
return FILTERING_BEHAVIOR_BLOCK_MANUAL;
case SupervisedUserURLFilter::DEFAULT:
return FILTERING_BEHAVIOR_BLOCK_DEFAULT;
}
case SupervisedUserURLFilter::INVALID:
NOTREACHED(); NOTREACHED();
} }
return 0; return 0;
...@@ -75,19 +82,23 @@ int GetHistogramValueForTransitionType(ui::PageTransition transition_type) { ...@@ -75,19 +82,23 @@ int GetHistogramValueForTransitionType(ui::PageTransition transition_type) {
} }
void RecordFilterResultEvent( void RecordFilterResultEvent(
bool safesites_histogram,
SupervisedUserURLFilter::FilteringBehavior behavior, SupervisedUserURLFilter::FilteringBehavior behavior,
SupervisedUserURLFilter::FilteringBehaviorReason reason, SupervisedUserURLFilter::FilteringBehaviorReason reason,
bool uncertain, bool uncertain,
ui::PageTransition transition_type) { ui::PageTransition transition_type) {
DCHECK(reason == SupervisedUserURLFilter::ASYNC_CHECKER ||
reason == SupervisedUserURLFilter::BLACKLIST);
int value = int value =
GetHistogramValueForFilteringBehavior(behavior, reason, uncertain) * GetHistogramValueForFilteringBehavior(behavior, reason, uncertain) *
kHistogramFilteringBehaviorSpacing + kHistogramFilteringBehaviorSpacing +
GetHistogramValueForTransitionType(transition_type); GetHistogramValueForTransitionType(transition_type);
DCHECK_LT(value, kHistogramMax); DCHECK_LT(value, kHistogramMax);
UMA_HISTOGRAM_ENUMERATION("ManagedUsers.SafetyFilter", // Note: We can't pass in the histogram name as a parameter to this function
value, kHistogramMax); // because of how the macro works (look up the histogram on the first
// invocation and cache it in a static variable).
if (safesites_histogram)
UMA_HISTOGRAM_SPARSE_SLOWLY("ManagedUsers.SafetyFilter", value);
else
UMA_HISTOGRAM_SPARSE_SLOWLY("ManagedUsers.FilteringResult", value);
} }
} // namespace } // namespace
...@@ -100,7 +111,7 @@ SupervisedUserResourceThrottle::SupervisedUserResourceThrottle( ...@@ -100,7 +111,7 @@ SupervisedUserResourceThrottle::SupervisedUserResourceThrottle(
is_main_frame_(is_main_frame), is_main_frame_(is_main_frame),
url_filter_(url_filter), url_filter_(url_filter),
deferred_(false), deferred_(false),
behavior_(SupervisedUserURLFilter::HISTOGRAM_BOUNDING_VALUE), behavior_(SupervisedUserURLFilter::INVALID),
weak_ptr_factory_(this) {} weak_ptr_factory_(this) {}
SupervisedUserResourceThrottle::~SupervisedUserResourceThrottle() {} SupervisedUserResourceThrottle::~SupervisedUserResourceThrottle() {}
...@@ -113,18 +124,17 @@ void SupervisedUserResourceThrottle::ShowInterstitialIfNeeded(bool is_redirect, ...@@ -113,18 +124,17 @@ void SupervisedUserResourceThrottle::ShowInterstitialIfNeeded(bool is_redirect,
return; return;
deferred_ = false; deferred_ = false;
DCHECK_EQ(SupervisedUserURLFilter::HISTOGRAM_BOUNDING_VALUE, behavior_); DCHECK_EQ(SupervisedUserURLFilter::INVALID, behavior_);
bool got_result = url_filter_->GetFilteringBehaviorForURLWithAsyncChecks( bool got_result = url_filter_->GetFilteringBehaviorForURLWithAsyncChecks(
url, url,
base::Bind(&SupervisedUserResourceThrottle::OnCheckDone, base::Bind(&SupervisedUserResourceThrottle::OnCheckDone,
weak_ptr_factory_.GetWeakPtr(), url)); weak_ptr_factory_.GetWeakPtr(), url));
DCHECK_EQ(got_result, DCHECK_EQ(got_result, behavior_ != SupervisedUserURLFilter::INVALID);
(behavior_ != SupervisedUserURLFilter::HISTOGRAM_BOUNDING_VALUE));
// If we got a "not blocked" result synchronously, don't defer. // If we got a "not blocked" result synchronously, don't defer.
*defer = deferred_ = !got_result || *defer = deferred_ = !got_result ||
(behavior_ == SupervisedUserURLFilter::BLOCK); (behavior_ == SupervisedUserURLFilter::BLOCK);
if (got_result) if (got_result)
behavior_ = SupervisedUserURLFilter::HISTOGRAM_BOUNDING_VALUE; behavior_ = SupervisedUserURLFilter::INVALID;
} }
void SupervisedUserResourceThrottle::ShowInterstitial( void SupervisedUserResourceThrottle::ShowInterstitial(
...@@ -159,19 +169,22 @@ void SupervisedUserResourceThrottle::OnCheckDone( ...@@ -159,19 +169,22 @@ void SupervisedUserResourceThrottle::OnCheckDone(
SupervisedUserURLFilter::FilteringBehavior behavior, SupervisedUserURLFilter::FilteringBehavior behavior,
SupervisedUserURLFilter::FilteringBehaviorReason reason, SupervisedUserURLFilter::FilteringBehaviorReason reason,
bool uncertain) { bool uncertain) {
DCHECK_EQ(SupervisedUserURLFilter::HISTOGRAM_BOUNDING_VALUE, behavior_); DCHECK_EQ(SupervisedUserURLFilter::INVALID, behavior_);
// If we got a result synchronously, pass it back to ShowInterstitialIfNeeded. // If we got a result synchronously, pass it back to ShowInterstitialIfNeeded.
if (!deferred_) if (!deferred_)
behavior_ = behavior; behavior_ = behavior;
// If both the static blacklist and SafeSites are enabled, record UMA events. ui::PageTransition transition =
content::ResourceRequestInfo::ForRequest(request_)->GetPageTransition();
RecordFilterResultEvent(false, behavior, reason, uncertain, transition);
// If both the static blacklist and the async checker are enabled, also record
// SafeSites-only UMA events.
if (url_filter_->HasBlacklist() && url_filter_->HasAsyncURLChecker() && if (url_filter_->HasBlacklist() && url_filter_->HasAsyncURLChecker() &&
(reason == SupervisedUserURLFilter::ASYNC_CHECKER || (reason == SupervisedUserURLFilter::ASYNC_CHECKER ||
reason == SupervisedUserURLFilter::BLACKLIST)) { reason == SupervisedUserURLFilter::BLACKLIST)) {
const content::ResourceRequestInfo* info = RecordFilterResultEvent(true, behavior, reason, uncertain, transition);
content::ResourceRequestInfo::ForRequest(request_);
RecordFilterResultEvent(behavior, reason, uncertain,
info->GetPageTransition());
} }
if (behavior == SupervisedUserURLFilter::BLOCK) if (behavior == SupervisedUserURLFilter::BLOCK)
......
chrome/browser/supervised_user/supervised_user_url_filter.cc
View file @ 7856c1e2
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "base/containers/hash_tables.h" #include "base/containers/hash_tables.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/json/json_file_value_serializer.h" #include "base/json/json_file_value_serializer.h"
#include "base/metrics/histogram.h"
#include "base/sha1.h" #include "base/sha1.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
...@@ -417,16 +416,12 @@ void SupervisedUserURLFilter::SetManualHosts( ...@@ -417,16 +416,12 @@ void SupervisedUserURLFilter::SetManualHosts(
const std::map<std::string, bool>* host_map) { const std::map<std::string, bool>* host_map) {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
host_map_ = *host_map; host_map_ = *host_map;
UMA_HISTOGRAM_CUSTOM_COUNTS("ManagedMode.ManualHostsEntries",
host_map->size(), 1, 1000, 50);
} }
void SupervisedUserURLFilter::SetManualURLs( void SupervisedUserURLFilter::SetManualURLs(
const std::map<GURL, bool>* url_map) { const std::map<GURL, bool>* url_map) {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
url_map_ = *url_map; url_map_ = *url_map;
UMA_HISTOGRAM_CUSTOM_COUNTS("ManagedMode.ManualURLsEntries",
url_map->size(), 1, 1000, 50);
} }
void SupervisedUserURLFilter::InitAsyncURLChecker( void SupervisedUserURLFilter::InitAsyncURLChecker(
......
chrome/browser/supervised_user/supervised_user_url_filter.h
View file @ 7856c1e2
...@@ -48,7 +48,7 @@ class SupervisedUserURLFilter ...@@ -48,7 +48,7 @@ class SupervisedUserURLFilter
ALLOW, ALLOW,
WARN, WARN,
BLOCK, BLOCK,
HISTOGRAM_BOUNDING_VALUE INVALID
}; };
enum FilteringBehaviorReason { enum FilteringBehaviorReason {
DEFAULT, DEFAULT,
......
tools/metrics/histograms/histograms.xml
View file @ 7856c1e2
...@@ -13481,6 +13481,18 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -13481,6 +13481,18 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</summary> </summary>
</histogram> </histogram>
<histogram name="ManagedUsers.FilteringResult"
enum="SupervisedUserSafetyFilterResult">
<owner>treib@chromium.org</owner>
<owner>pam@chromium.org</owner>
<summary>
The counts of results from supervised user URL filtering. Each entry
includes the outcome of the filter (i.e. allowed, blocked, or unknown) and
the page transition type (how the user got there, e.g. typed URL, clicked
link).
</summary>
</histogram>
<histogram name="ManagedUsers.SafeSitesDelay" units="milliseconds"> <histogram name="ManagedUsers.SafeSitesDelay" units="milliseconds">
<owner>treib@chromium.org</owner> <owner>treib@chromium.org</owner>
<owner>pam@chromium.org</owner> <owner>pam@chromium.org</owner>
...@@ -13498,8 +13510,8 @@ Therefore, the affected-histogram name has to have at least one dot in it. ...@@ -13498,8 +13510,8 @@ Therefore, the affected-histogram name has to have at least one dot in it.
The counts of results from the supervised user safety filter. Each entry The counts of results from the supervised user safety filter. Each entry
includes the outcome of the filter (i.e. allowed, blocked, or unknown) and includes the outcome of the filter (i.e. allowed, blocked, or unknown) and
the page transition type (how the user got there, e.g. typed URL, clicked the page transition type (how the user got there, e.g. typed URL, clicked
link). This only includes URL requests that were not handled by a manual link). This only includes URL requests that were handled by the SafeSites
exception. filter (both online and the static blacklist).
</summary> </summary>
</histogram> </histogram>
...@@ -61706,6 +61718,78 @@ To add a new entry, add it with any value and run test to compute valid value. ...@@ -61706,6 +61718,78 @@ To add a new entry, add it with any value and run test to compute valid value.
<int value="499" label="OTHER_BLOCKED_SAFESITES"> <int value="499" label="OTHER_BLOCKED_SAFESITES">
Other navigation; Blocked by SafeSites Other navigation; Blocked by SafeSites
</int> </int>
<int value="500" label="LINK_BLOCKED_MANUAL">
Link; Blocked by manual exception
</int>
<int value="501" label="TYPED_BLOCKED_MANUAL">
Typed URL; Blocked by manual exception
</int>
<int value="502" label="AUTO_BOOKMARK_BLOCKED_MANUAL">
Bookmark; Blocked by manual exception
</int>
<int value="503" label="AUTO_SUBFRAME_BLOCKED_MANUAL">
Subframe navigation; Blocked by manual exception
</int>
<int value="504" label="MANUAL_SUBFRAME_BLOCKED_MANUAL">
Manual subframe navigation; Blocked by manual exception
</int>
<int value="505" label="GENERATED_BLOCKED_MANUAL">
Generated from Omnibox; Blocked by manual exception
</int>
<int value="506" label="AUTO_TOPLEVEL_BLOCKED_MANUAL">
Automatic toplevel navigation; Blocked by manual exception
</int>
<int value="507" label="FORM_SUBMIT_BLOCKED_MANUAL">
Form submission; Blocked by manual exception
</int>
<int value="508" label="RELOAD_BLOCKED_MANUAL">
Reload; Blocked by manual exception
</int>
<int value="509" label="KEYWORD_BLOCKED_MANUAL">
Omnibox keyword; Blocked by manual exception
</int>
<int value="510" label="KEYWORD_GENERATED_BLOCKED_MANUAL">
URL generated from Omnibox keyword; Blocked by manual exception
</int>
<int value="599" label="OTHER_BLOCKED_MANUAL">
Other navigation; Blocked by manual exception
</int>
<int value="600" label="LINK_BLOCKED_DEFAULT">
Link; Blocked by global settings
</int>
<int value="601" label="TYPED_BLOCKED_DEFAULT">
Typed URL; Blocked by global settings
</int>
<int value="602" label="AUTO_BOOKMARK_BLOCKED_DEFAULT">
Bookmark; Blocked by global settings
</int>
<int value="603" label="AUTO_SUBFRAME_BLOCKED_DEFAULT">
Subframe navigation; Blocked by global settings
</int>
<int value="604" label="MANUAL_SUBFRAME_BLOCKED_DEFAULT">
Manual subframe navigation; Blocked by global settings
</int>
<int value="605" label="GENERATED_BLOCKED_DEFAULT">
Generated from Omnibox; Blocked by global settings
</int>
<int value="606" label="AUTO_TOPLEVEL_BLOCKED_DEFAULT">
Automatic toplevel navigation; Blocked by global settings
</int>
<int value="607" label="FORM_SUBMIT_BLOCKED_DEFAULT">
Form submission; Blocked by global settings
</int>
<int value="608" label="RELOAD_BLOCKED_DEFAULT">
Reload; Blocked by global settings
</int>
<int value="609" label="KEYWORD_BLOCKED_DEFAULT">
Omnibox keyword; Blocked by global settings
</int>
<int value="610" label="KEYWORD_GENERATED_BLOCKED_DEFAULT">
URL generated from Omnibox keyword; Blocked by global settings
</int>
<int value="699" label="OTHER_BLOCKED_DEFAULT">
Other navigation; Blocked by global settings
</int>
</enum> </enum>
<enum name="SuspendAttempt" type="int"> <enum name="SuspendAttempt" type="int">
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment