Closed Bug 1586381 Opened 6 years ago Closed 6 years ago

[wpt-sync] Sync PR 19525 - [ElementTiming] Report text paints of size 0

Categories

(Core :: Performance: General, task, P4)

Product:
Core
Component:
Performance: General
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla72
Tracking Flags:
Tracking Status
firefox72 --- fixed

People

(Reporter: wpt-sync, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 19525 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/19525
Details from upstream follow.

Nicolás Peña Moreno <npm@chromium.org> wrote:

[ElementTiming] Report text paints of size 0

Currently, TextPaintTimingDetector uses |invisible_objects_| to keep
track of text paints which occupy a size of 0. It is an optimization:
LCP does not care about these paints, so it does not need to queue
them to obtain paint times nor store a full TextRecord for them. However
ElementTiming wants to expose all of the text paints for elements that
are annotated with the elementtiming attribute. It is confusing for an
element that is painted when outside the viewport to never be reported.

To solve this problem, there are two alternatives. One is to use a
single |painted_objects_| map which would contain both the visible and
the invisible objects, and hence no extra logic would be needed for
ElementTimng to receive these objects (but some care might be needed so
LCP does not report objects of size 0). This is simpler to code but is
less efficient as it adds extra work and memory for the invisible
objects.

The second alternative is to use a new data structure for text nodes
that are of size 0 but need to be reported to ElementTiming. Paint times
for these are assigned in AssignPaintTimeToQueuedRecords() and then the
TextRecords are deleted (the fact that the element has been painted is
still being handled by |invisible_objects_| so there is no need to keep
the records for longer). This is the alternative implemented in this CL.

Bug: 1011009
Change-Id: I2d15393fc61134b08a5c15bd81d062d42dfb29e4
Reviewed-on: https://chromium-review.googlesource.com/1838260
WPT-Export-Revision: dbf06ddb8856271290377e8d19053916e0880713

Component: web-platform-tests → Performance
Product: Testing → Core
## GitHub CI Results wpt.fyi [PR Results](https://wpt.fyi/results/?sha=07ed08ae2a4ddfae600cb1aefd5a3a757e5deee7&label=pr_head) [Base Results](https://wpt.fyi/results/?sha=07ed08ae2a4ddfae600cb1aefd5a3a757e5deee7&label=pr_base) Ran 1 tests and 1 subtests ### Firefox OK : 1 FAIL: 1 ### Chrome OK : 1 PASS: 1 ### Safari OK : 1 FAIL: 1 ### New tests that's don't pass /element-timing/scroll-to-text.html Paragraph with elementtiming attribute is observed even when not initially visible.: Firefox: FAIL, Chrome: PASS, Safari: FAIL
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/mozilla-inbound/rev/5b28db890a4d [wpt PR 19525] - [ElementTiming] Report text paints of size 0, a=testonly https://hg.mozilla.org/integration/mozilla-inbound/rev/8aea25c897c7 [wpt PR 19525] - Update wpt metadata, a=testonly

https://hg.mozilla.org/mozilla-central/rev/5b28db890a4d
https://hg.mozilla.org/mozilla-central/rev/8aea25c897c7

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox72: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.