Start RefreshDriver timer very early

RESOLVED FIXED in Firefox 67

Status

()

enhancement
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

(Blocks 1 bug)

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In order to be able to do the first contentful paint as soon as possible in the current painting setup, we need to get vsync messages from compositor asap.
So, better to ensure RefreshDriver is up and running in child processes.
Adding an "early runner" is enough to trigger timer.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e378d78f1206689bb6cb605c44d5b6e12b2c007d

Posted patch early_rf.diff (obsolete) — Splinter Review

(no idea who should review this)

Attachment #9041022 - Flags: review?(emilio)
Comment on attachment 9041022 [details] [diff] [review]
early_rf.diff

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

Why not making EnsureTimerStarted public instead?

Also, why not in the parent process? I assume too many borked chrome tests or something?

We want stuff there in RefreshDriver to keep it running https://searchfox.org/mozilla-central/source/layout/base/nsRefreshDriver.cpp#1418
And I don't want to change timing of browser startup or window opening.

I guess I could make EnsureTimerStarted public.

(calling it in nsRefreshDriver ctor would be scary, since refcnt is still 0 at that point)

(In reply to Olli Pettay [:smaug] (massive needinfo queue, ping on IRC on anything urgent) from comment #5)

We want stuff there in RefreshDriver to keep it running https://searchfox.org/mozilla-central/source/layout/base/nsRefreshDriver.cpp#1418
And I don't want to change timing of browser startup or window opening.

Shouldn't it keep running regardless given1? I don't see why the early runner would be needed.

(In reply to Olli Pettay [:smaug] (massive needinfo queue, ping on IRC on anything urgent) from comment #6)

I guess I could make EnsureTimerStarted public.

(calling it in nsRefreshDriver ctor would be scary, since refcnt is still 0 at that point)

Ah, true that's fair.

This then. Needs a new tryserver push since behavior is rather different.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=316ee76ed0735212eb07603ec81ea6a46c967a4d

Attachment #9041025 - Flags: review?(emilio)
Comment on attachment 9041025 [details] [diff] [review]
early_rf_2.diff

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

Thanks, looks good if green :)
Attachment #9041025 - Flags: review?(emilio) → review+
Comment on attachment 9041022 [details] [diff] [review]
early_rf.diff

If for some reason the second patch doesn't work we can revisit this.
Attachment #9041022 - Flags: review?(emilio)
Attachment #9041022 - Attachment is obsolete: true
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc6d44d38030
Start RefreshDriver timer very early, r=emilio
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.