Commit f3ae11fc authored by Karan Bhatia's avatar Karan Bhatia Committed by Commit Bot

DNR: Add UMA for initiator checks performed by the page whitelisting API.

The Declarative Net Request Page Whitelisting API uses initiator checks to guess
the main frame url for a main-frame subresource request. This CL adds UMA to
enumerate how often the different cases occur. This should help reason about the
correctness of these checks.

BUG=811460

Change-Id: Id72c05041477d05825fbe7e0ff7fecbf39fda861
Reviewed-on: https://chromium-review.googlesource.com/1089475Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565473}
parent e12209c4
...@@ -30,6 +30,19 @@ namespace { ...@@ -30,6 +30,19 @@ namespace {
namespace flat_rule = url_pattern_index::flat; namespace flat_rule = url_pattern_index::flat;
// Describes the different cases pertaining to initiator checks to find the main
// frame url for a main frame subresource.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class PageWhitelistingInitiatorCheck {
kInitiatorAbsent = 0,
kNeitherCandidateMatchesInitiator = 1,
kCommittedCandidateMatchesInitiator = 2,
kPendingCandidateMatchesInitiator = 3,
kBothCandidatesMatchInitiator = 4,
kMaxValue = kBothCandidatesMatchInitiator,
};
// Maps content::ResourceType to flat_rule::ElementType. // Maps content::ResourceType to flat_rule::ElementType.
flat_rule::ElementType GetElementType(content::ResourceType type) { flat_rule::ElementType GetElementType(content::ResourceType type) {
switch (type) { switch (type) {
...@@ -160,6 +173,12 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request, ...@@ -160,6 +173,12 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request,
// loads. // loads.
DCHECK_EQ(ExtensionApiFrameIdMap::kTopFrameId, request.frame_data->frame_id); DCHECK_EQ(ExtensionApiFrameIdMap::kTopFrameId, request.frame_data->frame_id);
auto log_uma = [](PageWhitelistingInitiatorCheck value) {
UMA_HISTOGRAM_ENUMERATION(
"Extensions.DeclarativeNetRequest.PageWhitelistingInitiatorCheck",
value);
};
// At this point, we are evaluating a main-frame subresource. There are two // At this point, we are evaluating a main-frame subresource. There are two
// candidate main frame urls - |pending_main_frame_url| and // candidate main frame urls - |pending_main_frame_url| and
// |last_committed_main_frame_url|. To predict the correct main frame url, // |last_committed_main_frame_url|. To predict the correct main frame url,
...@@ -167,7 +186,9 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request, ...@@ -167,7 +186,9 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request,
// of the main frame in this case) with the candidate urls' origins. If only // of the main frame in this case) with the candidate urls' origins. If only
// one of the candidate url's origin matches the request initiator, we can be // one of the candidate url's origin matches the request initiator, we can be
// reasonably sure that it is the correct main frame url. // reasonably sure that it is the correct main frame url.
if (request.initiator) { if (!request.initiator) {
log_uma(PageWhitelistingInitiatorCheck::kInitiatorAbsent);
} else {
const bool initiator_matches_pending_url = const bool initiator_matches_pending_url =
url::Origin::Create(*request.frame_data->pending_main_frame_url) == url::Origin::Create(*request.frame_data->pending_main_frame_url) ==
*request.initiator; *request.initiator;
...@@ -178,6 +199,8 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request, ...@@ -178,6 +199,8 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request,
if (initiator_matches_pending_url && !initiator_matches_committed_url) { if (initiator_matches_pending_url && !initiator_matches_committed_url) {
// We predict that |pending_main_frame_url| is the actual main frame url. // We predict that |pending_main_frame_url| is the actual main frame url.
log_uma(
PageWhitelistingInitiatorCheck::kPendingCandidateMatchesInitiator);
return whitelisted_pages.MatchesURL( return whitelisted_pages.MatchesURL(
*request.frame_data->pending_main_frame_url); *request.frame_data->pending_main_frame_url);
} }
...@@ -185,16 +208,26 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request, ...@@ -185,16 +208,26 @@ bool IsRequestPageWhitelisted(const WebRequestInfo& request,
if (initiator_matches_committed_url && !initiator_matches_pending_url) { if (initiator_matches_committed_url && !initiator_matches_pending_url) {
// We predict that |last_committed_main_frame_url| is the actual main // We predict that |last_committed_main_frame_url| is the actual main
// frame url. // frame url.
log_uma(
PageWhitelistingInitiatorCheck::kCommittedCandidateMatchesInitiator);
return whitelisted_pages.MatchesURL( return whitelisted_pages.MatchesURL(
request.frame_data->last_committed_main_frame_url); request.frame_data->last_committed_main_frame_url);
} }
if (initiator_matches_pending_url && initiator_matches_committed_url) {
log_uma(PageWhitelistingInitiatorCheck::kBothCandidatesMatchInitiator);
} else {
DCHECK(!initiator_matches_pending_url);
DCHECK(!initiator_matches_committed_url);
log_uma(
PageWhitelistingInitiatorCheck::kNeitherCandidateMatchesInitiator);
}
} }
// If we are not able to correctly predict the main frame url, simply test // If we are not able to correctly predict the main frame url, simply test
// against both the possible URLs. This means a small proportion of main frame // against both the possible URLs. This means a small proportion of main frame
// subresource requests might be incorrectly whitelisted by the page // subresource requests might be incorrectly whitelisted by the page
// whitelisting API. // whitelisting API.
// TODO(karandeepb): Add UMA to see how often this happens.
return whitelisted_pages.MatchesURL( return whitelisted_pages.MatchesURL(
request.frame_data->last_committed_main_frame_url) || request.frame_data->last_committed_main_frame_url) ||
whitelisted_pages.MatchesURL( whitelisted_pages.MatchesURL(
......
...@@ -35129,6 +35129,14 @@ Called by update_net_trust_anchors.py.--> ...@@ -35129,6 +35129,14 @@ Called by update_net_trust_anchors.py.-->
<int value="8" label="Unknown"/> <int value="8" label="Unknown"/>
</enum> </enum>
<enum name="PageWhitelistingInitiatorCheck">
<int value="0" label="Initiator absent"/>
<int value="1" label="Neither candidate matches initiator"/>
<int value="2" label="Committed candidate matches initiator"/>
<int value="3" label="Pending candidate matches initiator"/>
<int value="4" label="Both candidates match initiator"/>
</enum>
<enum name="PaletteModeCancelType"> <enum name="PaletteModeCancelType">
<int value="0" label="Palette laser pointer mode is cancelled."/> <int value="0" label="Palette laser pointer mode is cancelled."/>
<int value="1" label="Palette laser pointer mode is switched out of"/> <int value="1" label="Palette laser pointer mode is switched out of"/>
...@@ -26283,6 +26283,18 @@ uploading your change for review. ...@@ -26283,6 +26283,18 @@ uploading your change for review.
</summary> </summary>
</histogram> </histogram>
<histogram
name="Extensions.DeclarativeNetRequest.PageWhitelistingInitiatorCheck"
enum="PageWhitelistingInitiatorCheck" expires_after="2019-06-30">
<owner>karandeepb@chromium.org</owner>
<owner>lazyboy@chromium.org</owner>
<summary>
Describes the different cases pertaining to initiator checks used by the
Declarative Net Request Page Whitelisting API. Emitted whenever an extension
ruleset is evaluated for a main frame sub-resource request.
</summary>
</histogram>
<histogram <histogram
name="Extensions.DeclarativeNetRequest.ShouldBlockRequestTime.AllExtensions" name="Extensions.DeclarativeNetRequest.ShouldBlockRequestTime.AllExtensions"
units="ms"> units="ms">
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