[webkit-reviews] review granted: [Bug 182836] [Web Animations] Ensure that changing the timing model updates styles synchronously : [Attachment 333936] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 182836: [Web Animations] Ensure that changing the timing model updates
styles synchronously
https://bugs.webkit.org/show_bug.cgi?id=182836

Attachment 333936: Patch

https://bugs.webkit.org/attachment.cgi?id=333936&action=review




--- Comment #2 from Dean Jackson <dino at apple.com> ---
Comment on attachment 333936
  --> https://bugs.webkit.org/attachment.cgi?id=333936
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=333936&action=review

> Source/WebCore/animation/AnimationEffectReadOnly.cpp:47
> +    m_timing->setEffect(nullptr);

Won't m_timing go away and stop referencing the effect anyway?

> Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:114
> -    m_iterationStart = iterationStart;
> +    if (m_iterationStart != iterationStart) {
> +	   m_iterationStart = iterationStart;
> +	   propertyDidChange();
> +    }
> +

While it is more core, we usually write it as an early return:

if (m_iterationStart == iterationStart)
  return { };

m_iterationStart = iterationStart;
propertyDidChange();

return { };

> Source/WebCore/animation/AnimationEffectTimingReadOnly.cpp:127
> +	   m_iterations = iterations;

DItto