Closed Bug 1521603 Opened Last year Closed Last year

Add a helper method to check if the top level content document hasn't yet got first contentful paint and there is high prio event pending

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(2 files)

If the method returns true, we probably should yield during page load so that we get first contentful paint asap. So, it will be useful in many places and I have patches using it.

The method name is quite horrible, but hard to have descriptive name.
The high priority event could be of course for something else too, but hopefully the documentation is clear enough.

GetShell() and GetPresContext() are fast inline calls.

Attachment #9038057 - Flags: review?(rjesup)
Comment on attachment 9038057 [details] [diff] [review]
HighPriorityEventPendingForTopLevelDocumentContentfulPaint.diff

Review of attachment 9038057 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsContentUtils.cpp
@@ +10435,5 @@
>    return true;
>  }
>  
> +/* static */
> +bool nsContentUtils::HighPriorityEventPendingForTopLevelDocumentContentfulPaint(

Is this really ....BeforeContentfulPaint()? The current name is confusing

@@ +10438,5 @@
> +/* static */
> +bool nsContentUtils::HighPriorityEventPendingForTopLevelDocumentContentfulPaint(
> +    Document* aDocument) {
> +  if (!aDocument) {
> +    return false;

Is this an expected path? 
Since we dereference immediately, without the nullcheck it would be a clean null dereference, so an ASSERT is quite optional.
Attachment #9038057 - Flags: review?(rjesup) → review+

Null check makes certain call sites simpler.

I'm fine with longer *BeforeContentfulPaint.
All the names are quite bad.

HighPriorityEventPendingForTopLevelDocumentBeforeContentfulPaint
Horribly long name, but oh well.

Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad35cea96d9
Add a helper method to check if the top level content document hasn't yet got first contentful paint and there is high prio event pending, r=jesup
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.