Bug 80123 – [BlackBerry] Upstream LayerAnimation.{cpp, h}

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Robin Cao]

LayerAnimation is a helper class used in accelerated animations.

[Robin Cao]

Created attachment 133971 [details]
patch

[Rob Buis]

Comment on attachment 133971 [details]
patch

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

Looks good, but can still be improved a bit.

> Source/WebCore/platform/graphics/blackberry/LayerAnimation.cpp:112
> +                                        ctf->y2(),

These params can be placed in the same line.

> Source/WebCore/platform/graphics/blackberry/LayerAnimation.cpp:221
> +        // A trick to avoid touching the refcount of shared TransformOperations on the wrong thread

Lacks period at end of sentence.

> Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:24
> +#include "GraphicsLayer.h"

Is this one needed?

> Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:50
> +

Remove empty line.

> Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:58
> +        // This returns a copy of the animation.

This comment does not add much info.

[Robin Cao]

(In reply to comment #2)
> (From update of attachment 133971 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133971&action=review
> 
> Looks good, but can still be improved a bit.
> 
> > Source/WebCore/platform/graphics/blackberry/LayerAnimation.cpp:112
> > +                                        ctf->y2(),
> 
> These params can be placed in the same line.
> 
Agree, looks better in the same line.

> > Source/WebCore/platform/graphics/blackberry/LayerAnimation.cpp:221
> > +        // A trick to avoid touching the refcount of shared TransformOperations on the wrong thread
> 
> Lacks period at end of sentence.
> 
Added.

> > Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:24
> > +#include "GraphicsLayer.h"
> 
> Is this one needed?
> 
This is needed because LayerAnimation has a member variable: KeyframeValueList m_values;
And KeyframeValueList is defined in "GraphicsLayer.h".

> > Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:50
> > +
> 
> Remove empty line.
> 
Fixed.

> > Source/WebCore/platform/graphics/blackberry/LayerAnimation.h:58
> > +        // This returns a copy of the animation.
> 
> This comment does not add much info.
Removed.

Thanks for the review.

[Robin Cao]

Created attachment 134203 [details]
updated-patch

[Rob Buis]

Comment on attachment 134203 [details]
updated-patch

LGTM.

[WebKit Review Bot]

Comment on attachment 134203 [details]
updated-patch

Clearing flags on attachment: 134203

Committed r112358: <http://trac.webkit.org/changeset/112358>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.