[webkit-reviews] review denied: [Bug 72106] Consolidate the logic that creates run loop observers for flushing layer tree changes to CoreAnimation : [Attachment 114661] Patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Andy Estes <aestes at apple.com>'s
request for review:
Bug 72106: Consolidate the logic that creates run loop observers for flushing
layer tree changes to CoreAnimation
https://bugs.webkit.org/show_bug.cgi?id=72106

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

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=114661&action=review


> Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h:42
> +    typedef void (*CallbackFunction)(void* context);
> +
> +    static PassOwnPtr<LayerFlushScheduler> create(CallbackFunction callback,
void* context)

It would be nicer if this took a Client class instead of a callback and a
context. You'd have to add an extra class to WebKit1, but maybe that's fine.

> Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h:52
> +    void invalidate();
> +    void invalidateIfScheduled();

I don't think we need both invalidate and invalidateIfScheduled. Let's just
have invalidate() become invalidateIfScheduled().

> Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h:53
> +    void setEnabled(bool);

Instead of having a setEnabled function, it would be nicer to have
suspend/resume calls that would increment/decrement a counter.