Commit 84e62dd3 authored by Ryan Sturm's avatar Ryan Sturm Committed by Commit Bot

Removing layout size calls from AnchorElementMetrics OnClick

The clicked element will no longer report its size (or relative size) to
the browser/UMA. Instead, we plan to see if it is in the Anchors
reported on load. Its size would only be valuable if we knew about the
anchor from OnLoad.

Bug: 957026
Change-Id: I59a98348fd0d58a4dd1a99bd86c2720b36f1af88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1602773
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659283}
parent f1b97e0c
...@@ -222,15 +222,18 @@ AnchorElementMetrics::MaybeReportClickedMetricsOnClick( ...@@ -222,15 +222,18 @@ AnchorElementMetrics::MaybeReportClickedMetricsOnClick(
return base::nullopt; return base::nullopt;
} }
auto anchor_metrics = Create(anchor_element); // Create metrics that don't have sizes set. The browser only records
if (anchor_metrics.has_value()) { // metrics unrelated to sizes.
anchor_metrics.value().RecordMetricsOnClick(); AnchorElementMetrics anchor_metrics(
anchor_element, 0, 0, 0, 0, 0, 0, 0, IsInIFrame(*anchor_element),
// Send metrics of the anchor element to the browser process. ContainsImage(*anchor_element), IsSameHost(*anchor_element),
AnchorElementMetricsSender::From(*GetRootDocument(*anchor_element)) IsUrlIncrementedByOne(*anchor_element));
->SendClickedAnchorMetricsToBrowser(
anchor_metrics.value().CreateMetricsPtr()); anchor_metrics.RecordMetricsOnClick();
}
// Send metrics of the anchor element to the browser process.
AnchorElementMetricsSender::From(*GetRootDocument(*anchor_element))
->SendClickedAnchorMetricsToBrowser(anchor_metrics.CreateMetricsPtr());
return anchor_metrics; return anchor_metrics;
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "third_party/blink/public/common/features.h" #include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/local_frame.h" #include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/html/anchor_element_metrics.h"
#include "third_party/blink/renderer/core/html/html_anchor_element.h" #include "third_party/blink/renderer/core/html/html_anchor_element.h"
namespace blink { namespace blink {
......
...@@ -90,7 +90,7 @@ TEST_F(AnchorElementMetricsTest, FinchControl) { ...@@ -90,7 +90,7 @@ TEST_F(AnchorElementMetricsTest, FinchControl) {
base::test::ScopedFeatureList disabled_feature_list; base::test::ScopedFeatureList disabled_feature_list;
disabled_feature_list.InitAndDisableFeature(features::kNavigationPredictor); disabled_feature_list.InitAndDisableFeature(features::kNavigationPredictor);
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element); AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element);
histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.RatioArea", histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.IsSameHost",
0); 0);
// If we enable feature kNavigationPredictor, we should see count is 1 // If we enable feature kNavigationPredictor, we should see count is 1
...@@ -98,7 +98,7 @@ TEST_F(AnchorElementMetricsTest, FinchControl) { ...@@ -98,7 +98,7 @@ TEST_F(AnchorElementMetricsTest, FinchControl) {
base::test::ScopedFeatureList enabled_feature_list; base::test::ScopedFeatureList enabled_feature_list;
enabled_feature_list.InitAndEnableFeature(features::kNavigationPredictor); enabled_feature_list.InitAndEnableFeature(features::kNavigationPredictor);
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element); AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element);
histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.RatioArea", histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.IsSameHost",
1); 1);
} }
...@@ -115,7 +115,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) { ...@@ -115,7 +115,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) {
ToHTMLAnchorElement(GetDocument().getElementById("anchor")); ToHTMLAnchorElement(GetDocument().getElementById("anchor"));
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element); AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element);
histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.RatioArea", histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.IsSameHost",
0); 0);
// Tests that a data page with an HTTPS anchor is not reported when the anchor // Tests that a data page with an HTTPS anchor is not reported when the anchor
...@@ -125,7 +125,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) { ...@@ -125,7 +125,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) {
anchor_element = ToHTMLAnchorElement(GetDocument().getElementById("anchor")); anchor_element = ToHTMLAnchorElement(GetDocument().getElementById("anchor"));
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element); AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element);
histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.RatioArea", histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.IsSameHost",
0); 0);
// Tests that an HTTPS page with an HTTPS anchor is reported when the anchor // Tests that an HTTPS page with an HTTPS anchor is reported when the anchor
...@@ -137,7 +137,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) { ...@@ -137,7 +137,7 @@ TEST_F(AnchorElementMetricsTest, NonHTTPOnClick) {
anchor_element = ToHTMLAnchorElement(GetDocument().getElementById("anchor")); anchor_element = ToHTMLAnchorElement(GetDocument().getElementById("anchor"));
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element); AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element);
histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.RatioArea", histogram_tester.ExpectTotalCount("AnchorElementMetrics.Clicked.IsSameHost",
1); 1);
} }
...@@ -164,12 +164,6 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureImageLink) { ...@@ -164,12 +164,6 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureImageLink) {
auto feature = auto feature =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_FLOAT_EQ(0.25, feature.GetRatioArea());
EXPECT_FLOAT_EQ(0.25, feature.GetRatioVisibleArea());
EXPECT_FLOAT_EQ(0.5, feature.GetRatioDistanceTopToVisibleTop());
EXPECT_FLOAT_EQ(0.75, feature.GetRatioDistanceCenterToVisibleTop());
EXPECT_FLOAT_EQ(0.5, feature.GetRatioDistanceRootTop());
EXPECT_FLOAT_EQ(10, feature.GetRatioDistanceRootBottom());
EXPECT_FALSE(feature.GetIsInIframe()); EXPECT_FALSE(feature.GetIsInIframe());
EXPECT_TRUE(feature.GetContainsImage()); EXPECT_TRUE(feature.GetContainsImage());
EXPECT_TRUE(feature.GetIsSameHost()); EXPECT_TRUE(feature.GetIsSameHost());
...@@ -199,18 +193,9 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureExtract) { ...@@ -199,18 +193,9 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureExtract) {
auto feature = auto feature =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_GT(feature.GetRatioArea(), 0);
EXPECT_FLOAT_EQ(feature.GetRatioDistanceRootTop(), 2);
EXPECT_FLOAT_EQ(feature.GetRatioDistanceTopToVisibleTop(), 2);
EXPECT_EQ(feature.GetIsInIframe(), false); EXPECT_EQ(feature.GetIsInIframe(), false);
// Element not in the viewport. // Element not in the viewport.
EXPECT_GT(feature.GetRatioArea(), 0);
EXPECT_FLOAT_EQ(0, feature.GetRatioVisibleArea());
EXPECT_FLOAT_EQ(2, feature.GetRatioDistanceTopToVisibleTop());
EXPECT_LT(2, feature.GetRatioDistanceCenterToVisibleTop());
EXPECT_FLOAT_EQ(2, feature.GetRatioDistanceRootTop());
EXPECT_FLOAT_EQ(10, feature.GetRatioDistanceRootBottom());
EXPECT_FALSE(feature.GetIsInIframe()); EXPECT_FALSE(feature.GetIsInIframe());
EXPECT_FALSE(feature.GetContainsImage()); EXPECT_FALSE(feature.GetContainsImage());
EXPECT_FALSE(feature.GetIsSameHost()); EXPECT_FALSE(feature.GetIsSameHost());
...@@ -223,11 +208,11 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureExtract) { ...@@ -223,11 +208,11 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureExtract) {
auto feature2 = auto feature2 =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_LT(0, feature2.GetRatioVisibleArea()); EXPECT_EQ(feature2.GetIsInIframe(), false);
EXPECT_FLOAT_EQ(0.5, feature2.GetRatioDistanceTopToVisibleTop()); EXPECT_FALSE(feature2.GetIsInIframe());
EXPECT_LT(0.5, feature2.GetRatioDistanceCenterToVisibleTop()); EXPECT_FALSE(feature2.GetContainsImage());
EXPECT_FLOAT_EQ(2, feature2.GetRatioDistanceRootTop()); EXPECT_FALSE(feature2.GetIsSameHost());
EXPECT_FLOAT_EQ(10, feature2.GetRatioDistanceRootBottom()); EXPECT_FALSE(feature2.GetIsUrlIncrementedByOne());
} }
// The main frame contains an iframe. The iframe contains an anchor element. // The main frame contains an iframe. The iframe contains an anchor element.
...@@ -273,11 +258,6 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) { ...@@ -273,11 +258,6 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) {
auto feature = auto feature =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_LT(0, feature.GetRatioArea());
EXPECT_FLOAT_EQ(0, feature.GetRatioVisibleArea());
EXPECT_FLOAT_EQ(2.5, feature.GetRatioDistanceTopToVisibleTop());
EXPECT_LT(2.5, feature.GetRatioDistanceCenterToVisibleTop());
EXPECT_FLOAT_EQ(2.5, feature.GetRatioDistanceRootTop());
EXPECT_TRUE(feature.GetIsInIframe()); EXPECT_TRUE(feature.GetIsInIframe());
EXPECT_FALSE(feature.GetContainsImage()); EXPECT_FALSE(feature.GetContainsImage());
EXPECT_TRUE(feature.GetIsSameHost()); EXPECT_TRUE(feature.GetIsSameHost());
...@@ -290,9 +270,10 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) { ...@@ -290,9 +270,10 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) {
auto feature2 = auto feature2 =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_LT(0, feature2.GetRatioVisibleArea()); EXPECT_TRUE(feature2.GetIsInIframe());
EXPECT_FLOAT_EQ(0.7, feature2.GetRatioDistanceTopToVisibleTop()); EXPECT_FALSE(feature2.GetContainsImage());
EXPECT_FLOAT_EQ(2.5, feature2.GetRatioDistanceRootTop()); EXPECT_TRUE(feature2.GetIsSameHost());
EXPECT_TRUE(feature2.GetIsUrlIncrementedByOne());
// Scroll down inside iframe. Now the anchor element is visible. // Scroll down inside iframe. Now the anchor element is visible.
subframe->View()->LayoutViewport()->SetScrollOffset( subframe->View()->LayoutViewport()->SetScrollOffset(
...@@ -301,11 +282,10 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) { ...@@ -301,11 +282,10 @@ TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframe) {
auto feature3 = auto feature3 =
AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element) AnchorElementMetrics::MaybeReportClickedMetricsOnClick(anchor_element)
.value(); .value();
EXPECT_LT(0, feature3.GetRatioVisibleArea()); EXPECT_TRUE(feature3.GetIsInIframe());
EXPECT_FLOAT_EQ(0.5, feature3.GetRatioDistanceTopToVisibleTop()); EXPECT_FALSE(feature3.GetContainsImage());
EXPECT_FLOAT_EQ(2.5, feature3.GetRatioDistanceRootTop()); EXPECT_TRUE(feature3.GetIsSameHost());
// The distance is expected to be 10.2 - height of the anchor element. EXPECT_TRUE(feature3.GetIsUrlIncrementedByOne());
EXPECT_GT(10.2, feature3.GetRatioDistanceRootBottom());
} }
TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframeNonHttp) { TEST_F(AnchorElementMetricsTest, AnchorFeatureInIframeNonHttp) {
......
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