Commit 4a9e20a2 authored by Nicolás Peña Moreno's avatar Nicolás Peña Moreno Committed by Commit Bot

[ElementTiming and LargestContentfulPaint]Fix background img sizes

Currently, we use exclusively the size of the LayoutObject to compute
the background image sizes. This is incorrect, for instance when looking
at CSS styles like ::first-letter. This CL fixes this by passing the
image border to ImagePaintTimingDetector and ImageElementTiming.

Bug: 996921
Change-Id: I94e4a2f3c8873be765b144179f7729430c885dac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023523
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737289}
parent b6fc1e44
......@@ -610,14 +610,16 @@ inline bool PaintFastBottomLayer(Node* node,
if (info.image && info.image->IsImageResource()) {
PaintTimingDetector::NotifyBackgroundImagePaint(
node, image, To<StyleFetchedImage>(info.image),
paint_info.context.GetPaintController().CurrentPaintChunkProperties());
paint_info.context.GetPaintController().CurrentPaintChunkProperties(),
RoundedIntRect(image_border.Rect()));
}
if (node && info.image && info.image->IsImageResource()) {
LocalDOMWindow* window = node->GetDocument().domWindow();
DCHECK(window);
ImageElementTiming::From(*window).NotifyBackgroundImagePainted(
node, To<StyleFetchedImage>(info.image),
context.GetPaintController().CurrentPaintChunkProperties());
context.GetPaintController().CurrentPaintChunkProperties(),
RoundedIntRect(image_border.Rect()));
}
return true;
}
......@@ -740,14 +742,16 @@ void PaintFillLayerBackground(GraphicsContext& context,
if (info.image && info.image->IsImageResource()) {
PaintTimingDetector::NotifyBackgroundImagePaint(
node, image, To<StyleFetchedImage>(info.image),
context.GetPaintController().CurrentPaintChunkProperties());
context.GetPaintController().CurrentPaintChunkProperties(),
EnclosingIntRect(geometry.SnappedDestRect()));
}
if (node && info.image && info.image->IsImageResource()) {
LocalDOMWindow* window = node->GetDocument().domWindow();
DCHECK(window);
ImageElementTiming::From(*window).NotifyBackgroundImagePainted(
node, To<StyleFetchedImage>(info.image),
context.GetPaintController().CurrentPaintChunkProperties());
context.GetPaintController().CurrentPaintChunkProperties(),
EnclosingIntRect(geometry.SnappedDestRect()));
}
}
}
......
......@@ -101,7 +101,7 @@ void ImageElementTiming::NotifyImagePainted(
it->value.is_painted_ = true;
NotifyImagePaintedInternal(layout_object->GetNode(), *layout_object,
*cached_image, current_paint_chunk_properties,
it->value.load_time_);
it->value.load_time_, nullptr);
}
}
......@@ -110,7 +110,8 @@ void ImageElementTiming::NotifyImagePaintedInternal(
const LayoutObject& layout_object,
const ImageResourceContent& cached_image,
const PropertyTreeState& current_paint_chunk_properties,
base::TimeTicks load_time) {
base::TimeTicks load_time,
const IntRect* image_border) {
LocalFrame* frame = GetSupplementable()->GetFrame();
DCHECK(frame == layout_object.GetDocument().GetFrame());
DCHECK(node);
......@@ -138,7 +139,9 @@ void ImageElementTiming::NotifyImagePaintedInternal(
LayoutObject::ShouldRespectImageOrientation(&layout_object);
FloatRect intersection_rect = ElementTimingUtils::ComputeIntersectionRect(
frame, layout_object.FirstFragment().VisualRect(),
frame,
image_border ? *image_border
: layout_object.FragmentsVisualRectBoundingBox(),
current_paint_chunk_properties);
const AtomicString attr =
element->FastGetAttribute(html_names::kElementtimingAttr);
......@@ -205,7 +208,8 @@ void ImageElementTiming::NotifyImagePaintedInternal(
void ImageElementTiming::NotifyBackgroundImagePainted(
Node* node,
const StyleFetchedImage* background_image,
const PropertyTreeState& current_paint_chunk_properties) {
const PropertyTreeState& current_paint_chunk_properties,
const IntRect& image_border) {
DCHECK(node);
DCHECK(background_image);
......@@ -231,7 +235,7 @@ void ImageElementTiming::NotifyBackgroundImagePainted(
info.is_painted_ = true;
NotifyImagePaintedInternal(layout_object->GetNode(), *layout_object,
*cached_image, current_paint_chunk_properties,
it->value);
it->value, &image_border);
}
}
......
......@@ -57,7 +57,8 @@ class CORE_EXPORT ImageElementTiming final
void NotifyBackgroundImagePainted(
Node*,
const StyleFetchedImage* background_image,
const PropertyTreeState& current_paint_chunk_properties);
const PropertyTreeState& current_paint_chunk_properties,
const IntRect& image_border);
void NotifyImageRemoved(const LayoutObject*,
const ImageResourceContent* image);
......@@ -72,7 +73,8 @@ class CORE_EXPORT ImageElementTiming final
const LayoutObject&,
const ImageResourceContent& cached_image,
const PropertyTreeState& current_paint_chunk_properties,
base::TimeTicks load_time);
base::TimeTicks load_time,
const IntRect* image_border);
// Callback for the swap promise. Reports paint timestamps.
void ReportImagePaintSwapTime(WebWidgetClient::SwapResult,
......
......@@ -215,7 +215,8 @@ void ImagePaintTimingDetector::RecordImage(
const IntSize& intrinsic_size,
const ImageResourceContent& cached_image,
const PropertyTreeState& current_paint_chunk_properties,
const StyleFetchedImage* style_image) {
const StyleFetchedImage* style_image,
const IntRect* image_border) {
Node* node = object.GetNode();
if (!node)
return;
......@@ -234,7 +235,8 @@ void ImagePaintTimingDetector::RecordImage(
frame_view_->GetPaintTimingDetector().Visualizer()) {
FloatRect mapped_visual_rect =
frame_view_->GetPaintTimingDetector().CalculateVisualRect(
object.FragmentsVisualRectBoundingBox(),
image_border ? *image_border
: object.FragmentsVisualRectBoundingBox(),
current_paint_chunk_properties);
visualizer->DumpImageDebuggingRect(object, mapped_visual_rect,
cached_image);
......@@ -244,7 +246,8 @@ void ImagePaintTimingDetector::RecordImage(
if (is_recored_visible_image || !is_recording_)
return;
IntRect visual_rect = object.FragmentsVisualRectBoundingBox();
IntRect visual_rect =
image_border ? *image_border : object.FragmentsVisualRectBoundingBox();
// Before the image resource starts loading, <img> has no size info. We wait
// until the size is known.
if (visual_rect.IsEmpty())
......
......@@ -213,7 +213,8 @@ class CORE_EXPORT ImagePaintTimingDetector final
const IntSize& intrinsic_size,
const ImageResourceContent&,
const PropertyTreeState& current_paint_chunk_properties,
const StyleFetchedImage*);
const StyleFetchedImage*,
const IntRect* image_border);
void NotifyImageFinished(const LayoutObject&, const ImageResourceContent*);
void OnPaintFinished();
void LayoutObjectWillBeDestroyed(const LayoutObject&);
......
......@@ -99,7 +99,8 @@ void PaintTimingDetector::NotifyBackgroundImagePaint(
const Node* node,
const Image* image,
const StyleFetchedImage* style_image,
const PropertyTreeState& current_paint_chunk_properties) {
const PropertyTreeState& current_paint_chunk_properties,
const IntRect& image_border) {
DCHECK(image);
DCHECK(style_image->CachedImage());
if (!node)
......@@ -117,7 +118,7 @@ void PaintTimingDetector::NotifyBackgroundImagePaint(
return;
detector.GetImagePaintTimingDetector()->RecordImage(
*object, image->Size(), *style_image->CachedImage(),
current_paint_chunk_properties, style_image);
current_paint_chunk_properties, style_image, &image_border);
}
// static
......@@ -136,7 +137,7 @@ void PaintTimingDetector::NotifyImagePaint(
return;
detector.GetImagePaintTimingDetector()->RecordImage(
object, intrinsic_size, *cached_image, current_paint_chunk_properties,
nullptr);
nullptr, nullptr);
}
void PaintTimingDetector::NotifyImageFinished(
......
......@@ -124,7 +124,8 @@ class CORE_EXPORT PaintTimingDetector
const Node*,
const Image*,
const StyleFetchedImage*,
const PropertyTreeState& current_paint_chunk_properties);
const PropertyTreeState& current_paint_chunk_properties,
const IntRect& image_border);
static void NotifyImagePaint(
const LayoutObject&,
const IntSize& intrinsic_size,
......
......@@ -6,6 +6,9 @@
#target::first-letter {
background-image: url('/images/black-rectangle.png');
}
#target {
font-size: 12px;
}
</style>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
......@@ -17,12 +20,20 @@
const div = document.getElementById('target');
let textObserved = false;
let imageObserved = false;
function calculateSize(entry) {
const ir = entry.intersectionRect;
return (ir.right - ir.left) * (ir.bottom - ir.top);
}
const observer = new PerformanceObserver(
t.step_func(function(entryList) {
entryList.getEntries().forEach(entry => {
if (entry.name === 'text-paint') {
checkTextElement(entry, 'my_div', 'target', beforeRender, div);
textObserved = true;
const size = calculateSize(entry);
// Assume average width is between 4px and 15px.
// Therefore, text size must be between 12 * (35*4) and 12 * (35*15).
assert_between_inclusive(size, 1680, 6300);
}
else {
assert_equals(entry.name, 'image-paint');
......@@ -30,6 +41,9 @@
checkElement(entry, pathname, 'my_div', 'target', beforeRender, div);
checkNaturalSize(entry, 100, 50);
imageObserved = true;
const size = calculateSize(entry);
// Background image size should only be approximately the size of a single letter.
assert_between_inclusive(size, 48, 180);
}
});
if (textObserved && imageObserved)
......@@ -39,5 +53,5 @@
observer.observe({entryTypes: ['element']});
}, 'Element with elementtiming attribute and background image in first-letter is observable.');
</script>
<div id='target' elementtiming='my_div'>A</div>
<div id='target' elementtiming='my_div'>This is some text that I care about</div>
</body>
......@@ -3,9 +3,12 @@
<title>Largest Contentful Paint: observe element with background image in its first letter</title>
<body>
<style>
#target::first-letter {
div::first-letter {
background-image: url('/images/black-rectangle.png');
}
div {
font-size: 12px;
}
</style>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
......@@ -13,17 +16,56 @@
<script>
async_test(function (t) {
assert_precondition(window.LargestContentfulPaint, "LargestContentfulPaint is not implemented");
const beforeLoad = performance.now();
let beforeLoad = performance.now();
let observedFirstLetter = false;
const observer = new PerformanceObserver(
t.step_func(function(entryList) {
entryList.getEntries().forEach(entry => {
const entry = entryList.getEntries()[entryList.getEntries().length -1];
if (!observedFirstLetter) {
// When we haven't observed first-letter as LCP...
// If we happen to get a text entry due to text happening before the image, return.
if (entry.url === '')
if (entry.url === '') {
assert_equals(entry.entryType, 'largest-contentful-paint');
assert_greater_than_equal(entry.renderTime, beforeLoad);
assert_greater_than_equal(performance.now(), entry.renderTime);
assert_equals(entry.startTime, entry.renderTime, 'startTime should equal renderTime');
assert_equals(entry.duration, 0);
assert_equals(entry.loadTime, 0);
assert_equals(entry.id, 'target');
assert_equals(entry.element, document.getElementById('target'));
} else {
const url = window.location.origin + '/images/black-rectangle.png';
checkImage(entry, url, 'target', 0, beforeLoad, ['sizeLowerBound']);
}
// Now change the div content to proceed to the second part of the test.
beforeLoad = performance.now();
const div = document.createElement('div');
div.id = 'target2';
div.innerHTML = 'long text will now be LCP';
document.body.appendChild(div);
observedFirstLetter = true;
} else {
// Ignore entries that are caused by the initial 'target'.
if (entry.id === 'target')
return;
const url = window.location.origin + '/images/black-rectangle.png';
checkImage(entry, url, 'target', 0, beforeLoad, ['sizeLowerBound']);
// The LCP must now be text.
if (entry.url !== '')
assert_unreached('First-letter background should not be LCP!');
assert_equals(entry.entryType, 'largest-contentful-paint');
assert_greater_than_equal(entry.renderTime, beforeLoad, 'blaaa');
assert_greater_than_equal(performance.now(), entry.renderTime, 'bleee');
assert_equals(entry.startTime, entry.renderTime, 'startTime should equal renderTime');
assert_equals(entry.duration, 0);
assert_equals(entry.id, 'target2');
const div = document.getElementById('target2');
// Estimate the text size: 12 * 100
assert_greater_than_equal(entry.size, 1200);
assert_equals(entry.loadTime, 0);
assert_equals(entry.element, div);
t.done();
})
}
}));
observer.observe({entryTypes: ['largest-contentful-paint']});
}, 'Largest Contentful Paint: first-letter is observable.');
......
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