Bug 223148 – Improve coding style of CSSPropertyAnimation.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Antoine Quint]

Darin provided review feedback in bug 223124 which would really apply to most of CSSPropertyAnimation.cpp. This bug is to ensure that we:

- use final instead of override where applicable
- put override methods in private sections
- don't abbreviate parameters

There may be more.

[Antoine Quint]

Created attachment 423102 [details]
Patch

[Antoine Quint]

The patch is failing to apply due to the GitHub mirror being way behind :(

[Aakash Jain]

(In reply to Antoine Quint from comment #2)
> The patch is failing to apply due to the GitHub mirror being way behind :(
That has been fixed now. Your patch is being processed by EWS now.

[Antoine Quint]

Created attachment 423108 [details]
Patch

[Darin Adler]

Comment on attachment 423108 [details]
Patch

A lot of this code also uses pointers that are required to be non-null rather than references. Like the RenderStyle and the clients.

[Sam Weinig]

Comment on attachment 423108 [details]
Patch

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

> Source/WebCore/animation/CSSPropertyAnimation.cpp:2249
>  // Returns true if we need to start animation timers
> -void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* anim, CSSPropertyID prop, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress)
> +void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* client, CSSPropertyID property, RenderStyle* destination, const RenderStyle* from, const RenderStyle* to, double progress)

Can this comment be removed? The function returns void.

[Antoine Quint]

Committed r274409 (235276@main): <https://commits.webkit.org/235276@main>

[Radar WebKit Bug Importer]

<rdar://problem/75420721>

[Antoine Quint]

(In reply to Sam Weinig from comment #6)
> Comment on attachment 423108 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423108&action=review
> 
> > Source/WebCore/animation/CSSPropertyAnimation.cpp:2249
> >  // Returns true if we need to start animation timers
> > -void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* anim, CSSPropertyID prop, RenderStyle* dst, const RenderStyle* a, const RenderStyle* b, double progress)
> > +void CSSPropertyAnimation::blendProperties(const CSSPropertyBlendingClient* client, CSSPropertyID property, RenderStyle* destination, const RenderStyle* from, const RenderStyle* to, double progress)
> 
> Can this comment be removed? The function returns void.

Fixed in commit.

[Antoine Quint]

(In reply to Darin Adler from comment #5)
> Comment on attachment 423108 [details]
> Patch
> 
> A lot of this code also uses pointers that are required to be non-null
> rather than references. Like the RenderStyle and the clients.

I'll look into that in bug 223173.