[webkit-reviews] review granted: [Bug 224512] Use range-for loop in target contrast selection to simplify/improve code readability : [Attachment 425920] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 224512: Use range-for loop in target contrast selection to simplify/improve
code readability
https://bugs.webkit.org/show_bug.cgi?id=224512

Attachment 425920: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 425920
  --> https://bugs.webkit.org/attachment.cgi?id=425920
Patch

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

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1512
> +static Color selectFirstColorThatMeetsOrExceedsTargetContrast(const Color&
originBackgroundColor, Vector<Color>&& colorsToCompareAgainst, double
targetContrast)

Annoying that this is double and the return value of WebCore::contrastRatio is
float.

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1519
>	       return color;

Could use WTFMove(color) here since the vector is an rvalue reference. I
presume that is why we wanted to pass ownership?

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1533
> +    auto* colorWithHigestContrastRatio = &colorsToCompareAgainst[0];

Just noticed "higest".

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1543
> +    return *colorWithHigestContrastRatio;

Could use WTFMove(*colorWithHighestContrastRatio) here since the vector is an
rvalue reference. I presume that is why we wanted to pass ownership?