[webkit-reviews] review granted: [Bug 185917] [Web Animations] WebAnimation objects never get destroyed : [Attachment 341114] 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 185917: [Web Animations] WebAnimation objects never get destroyed
https://bugs.webkit.org/show_bug.cgi?id=185917

Attachment 341114: Patch

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




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

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

> Source/WebCore/animation/AnimationTimeline.cpp:104
> +    if (!animations.contains(&animation))
> +	   animations.append(&animation);

Maybe it should be a Set instead of a Vector in the map?

> Source/WebCore/animation/AnimationTimeline.cpp:141
> +	   }
> +    } else if (is<CSSTransition>(animation)) {
> +	   auto iterator =
m_elementToCSSTransitionByCSSPropertyID.find(&element);
> +	   if (iterator != m_elementToCSSTransitionByCSSPropertyID.end()) {
> +	       auto& cssTransitionsByProperty = iterator->value;
> +	       auto property = downcast<CSSTransition>(animation).property();
> +	       cssTransitionsByProperty.remove(property);
> +	       if (cssTransitionsByProperty.isEmpty())
> +		   m_elementToCSSTransitionByCSSPropertyID.remove(&element);
> +	   }
> +    }

You can early return from the first one to avoid the else.