Skip to content

GitLab

T
tangled
Commit 2fe3e831 authored by Nicolás Peña Moreno's avatar Nicolás Peña Moreno Committed by Commit Bot
Browse files
Options

[ElementTiming] Allow empty but present elementtiming attribute 

This CL changes the behavior in cases where the elementtiming attribute
is set but not to a specific value, so that the affected elements are
considered registered for observation. It also adds a test with image
and text testcases. This follow the spec change at
https://github.com/WICG/element-timing/pull/38

Bug: 1011009
Change-Id: If33f55b0992545e6e13f108b923bbcfc546f4d33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871638Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707947}
parent 2d8fd1e8
Hide whitespace changes
Inline Side-by-side
Showing with 51 additions and 13 deletions
third_party/blink/renderer/core/paint/image_element_timing.cc
View file @ 2fe3e831
......@@ -29,13 +29,11 @@ IsExplicitlyRegisteredForTiming(const LayoutObject* layout_object) {
if (!element)
return false;
// If the element has no 'elementtiming' attribute or an empty value, do not
// If the element has no 'elementtiming' attribute, do not
// generate timing entries for the element. See
// https://wicg.github.io/element-timing/#sec-modifications-DOM for report
// vs. ignore criteria.
const AtomicString& attr =
element->FastGetAttribute(html_names::kElementtimingAttr);
return !attr.IsEmpty();
return element->FastHasAttribute(html_names::kElementtimingAttr);
}
} // namespace internal
......
third_party/blink/renderer/core/paint/image_element_timing_test.cc
View file @ 2fe3e831
......@@ -122,13 +122,13 @@ TEST_F(ImageElementTimingTest, TestIsExplicitlyRegisteredForTiming) {
LayoutObject* with_undefined_attribute =
GetLayoutObjectById("unset-attribute");
actual = internal::IsExplicitlyRegisteredForTiming(with_undefined_attribute);
EXPECT_FALSE(actual) << "Nodes with undefined 'elementtiming' attribute "
"should not be explicitly registered.";
EXPECT_TRUE(actual) << "Nodes with undefined 'elementtiming' attribute "
"should be explicitly registered.";
LayoutObject* with_empty_attribute = GetLayoutObjectById("empty-attribute");
actual = internal::IsExplicitlyRegisteredForTiming(with_empty_attribute);
EXPECT_FALSE(actual) << "Nodes with an empty 'elementtiming' attribute "
"should not be explicitly registered.";
EXPECT_TRUE(actual) << "Nodes with an empty 'elementtiming' attribute "
"should be explicitly registered.";
LayoutObject* with_explicit_element_timing =
GetLayoutObjectById("valid-attribute");
......
third_party/blink/renderer/core/paint/text_element_timing.cc
View file @ 2fe3e831
......@@ -65,16 +65,16 @@ void TextElementTiming::OnTextObjectPainted(const TextRecord& record) {
// Text aggregators should be Elements!
DCHECK(node->IsElementNode());
auto* element = To<Element>(node);
const AtomicString& attr =
element->FastGetAttribute(html_names::kElementtimingAttr);
if (attr.IsEmpty())
if (!element->FastHasAttribute(html_names::kElementtimingAttr))
return;
const AtomicString& id = element->GetIdAttribute();
DEFINE_STATIC_LOCAL(const AtomicString, kTextPaint, ("text-paint"));
performance_->AddElementTiming(
kTextPaint, g_empty_string, record.element_timing_rect_,
record.paint_time, base::TimeTicks(), attr, IntSize(), id, element);
record.paint_time, base::TimeTicks(),
element->FastGetAttribute(html_names::kElementtimingAttr), IntSize(), id,
element);
}
void TextElementTiming::Trace(blink::Visitor* visitor) {
......
third_party/blink/renderer/core/paint/text_element_timing.h
View file @ 2fe3e831
......@@ -37,7 +37,7 @@ class CORE_EXPORT TextElementTiming final
static inline bool NeededForElementTiming(Node& node) {
auto* element = DynamicTo<Element>(node);
return !node.IsInShadowTree() && element &&
!element->FastGetAttribute(html_names::kElementtimingAttr).IsEmpty();
element->FastHasAttribute(html_names::kElementtimingAttr);
}
static FloatRect ComputeIntersectionRect(
......
third_party/blink/web_tests/external/wpt/element-timing/observe-empty-attribute.html 0 → 100644
View file @ 2fe3e831
<!DOCTYPE HTML>
<meta charset=utf-8>
<title>Element Timing: observe with empty elementtiming attribute</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/element-timing-helpers.js"></script>
<script>
let beforeRender;
async_test(function (t) {
if (!window.PerformanceElementTiming) {
assert_unreached("PerformanceElementTiming is not implemented");
}
let observedImage = false;
let observedText = false;
const observer = new PerformanceObserver(
t.step_func(function(entryList) {
entryList.getEntries().forEach(entry => {
if (entry.name === 'image-paint') {
assert_false(observedImage, 'Image should only be observed once.');
const pathname = window.location.origin + '/element-timing/resources/square20.png';
checkElement(entry, pathname, '', 'square', beforeRender,
document.getElementById('square'));
checkNaturalSize(entry, 20, 20);
observedImage = true;
} else {
assert_false(observedText, 'Text should only be observed once.');
checkTextElement(entry, '', 'text', beforeRender, document.getElementById('text'));
observedText = true;
}
});
if (observedImage && observedText)
t.done();
})
);
observer.observe({type: 'element', buffered: true});
beforeRender = performance.now();
}, "Able to observe image and text with empty elementtiming attribute.");
</script>
<img id='square' src='resources/square20.png' elementtiming/>
<p id='text' elementtiming>Text!</p>
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