[webkit-reviews] review granted: [Bug 70713] [chromium] Implement frame rate control portions of CCScheduler : [Attachment 112621] Cleanup

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Nat Duca
<nduca at chromium.org>'s request for review:
Bug 70713: [chromium] Implement frame rate control portions of CCScheduler
https://bugs.webkit.org/show_bug.cgi?id=70713

Attachment 112621: Cleanup
https://bugs.webkit.org/attachment.cgi?id=112621&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112621&action=review


R still =me. I like the explanation a lot.

> Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:84
> +// We treat delays in tasks differently depending on the amount of delay we
encouter. Suppose we

typo 'encouter'->'encounter'. 'encouter' sounds like a completely different
sort of thing entirely

> Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.h:40
> +// NOTE: Deleting CCDelayBasedTimeSource when a tick is pending will lead to
a crash.
> +//	   To fix this, we need to generalize CCScopedMainThreadProxy.

instead of just saying that this crashes, i think you can fix this by making
this class refcounted and calling ref() in postTickTask() and deref() before
returning from onTick(). it's not much code and one less headache to deal with
later