[webkit-reviews] review granted: [Bug 184518] [Web Animations] Enable seeking for hardware animations : [Attachment 337777] 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 184518: [Web Animations] Enable seeking for hardware animations
https://bugs.webkit.org/show_bug.cgi?id=184518

Attachment 337777: Patch

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




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

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

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2806
> +	       const String& currAnimationName = it->key;
> +	       AnimationsMap::iterator animationIt =
m_runningAnimations.find(currAnimationName);

Nit: Just say currentAnimationName and animationIterator

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2813
> +		   for (size_t i = 0; i < animations.size(); ++i) {
> +		       const LayerPropertyAnimation& currAnimation =
animations[i];

Can't this be a for (const auto& currentAnimation : animations) loop?

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2950
> +    RefPtr<PlatformCAAnimation> curAnim =
layer->animationForKey(animationID);

Again, just say currentAnimation. Or at least be consistent - above is
currAnimation, this is curAnim.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2955
> +    RefPtr<PlatformCAAnimation> newAnim = curAnim->copy();

Ditto.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:531
> +	   Seconds timeOffset; // only used for pause

Nit: // Only used for pause. (uppercase O and period)