Commit 7c4439ab authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot

[CSS Env Vars] Do not record metrics on media controls

The media controls are using env() which is causing
pages using the media controls to record UseCounter
metrics for env().

This CL silences the metrics if the TreeScope when
resolving the variables is a UA shadow root.

BUG=855739

Change-Id: I48a17e426511021e29d6ff6d4f5e535eafbd2b24
Reviewed-on: https://chromium-review.googlesource.com/1123299
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572028}
parent 1dcf2e85
......@@ -42,15 +42,22 @@ DocumentStyleEnvironmentVariables::Create(StyleEnvironmentVariables& parent,
}
CSSVariableData* DocumentStyleEnvironmentVariables::ResolveVariable(
const AtomicString& name) {
const AtomicString& name,
bool record_metrics) {
unsigned id = GenerateHashFromName(name);
RecordVariableUsage(id);
if (record_metrics)
RecordVariableUsage(id);
// Mark the variable as seen so we will invalidate the style if we change it.
seen_variables_.insert(id);
return StyleEnvironmentVariables::ResolveVariable(name);
}
CSSVariableData* DocumentStyleEnvironmentVariables::ResolveVariable(
const AtomicString& name) {
return ResolveVariable(name, true /* record_metrics */);
}
void DocumentStyleEnvironmentVariables::InvalidateVariable(
const AtomicString& name) {
DCHECK(document_);
......
......@@ -28,7 +28,14 @@ class CORE_EXPORT DocumentStyleEnvironmentVariables
// Resolve the variable |name| and return the data. This will also cause
// future changes to this variable to invalidate the associated document's
// style.
// style. If |record_metrics| is true we will record UseCounter metrics when
// this function is called.
CSSVariableData* ResolveVariable(const AtomicString& name,
bool record_metrics);
// Resolve the variable |name| and return the data. This will also cause
// future changes to this variable to invalidate the associated document's
// style. UseCounter metrics will be recorded when this function is used.
CSSVariableData* ResolveVariable(const AtomicString& name) override;
protected:
......
......@@ -21,6 +21,7 @@
#include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/css_property_names.h"
#include "third_party/blink/renderer/core/css_value_keywords.h"
#include "third_party/blink/renderer/core/dom/shadow_root.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/style_inherited_variables.h"
#include "third_party/blink/renderer/core/style/style_non_inherited_variables.h"
......@@ -240,10 +241,15 @@ bool CSSVariableResolver::ResolveVariableReference(
CSSVariableData* CSSVariableResolver::ValueForEnvironmentVariable(
const AtomicString& name) {
// If we are in a User Agent Shadow DOM then we should not record metrics.
ContainerNode& scope_root = state_.GetTreeScope().RootNode();
bool is_ua_scope =
scope_root.IsShadowRoot() && ToShadowRoot(scope_root).IsUserAgent();
return state_.GetDocument()
.GetStyleEngine()
.EnsureEnvironmentVariables()
.ResolveVariable(name);
.ResolveVariable(name, !is_ua_scope);
}
bool CSSVariableResolver::ResolveTokenRange(
......
......@@ -39,6 +39,9 @@ class StyleEnvironmentVariablesTest : public PageTestBase {
PageTestBase::SetUp();
RuntimeEnabledFeatures::SetCSSEnvironmentVariablesEnabled(true);
// Needed for RecordUseCounter_IgnoreMediaControls.
RuntimeEnabledFeatures::SetModernMediaControlsEnabled(true);
}
void TearDown() override {
......@@ -337,6 +340,21 @@ TEST_F(StyleEnvironmentVariablesTest,
}
}
TEST_F(StyleEnvironmentVariablesTest, RecordUseCounter_IgnoreMediaControls) {
InitializeWithHTML(GetFrame(), "<video controls />");
EXPECT_FALSE(UseCounter::IsCounted(GetDocument(),
WebFeature::kCSSEnvironmentVariable));
EXPECT_FALSE(UseCounter::IsCounted(
GetDocument(), WebFeature::kCSSEnvironmentVariable_SafeAreaInsetTop));
EXPECT_FALSE(UseCounter::IsCounted(
GetDocument(), WebFeature::kCSSEnvironmentVariable_SafeAreaInsetLeft));
EXPECT_FALSE(UseCounter::IsCounted(
GetDocument(), WebFeature::kCSSEnvironmentVariable_SafeAreaInsetBottom));
EXPECT_FALSE(UseCounter::IsCounted(
GetDocument(), WebFeature::kCSSEnvironmentVariable_SafeAreaInsetRight));
}
TEST_F(StyleEnvironmentVariablesTest, RecordUseCounter_InvalidProperty) {
InitializeTestPageWithVariableNamed(GetFrame(), kVariableName);
EXPECT_TRUE(UseCounter::IsCounted(GetDocument(),
......
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