Skip to content

GitLab

T
tangled
Commit 290d578e authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot
Browse files
Options

Include anchor element metrics that differ from one from document URL 

Currently, the render process sends the statistics of
only the anchor elements that are in the viewport to
the browser. This CL changes it to also include the
anchor elements whose target URL is one digit different
from the document URL.

Note that there is still a limit of 40 on the count of anchor
elements that are sent from the render to the browser process.

Change-Id: I4083e3605095a44700cc2178615713ede6e0dde2
Bug: 850624
TBR: ryansturm@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1329848
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607328}
parent 39a044c9
Hide whitespace changes
Inline Side-by-side
Showing with 65 additions and 9 deletions
chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
View file @ 290d578e
...@@ -435,3 +435,27 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest, ...@@ -435,3 +435,27 @@ IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
"AnchorElementMetrics.Clicked.OnNonDSE.SameHost", 0, 1); "AnchorElementMetrics.Clicked.OnNonDSE.SameHost", 0, 1);
} }
} }
// Tests that the browser only receives anchor elements that are in the
// viewport, and from anchor elements whose target differ from document URL
// by one digit.
IN_PROC_BROWSER_TEST_P(NavigationPredictorBrowserTest,
ViewportOnlyAndUrlIncrementByOne) {
base::HistogramTester histogram_tester;
const GURL& url = GetTestURL("/long_page_with_anchors-1.html");
ui_test_utils::NavigateToURL(browser(), url);
base::RunLoop().RunUntilIdle();
if (base::FeatureList::IsEnabled(
blink::features::kRecordAnchorMetricsVisible)) {
histogram_tester.ExpectUniqueSample(
"AnchorElementMetrics.Visible.NumberOfAnchorElements", 2, 1);
// Same document anchor element should be removed after merge.
histogram_tester.ExpectUniqueSample(
"AnchorElementMetrics.Visible.NumberOfAnchorElementsAfterMerge", 2, 1);
} else {
histogram_tester.ExpectTotalCount(
"AnchorElementMetrics.Visible.NumberOfAnchorElements", 0);
}
}
chrome/test/data/navigation_predictor/long_page_with_anchors-1.html 0 → 100644
View file @ 290d578e
<html>
<head>
</head>
<body>
<a id="google" href="https://google.com">Google</a>
<img src="large_image_takes_all_viewport.gif" height="4200" width="4200">
<a id="example2" href="https://example2.com">Below viewport</a>
<a id="example3" href="long_page_with_anchors-2.html">Below viewport differ by one</a>
</body>
</html>
\ No newline at end of file
third_party/blink/renderer/core/html/anchor_element_metrics.cc
View file @ 290d578e
...@@ -141,7 +141,8 @@ const int AnchorElementMetrics::kMaxAnchorElementMetricsSize = 40; ...@@ -141,7 +141,8 @@ const int AnchorElementMetrics::kMaxAnchorElementMetricsSize = 40;
// static // static
base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create( base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create(
const HTMLAnchorElement* anchor_element) { const HTMLAnchorElement* anchor_element,
bool is_url_incremented_by_one) {
LocalFrame* local_frame = anchor_element->GetDocument().GetFrame(); LocalFrame* local_frame = anchor_element->GetDocument().GetFrame();
LayoutObject* layout_object = anchor_element->GetLayoutObject(); LayoutObject* layout_object = anchor_element->GetLayoutObject();
if (!local_frame || !layout_object) if (!local_frame || !layout_object)
...@@ -153,7 +154,11 @@ base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create( ...@@ -153,7 +154,11 @@ base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create(
return base::nullopt; return base::nullopt;
IntRect viewport = root_frame_view->LayoutViewport()->VisibleContentRect(); IntRect viewport = root_frame_view->LayoutViewport()->VisibleContentRect();
if (viewport.Size().IsEmpty())
// Do not ignore anchor elements for which |is_url_incremented_by_one| is
// true since there is typically high click probability for such anchor
// elements.
if (viewport.Size().IsEmpty() && !is_url_incremented_by_one)
return base::nullopt; return base::nullopt;
// Use the viewport size to normalize anchor element metrics. // Use the viewport size to normalize anchor element metrics.
...@@ -205,7 +210,7 @@ base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create( ...@@ -205,7 +210,7 @@ base::Optional<AnchorElementMetrics> AnchorElementMetrics::Create(
ratio_distance_top_to_visible_top, ratio_distance_center_to_visible_top, ratio_distance_top_to_visible_top, ratio_distance_center_to_visible_top,
ratio_distance_root_top, ratio_distance_root_bottom, ratio_root_height, ratio_distance_root_top, ratio_distance_root_bottom, ratio_root_height,
IsInIFrame(*anchor_element), ContainsImage(*anchor_element), IsInIFrame(*anchor_element), ContainsImage(*anchor_element),
IsSameHost(*anchor_element), IsUrlIncrementedByOne(*anchor_element)); IsSameHost(*anchor_element), is_url_incremented_by_one);
} }
// static // static
...@@ -219,7 +224,8 @@ AnchorElementMetrics::MaybeReportClickedMetricsOnClick( ...@@ -219,7 +224,8 @@ AnchorElementMetrics::MaybeReportClickedMetricsOnClick(
return base::nullopt; return base::nullopt;
} }
auto anchor_metrics = Create(anchor_element); bool is_url_incremented_by_one = IsUrlIncrementedByOne(*anchor_element);
auto anchor_metrics = Create(anchor_element, is_url_incremented_by_one);
if (anchor_metrics.has_value()) { if (anchor_metrics.has_value()) {
anchor_metrics.value().RecordMetricsOnClick(); anchor_metrics.value().RecordMetricsOnClick();
...@@ -249,14 +255,22 @@ void AnchorElementMetrics::MaybeReportViewportMetricsOnLoad( ...@@ -249,14 +255,22 @@ void AnchorElementMetrics::MaybeReportViewportMetricsOnLoad(
for (const auto& member_element : sender->GetAnchorElements()) { for (const auto& member_element : sender->GetAnchorElements()) {
const HTMLAnchorElement& anchor_element = *member_element; const HTMLAnchorElement& anchor_element = *member_element;
if (!anchor_element.Href().ProtocolIsInHTTPFamily())
continue;
bool is_url_incremented_by_one = IsUrlIncrementedByOne(anchor_element);
// We ignore anchor elements that are not in the visual viewport. // We ignore anchor elements that are not in the visual viewport.
if (!anchor_element.Href().ProtocolIsInHTTPFamily() || // Do not ignore anchor elements for which |is_url_incremented_by_one| is
anchor_element.VisibleBoundsInVisualViewport().IsEmpty()) { // true since there is typically high click probability for such anchor
// elements.
if (anchor_element.VisibleBoundsInVisualViewport().IsEmpty() &&
!is_url_incremented_by_one) {
continue; continue;
} }
base::Optional<AnchorElementMetrics> anchor_metric = base::Optional<AnchorElementMetrics> anchor_metric =
Create(&anchor_element); Create(&anchor_element, is_url_incremented_by_one);
if (!anchor_metric.has_value()) if (!anchor_metric.has_value())
continue; continue;
......
third_party/blink/renderer/core/html/anchor_element_metrics.h
View file @ 290d578e
...@@ -55,8 +55,16 @@ class CORE_EXPORT AnchorElementMetrics { ...@@ -55,8 +55,16 @@ class CORE_EXPORT AnchorElementMetrics {
// browser on page load. // browser on page load.
static const int kMaxAnchorElementMetricsSize; static const int kMaxAnchorElementMetricsSize;
// Extract features of the anchor element. // Extract features of the anchor element. |is_url_incremented_by_one| is
static base::Optional<AnchorElementMetrics> Create(const HTMLAnchorElement*); // true if |anchor_element| links to a URL which is one number different
// from the document's URL. Based on the value of the anchor element URL,
// and document URL, |is_url_incremented_by_one| may be true or false.
// Examples for different values of |is_url_incremented_by_one|:
// example.com/page9/cat5, example.com/page10/cat5 => true
// example.com/page9/cat5, example.com/page10/cat10 => false
static base::Optional<AnchorElementMetrics> Create(
const HTMLAnchorElement* anchor_element,
bool is_url_incremented_by_one);
// Returns the mojom struct used to send metrics to the browser process. // Returns the mojom struct used to send metrics to the browser process.
mojom::blink::AnchorElementMetricsPtr CreateMetricsPtr() const; mojom::blink::AnchorElementMetricsPtr CreateMetricsPtr() const;
......
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