Commit 1d39d256 authored by Doug Arnett's avatar Doug Arnett Committed by Commit Bot

[Previews] Records new PreviewsEligibilityReason for redirects and deny

Records PreviewsEligibilityReason in UKM for detected redirect loop
cycle and for deny list match.

Bug: 1012852
Change-Id: I8b0dc745a4ef5757d07383ed866e846839bc954d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1852193Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704774}
parent 8ed269d5
......@@ -297,6 +297,14 @@ IN_PROC_BROWSER_TEST_P(
&histogram_tester, "Previews.DeferAllScript.DenyListMatch", 1);
histogram_tester.ExpectUniqueSample("Previews.DeferAllScript.DenyListMatch",
true, 1);
// Verify UKM entry.
using UkmEntry = ukm::builders::Previews;
auto entries = test_ukm_recorder.GetEntriesByName(UkmEntry::kEntryName);
ASSERT_EQ(1u, entries.size());
auto* entry = entries.at(0);
test_ukm_recorder.ExpectEntryMetric(
entry, UkmEntry::kdefer_all_script_eligibility_reasonName,
static_cast<int>(previews::PreviewsEligibilityReason::DENY_LIST_MATCHED));
EXPECT_EQ(kNonDeferredPageExpectedOutput, GetScriptLog());
}
......@@ -439,6 +447,7 @@ IN_PROC_BROWSER_TEST_P(
histogram_tester.ExpectTotalCount(
"Navigation.ClientRedirectCycle.RedirectToReferrer", 2);
histogram_tester.ExpectTotalCount("Previews.PageEndReason.DeferAllScript", 3);
EXPECT_FALSE(previews_service->IsUrlEligibleForDeferAllScriptPreview(
client_redirect_url()));
EXPECT_FALSE(previews_service->IsUrlEligibleForDeferAllScriptPreview(
......@@ -547,4 +556,14 @@ IN_PROC_BROWSER_TEST_P(
// preview.
EXPECT_TRUE(
previews_service->IsUrlEligibleForDeferAllScriptPreview(https_url()));
// Verify UKM entry.
using UkmEntry = ukm::builders::Previews;
auto entries = test_ukm_recorder.GetEntriesByName(UkmEntry::kEntryName);
ASSERT_EQ(4u, entries.size());
auto* entry = entries.at(3);
test_ukm_recorder.ExpectEntryMetric(
entry, UkmEntry::kdefer_all_script_eligibility_reasonName,
static_cast<int>(
previews::PreviewsEligibilityReason::REDIRECT_LOOP_DETECTED));
}
......@@ -463,6 +463,18 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
previews_state &= ~content::DEFER_ALL_SCRIPT_ON;
UMA_HISTOGRAM_BOOLEAN(
"Previews.DeferAllScript.RedirectLoopDetectedUsingCache", true);
if (previews_service->previews_ui_service()) {
previews::PreviewsDeciderImpl* previews_decider_impl =
previews_service->previews_ui_service()->previews_decider_impl();
DCHECK(previews_decider_impl);
std::vector<PreviewsEligibilityReason> passed_reasons;
previews_decider_impl->LogPreviewDecisionMade(
PreviewsEligibilityReason::REDIRECT_LOOP_DETECTED,
navigation_handle->GetURL(),
base::DefaultClock::GetInstance()->Now(),
PreviewsType::DEFER_ALL_SCRIPT, std::move(passed_reasons),
previews_data);
}
}
}
}
......@@ -478,6 +490,18 @@ content::PreviewsState DetermineCommittedClientPreviewsState(
previews_service->MatchesDeferAllScriptDenyListRegexp(url)) {
previews_state &= ~content::DEFER_ALL_SCRIPT_ON;
UMA_HISTOGRAM_BOOLEAN("Previews.DeferAllScript.DenyListMatch", true);
if (previews_service->previews_ui_service()) {
previews::PreviewsDeciderImpl* previews_decider_impl =
previews_service->previews_ui_service()->previews_decider_impl();
DCHECK(previews_decider_impl);
std::vector<PreviewsEligibilityReason> passed_reasons;
previews_decider_impl->LogPreviewDecisionMade(
PreviewsEligibilityReason::DENY_LIST_MATCHED,
navigation_handle->GetURL(),
base::DefaultClock::GetInstance()->Now(),
PreviewsType::DEFER_ALL_SCRIPT, std::move(passed_reasons),
previews_data);
}
}
}
}
......
......@@ -87,6 +87,10 @@ enum class PreviewsEligibilityReason {
NOT_ALLOWED_BY_OPTIMIZATION_GUIDE = 19,
// The preview was not performed due to a coinflip experiment holdback.
COINFLIP_HOLDBACK = 20,
// A redirect loop was detected.
REDIRECT_LOOP_DETECTED = 21,
// URL matched the deny list.
DENY_LIST_MATCHED = 22,
LAST,
};
......
......@@ -101,6 +101,12 @@ std::string GetReasonDescription(PreviewsEligibilityReason reason,
case PreviewsEligibilityReason::COINFLIP_HOLDBACK:
DCHECK(!want_inverse_description);
return "Coin flip holdback encountered";
case PreviewsEligibilityReason::REDIRECT_LOOP_DETECTED:
DCHECK(!want_inverse_description);
return "Redirect loop detected";
case PreviewsEligibilityReason::DENY_LIST_MATCHED:
DCHECK(!want_inverse_description);
return "URL matched deny list";
case PreviewsEligibilityReason::LAST:
break;
}
......
......@@ -49822,6 +49822,8 @@ Called by update_net_trust_anchors.py.-->
<int value="18" label="The navigation URL had an excluded media suffix."/>
<int value="19" label="Not allowed by server rules provided to the client."/>
<int value="20" label="Coin flip holdback encountered."/>
<int value="21" label="Redirect loop detected."/>
<int value="22" label="URL matched deny list."/>
</enum>
<enum name="PreviewsHintCacheLevelDBStoreLoadMetadataResult">
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