Skip to content

GitLab

T
tangled
Commit 4ad8268c authored by suzyh's avatar suzyh Committed by Commit bot
Browse files
Options

Move CSS animations use counters to UseCounter 

The use counts for animated CSS properties currently uses a custom
histogram which counts a property once per animation. This patch moves
the use counts to the existing UseCounter framework in order to bring
the counts in line with the CSS property UseCounters. In particular,
this is to ensure that properties are counted once per page instead of
once per animation.

This patch:
- adds a BitVector and corresponding functions to UseCounter
- adds test cases to UseCounterTest, this time with an optional
  legacyHistogram name
- exposes the new 'is counted' UseCounter function to Internals for
  testing
- adds layout tests to verify that the animated CSS property UseCounter
  has been correctly incremented for animations (standard and custom
  CSS properties) and transitions (standard CSS properties only).

One of the layout tests is failing because the standard CSS property
UseCounter currently excludes custom properties. This will be fixed in
a future patch.

BUG=458925

Review-Url: https://codereview.chromium.org/2678143003
Cr-Commit-Position: refs/heads/master@{#455978}
parent ae4a9e6d
Hide whitespace changes
Inline Side-by-side
Showing with 310 additions and 22 deletions
third_party/WebKit/LayoutTests/animations/animated-css-property-usecounter-expected.txt 0 → 100644
View file @ 4ad8268c
This is a testharness.js-based test.
PASS Animating properties via CSS causes UseCounter to be incremented.
FAIL Animating custom CSS properties causes UseCounter to be incremented. assert_true: Usage of custom property --x in style causes it to be counted in normal CSS property UseCounter expected true got false
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/animations/animated-css-property-usecounter-for-transitions.html 0 → 100644
View file @ 4ad8268c
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
#target {
width: 100px;
height: 100px;
transition: width 1s;
}
</style>
<div id="target"></div>
<script>
'use strict';
function waitForProgress() {
var initialWidth = getComputedStyle(target).width;
return new Promise(resolve => {
function poll() {
var width = getComputedStyle(target).width;
if (width === initialWidth) {
requestAnimationFrame(poll);
} else {
resolve();
}
}
requestAnimationFrame(poll);
});
}
async_test(t => {
assert_true(
internals.isCSSPropertyUseCounted(document, "width"),
'Usage of width in style causes it to be counted in normal CSS ' +
'property UseCounter');
assert_true(
internals.isCSSPropertyUseCounted(document, "height"),
'Usage of height in style causes it to be counted in normal CSS ' +
'property UseCounter');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "width"),
'Initially width animation has not been UseCounted');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "height"),
'Initially height animation has not been UseCounted');
target.offsetTop; // force recalc
target.style.width = '200px';
waitForProgress().then(t.step_func_done(() => {
assert_true(
internals.isAnimatedCSSPropertyUseCounted(document, "width"),
'After triggering the transition, width has been counted');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "height"),
'Height is not animated, so not counted');
}));
}, 'Using CSS transitions causes UseCounter to be incremented.');
</script>
third_party/WebKit/LayoutTests/animations/animated-css-property-usecounter.html 0 → 100644
View file @ 4ad8268c
<!DOCTYPE html>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
@keyframes bgColorAnim {
from {
background-color: blue;
}
to {
background-color: red;
}
}
@keyframes customPropertyAnim {
from {
--x: blue;
}
to {
--x: red;
}
}
#target {
width: 100px;
height: 100px;
}
</style>
<div id="target"></div>
<script>
'use strict';
test(() => {
assert_true(
internals.isCSSPropertyUseCounted(document, "background-color"),
'Usage of background-color in style causes it to be counted in ' +
'normal CSS property UseCounter');
assert_true(
internals.isCSSPropertyUseCounted(document, "width"),
'Usage of width in style causes it to be counted in normal CSS ' +
'property UseCounter');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "background-color"),
'Initially background-color animation has not been UseCounted');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "width"),
'Initially width animation has not been UseCounted');
target.style.animation = 'bgColorAnim 1s';
target.offsetTop; // force recalc
assert_true(
internals.isAnimatedCSSPropertyUseCounted(document, "background-color"),
'After applying the animation, background-color has been counted');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "width"),
'Width is not animated, so not counted');
}, 'Animating properties via CSS causes UseCounter to be incremented.');
test(() => {
assert_true(
internals.isCSSPropertyUseCounted(document, "--x"),
'Usage of custom property --x in style causes it to be counted in ' +
'normal CSS property UseCounter');
assert_true(
internals.isCSSPropertyUseCounted(document, "--y"),
'All custom properties are counted together in normal CSS property ' +
'UseCounter');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "--x"),
'Initially custom property --x animation has not been UseCounted');
assert_false(
internals.isAnimatedCSSPropertyUseCounted(document, "--y"),
'Initially custom property --y animation has not been UseCounted');
target.style.animation = 'customPropertyAnim 1s';
target.offsetTop; // force recalc
assert_true(
internals.isAnimatedCSSPropertyUseCounted(document, "--x"),
'After applying the animation, custom property animation has been counted');
assert_true(
internals.isAnimatedCSSPropertyUseCounted(document, "--y"),
'All custom property animations are counted together');
}, 'Animating custom CSS properties causes UseCounter to be incremented.');
</script>
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp
View file @ 4ad8268c
......@@ -134,6 +134,9 @@ static StringKeyframeEffectModel* createKeyframeEffectModel(
("WebCore.Animation.CSSProperties"));
for (CSSPropertyID property : specifiedPropertiesForUseCounter) {
DCHECK(isValidCSSPropertyID(property));
UseCounter::countAnimatedCSS(elementForScoping->document(), property);
// TODO(crbug.com/458925): Remove legacy histogram and counts
propertyHistogram.sample(
UseCounter::mapCSSPropertyIdToCSSSampleIdForHistogram(property));
}
......@@ -602,7 +605,9 @@ void CSSAnimations::maybeApplyPendingUpdate(Element* element) {
runningTransition.animation = animation;
m_transitions.set(property, runningTransition);
DCHECK(isValidCSSPropertyID(property.cssProperty()));
UseCounter::countAnimatedCSS(element->document(), property.cssProperty());
// TODO(crbug.com/458925): Remove legacy histogram and counts
DEFINE_STATIC_LOCAL(SparseHistogram, propertyHistogram,
("WebCore.Animation.CSSProperties"));
propertyHistogram.sample(
......
third_party/WebKit/Source/core/frame/UseCounter.cpp
View file @ 4ad8268c
......@@ -1103,7 +1103,8 @@ UseCounter::UseCounter(Context context)
m_disableReporting(false),
m_context(context),
m_featuresRecorded(NumberOfFeatures),
m_CSSRecorded(numCSSPropertyIDs) {}
m_CSSRecorded(numCSSPropertyIDs),
m_animatedCSSRecorded(numCSSPropertyIDs) {}
void UseCounter::muteForInspector() {
m_muteCount++;
......@@ -1168,9 +1169,11 @@ void UseCounter::didCommitLoad(KURL url) {
m_featuresRecorded.clearAll();
m_CSSRecorded.clearAll();
m_animatedCSSRecorded.clearAll();
if (!m_disableReporting && !m_muteCount) {
featuresHistogram().count(PageVisits);
cssHistogram().count(totalPagesMeasuredCSSSampleId());
animatedCSSHistogram().count(totalPagesMeasuredCSSSampleId());
}
}
......@@ -1234,6 +1237,7 @@ void UseCounter::countCrossOriginIframe(const Document& document,
}
void UseCounter::count(CSSParserMode cssParserMode, CSSPropertyID property) {
// FIXME(suzyh): Count custom properties as well as named properties
DCHECK(isCSSPropertyIDWithName(property));
if (!isUseCounterEnabledForMode(cssParserMode) || m_muteCount)
......@@ -1257,6 +1261,48 @@ void UseCounter::count(Feature feature) {
recordMeasurement(feature);
}
bool UseCounter::isCountedAnimatedCSS(CSSPropertyID unresolvedProperty) {
return m_animatedCSSRecorded.quickGet(unresolvedProperty);
}
bool UseCounter::isCountedAnimatedCSS(Document& document,
const String& string) {
Page* page = document.page();
if (!page)
return false;
CSSPropertyID unresolvedProperty = unresolvedCSSPropertyID(string);
if (unresolvedProperty == CSSPropertyInvalid)
return false;
return page->useCounter().isCountedAnimatedCSS(unresolvedProperty);
}
void UseCounter::countAnimatedCSS(const Document& document,
CSSPropertyID property) {
Page* page = document.page();
if (!page)
return;
page->useCounter().countAnimatedCSS(property);
}
void UseCounter::countAnimatedCSS(CSSPropertyID property) {
DCHECK(isCSSPropertyIDWithName(property) || property == CSSPropertyVariable);
if (m_muteCount)
return;
if (!m_animatedCSSRecorded.quickGet(property)) {
int sampleId = mapCSSPropertyIdToCSSSampleIdForHistogram(property);
if (!m_disableReporting) {
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.feature_usage"),
"AnimatedCSSFirstUsed", "feature", sampleId);
animatedCSSHistogram().count(sampleId);
}
m_animatedCSSRecorded.quickSet(property);
}
}
void UseCounter::notifyFeatureCounted(Feature feature) {
DCHECK(!m_muteCount);
DCHECK(!m_disableReporting);
......@@ -1295,6 +1341,17 @@ EnumerationHistogram& UseCounter::cssHistogram() const {
return m_context == SVGImageContext ? svgHistogram : histogram;
}
EnumerationHistogram& UseCounter::animatedCSSHistogram() const {
DEFINE_STATIC_LOCAL(
blink::EnumerationHistogram, histogram,
("Blink.UseCounter.AnimatedCSSProperties", kMaximumCSSSampleId));
DEFINE_STATIC_LOCAL(
blink::EnumerationHistogram, svgHistogram,
("Blink.UseCounter.SVGImage.AnimatedCSSProperties", kMaximumCSSSampleId));
return m_context == SVGImageContext ? svgHistogram : histogram;
}
/*
*
* LEGACY metrics support - WebCore.FeatureObserver is to be superceded by
......
third_party/WebKit/Source/core/frame/UseCounter.h
View file @ 4ad8268c
......@@ -1512,6 +1512,9 @@ class CORE_EXPORT UseCounter {
void count(CSSParserMode, CSSPropertyID);
void count(Feature);
static void countAnimatedCSS(const Document&, CSSPropertyID);
void countAnimatedCSS(CSSPropertyID);
// Count only features if they're being used in an iframe which does not
// have script access into the top level document.
static void countCrossOriginIframe(const Document&, Feature);
......@@ -1524,6 +1527,11 @@ class CORE_EXPORT UseCounter {
static bool isCounted(Document&, const String&);
bool isCounted(CSSPropertyID unresolvedProperty);
// Return whether the CSSPropertyID was previously counted for this document.
// NOTE: only for use in testing.
static bool isCountedAnimatedCSS(Document&, const String&);
bool isCountedAnimatedCSS(CSSPropertyID unresolvedProperty);
// Retains a reference to the observer to notify of UseCounter changes.
void addObserver(Observer*);
......@@ -1553,6 +1561,7 @@ class CORE_EXPORT UseCounter {
EnumerationHistogram& featuresHistogram() const;
EnumerationHistogram& cssHistogram() const;
EnumerationHistogram& animatedCSSHistogram() const;
// If non-zero, ignore all 'count' calls completely.
unsigned m_muteCount;
......@@ -1567,6 +1576,7 @@ class CORE_EXPORT UseCounter {
// histograms.
BitVector m_featuresRecorded;
BitVector m_CSSRecorded;
BitVector m_animatedCSSRecorded;
HeapHashSet<Member<Observer>> m_observers;
......
third_party/WebKit/Source/core/frame/UseCounterTest.cpp
View file @ 4ad8268c
......@@ -12,13 +12,18 @@
#include "testing/gtest/include/gtest/gtest.h"
namespace {
// Note that the new histogram names will change once the semantics stabilize;
const char* const kFeaturesHistogramName = "Blink.UseCounter.Features";
const char* const kCSSHistogramName = "Blink.UseCounter.CSSProperties";
const char* const kAnimatedCSSHistogramName =
"Blink.UseCounter.AnimatedCSSProperties";
const char* const kSVGFeaturesHistogramName =
"Blink.UseCounter.SVGImage.Features";
const char* const kSVGCSSHistogramName =
"Blink.UseCounter.SVGImage.CSSProperties";
const char* const kSVGAnimatedCSSHistogramName =
"Blink.UseCounter.SVGImage.AnimatedCSSProperties";
const char* const kLegacyFeaturesHistogramName = "WebCore.FeatureObserver";
const char* const kLegacyCSSHistogramName =
"WebCore.FeatureObserver.CSSProperties";
......@@ -45,12 +50,16 @@ void histogramBasicTest(const std::string& histogram,
count(item);
EXPECT_TRUE(counted(item));
histogramTester.expectUniqueSample(histogram, histogramMap(item), 1);
histogramTester.expectTotalCount(legacyHistogram, 0);
if (!legacyHistogram.empty()) {
histogramTester.expectTotalCount(legacyHistogram, 0);
}
// Test that repeated measurements have no effect
count(item);
histogramTester.expectUniqueSample(histogram, histogramMap(item), 1);
histogramTester.expectTotalCount(legacyHistogram, 0);
if (!legacyHistogram.empty()) {
histogramTester.expectTotalCount(legacyHistogram, 0);
}
// Test recording a different sample
EXPECT_FALSE(counted(secondItem));
......@@ -59,7 +68,9 @@ void histogramBasicTest(const std::string& histogram,
histogramTester.expectBucketCount(histogram, histogramMap(item), 1);
histogramTester.expectBucketCount(histogram, histogramMap(secondItem), 1);
histogramTester.expectTotalCount(histogram, 2);
histogramTester.expectTotalCount(legacyHistogram, 0);
if (!legacyHistogram.empty()) {
histogramTester.expectTotalCount(legacyHistogram, 0);
}
// After a page load, the histograms will be updated, even when the URL
// scheme is internal
......@@ -70,11 +81,13 @@ void histogramBasicTest(const std::string& histogram,
histogramTester.expectTotalCount(histogram, 3);
// And verify the legacy histogram now looks the same
histogramTester.expectBucketCount(legacyHistogram, histogramMap(item), 1);
histogramTester.expectBucketCount(legacyHistogram, histogramMap(secondItem),
1);
histogramTester.expectBucketCount(legacyHistogram, pageVisitBucket, 1);
histogramTester.expectTotalCount(legacyHistogram, 3);
if (!legacyHistogram.empty()) {
histogramTester.expectBucketCount(legacyHistogram, histogramMap(item), 1);
histogramTester.expectBucketCount(legacyHistogram, histogramMap(secondItem),
1);
histogramTester.expectBucketCount(legacyHistogram, pageVisitBucket, 1);
histogramTester.expectTotalCount(legacyHistogram, 3);
}
// Now a repeat measurement should get recorded again, exactly once
EXPECT_FALSE(counted(item));
......@@ -86,11 +99,13 @@ void histogramBasicTest(const std::string& histogram,
// And on the next page load, the legacy histogram will again be updated
didCommitLoad(URLTestHelpers::toKURL(url));
histogramTester.expectBucketCount(legacyHistogram, histogramMap(item), 2);
histogramTester.expectBucketCount(legacyHistogram, histogramMap(secondItem),
1);
histogramTester.expectBucketCount(legacyHistogram, pageVisitBucket, 2);
histogramTester.expectTotalCount(legacyHistogram, 5);
if (!legacyHistogram.empty()) {
histogramTester.expectBucketCount(legacyHistogram, histogramMap(item), 2);
histogramTester.expectBucketCount(legacyHistogram, histogramMap(secondItem),
1);
histogramTester.expectBucketCount(legacyHistogram, pageVisitBucket, 2);
histogramTester.expectTotalCount(legacyHistogram, 5);
}
for (size_t i = 0; i < unaffectedHistograms.size(); ++i) {
histogramTester.expectTotalCount(unaffectedHistograms[i], 0);
......@@ -107,8 +122,9 @@ TEST(UseCounterTest, MAYBE_RecordingFeatures) {
UseCounter useCounter;
histogramBasicTest<UseCounter::Feature>(
kFeaturesHistogramName, kLegacyFeaturesHistogramName,
{kSVGFeaturesHistogramName, kSVGCSSHistogramName}, UseCounter::Fetch,
UseCounter::FetchBodyStream,
{kSVGFeaturesHistogramName, kSVGCSSHistogramName,
kSVGAnimatedCSSHistogramName},
UseCounter::Fetch, UseCounter::FetchBodyStream,
[&](UseCounter::Feature feature) -> bool {
return useCounter.hasRecordedMeasurement(feature);
},
......@@ -124,8 +140,9 @@ TEST(UseCounterTest, RecordingCSSProperties) {
UseCounter useCounter;
histogramBasicTest<CSSPropertyID>(
kCSSHistogramName, kLegacyCSSHistogramName,
{kSVGFeaturesHistogramName, kSVGCSSHistogramName}, CSSPropertyFont,
CSSPropertyZoom,
{kSVGFeaturesHistogramName, kSVGCSSHistogramName,
kSVGAnimatedCSSHistogramName},
CSSPropertyFont, CSSPropertyZoom,
[&](CSSPropertyID property) -> bool {
return useCounter.isCounted(property);
},
......@@ -139,6 +156,23 @@ TEST(UseCounterTest, RecordingCSSProperties) {
"https://dummysite.com/", 1 /* page visit bucket */);
}
TEST(UseCounterTest, RecordingAnimatedCSSProperties) {
UseCounter useCounter;
histogramBasicTest<CSSPropertyID>(
kAnimatedCSSHistogramName, "",
{kSVGCSSHistogramName, kSVGAnimatedCSSHistogramName}, CSSPropertyOpacity,
CSSPropertyVariable,
[&](CSSPropertyID property) -> bool {
return useCounter.isCountedAnimatedCSS(property);
},
[&](CSSPropertyID property) { useCounter.countAnimatedCSS(property); },
[](CSSPropertyID property) -> int {
return UseCounter::mapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
[&](KURL kurl) { useCounter.didCommitLoad(kurl); },
"https://dummysite.com/", 1 /* page visit bucket */);
}
// Failing on Android: crbug.com/667913
#if OS(ANDROID)
#define MAYBE_SVGImageContextFeatures DISABLED_SVGImageContextFeatures
......@@ -149,7 +183,7 @@ TEST(UseCounterTest, MAYBE_SVGImageContextFeatures) {
UseCounter useCounter(UseCounter::SVGImageContext);
histogramBasicTest<UseCounter::Feature>(
kSVGFeaturesHistogramName, kLegacyFeaturesHistogramName,
{kFeaturesHistogramName, kCSSHistogramName},
{kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
UseCounter::SVGSMILAdditiveAnimation,
UseCounter::SVGSMILAnimationElementTiming,
[&](UseCounter::Feature feature) -> bool {
......@@ -168,8 +202,8 @@ TEST(UseCounterTest, SVGImageContextCSSProperties) {
UseCounter useCounter(UseCounter::SVGImageContext);
histogramBasicTest<CSSPropertyID>(
kSVGCSSHistogramName, kLegacyCSSHistogramName,
{kFeaturesHistogramName, kCSSHistogramName}, CSSPropertyFont,
CSSPropertyZoom,
{kFeaturesHistogramName, kCSSHistogramName, kAnimatedCSSHistogramName},
CSSPropertyFont, CSSPropertyZoom,
[&](CSSPropertyID property) -> bool {
return useCounter.isCounted(property);
},
......@@ -184,6 +218,24 @@ TEST(UseCounterTest, SVGImageContextCSSProperties) {
1 /* page visit bucket */);
}
TEST(UseCounterTest, SVGImageContextAnimatedCSSProperties) {
UseCounter useCounter(UseCounter::SVGImageContext);
histogramBasicTest<CSSPropertyID>(
kSVGAnimatedCSSHistogramName, "",
{kCSSHistogramName, kAnimatedCSSHistogramName}, CSSPropertyOpacity,
CSSPropertyVariable,
[&](CSSPropertyID property) -> bool {
return useCounter.isCountedAnimatedCSS(property);
},
[&](CSSPropertyID property) { useCounter.countAnimatedCSS(property); },
[](CSSPropertyID property) -> int {
return UseCounter::mapCSSPropertyIdToCSSSampleIdForHistogram(property);
},
[&](KURL kurl) { useCounter.didCommitLoad(kurl); }, "about:blank",
// In practice SVGs always appear to be loaded with an about:blank URL
1 /* page visit bucket */);
}
// Failing on Android: crbug.com/667913
#if OS(ANDROID)
#define MAYBE_InspectorDisablesMeasurement DISABLED_InspectorDisablesMeasurement
......
third_party/WebKit/Source/core/testing/Internals.cpp
View file @ 4ad8268c
......@@ -3113,6 +3113,11 @@ bool Internals::isCSSPropertyUseCounted(Document* document,
return UseCounter::isCounted(*document, propertyName);
}
bool Internals::isAnimatedCSSPropertyUseCounted(Document* document,
const String& propertyName) {
return UseCounter::isCountedAnimatedCSS(*document, propertyName);
}
ScriptPromise Internals::observeUseCounter(ScriptState* scriptState,
Document* document,
uint32_t feature) {
......
third_party/WebKit/Source/core/testing/Internals.h
View file @ 4ad8268c
......@@ -499,6 +499,7 @@ class Internals final : public GarbageCollected<Internals>,
// |feature| must be one of the values from the UseCounter::Feature enum.
bool isUseCounted(Document*, uint32_t feature);
bool isCSSPropertyUseCounted(Document*, const String&);
bool isAnimatedCSSPropertyUseCounted(Document*, const String&);
// Observes changes on Document's UseCounter. Returns a promise that is
// resolved when |feature| is counted. When |feature| was already counted,
......
third_party/WebKit/Source/core/testing/Internals.idl
View file @ 4ad8268c
......@@ -337,6 +337,7 @@
boolean isUseCounted(Document document, unsigned long feature);
boolean isCSSPropertyUseCounted(Document document, DOMString propertyName);
boolean isAnimatedCSSPropertyUseCounted(Document document, DOMString propertyName);
// Returns a promise that is resolved when |feature| is counted on
// |document|'s UseCounter. When |feature| was already counted, it's
......
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