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.
Created attachment 423102 [details] Patch
The patch is failing to apply due to the GitHub mirror being way behind :(
(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.
Created attachment 423108 [details] Patch
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.
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.
Committed r274409 (235276@main): <https://commits.webkit.org/235276@main>
<rdar://problem/75420721>
(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.
(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.