Closed Bug 538087 Opened 10 years ago Closed 10 years ago

Hook up reflows to refresh driver

Categories

(Core :: Layout, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Patch coming up.  I left the existing 30ms timer for dealing with interrupts, because if I take it out I end up with event starvation again on Mac (as expected, since the threshold there was 23ms or so).  But everything that the reflow event used to do is now done via refresh driver.
Attached patch FixSplinter Review
Attachment #420261 - Flags: review?(dbaron)
Blocks: 543458
Attachment #420261 - Flags: review?(dbaron) → review+
Comment on attachment 420261 [details] [diff] [review]
Fix

+  // At least on On Win32 and Mac after interupting a reflow we need to post

s/on On/on/



It might be worth adding an (#ifdef DEBUG) IsRefreshObserver method on
the refresh driver so you can add assertions in various places that
mReflowScheduled == RefreshDriver()->IsRefreshObserver(this, Flush_Layout) .

ScheduleReflow should probably also assert that mReflowScheduled is false.


r=dbaron
> s/on On/on/

Done.

> ScheduleReflow should probably also assert that mReflowScheduled is false.

Done.

> so you can add assertions in various places

Did you have particular places in mind?
I added preconditions and postconditions in MaybeScheduleReflow, ScheduleReflow, CancelAllPendingReflows, WillRefresh, ScheduleReflowOffTimer.
Pushed http://hg.mozilla.org/mozilla-central/rev/69413b4da4b6 and other than exacerbating some existing random-oranges (now fixed), things look good.  Not much Tp impact, though.  Or Tdhtml.  I'd been somewhat hoping for more.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 639377
Depends on: 848656
You need to log in before you can comment on or make changes to this bug.