[webkit-reviews] review denied: [Bug 223204] Refactor Pair.h to CSSValuePair.h : [Attachment 424205] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Mar 27 03:30:27 PDT 2021


Antti Koivisto <koivisto at iki.fi> has denied Tyler Wilcock
<twilco.o at protonmail.com>'s request for review:
Bug 223204: Refactor Pair.h to CSSValuePair.h
https://bugs.webkit.org/show_bug.cgi?id=223204

Attachment 424205: Patch

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




--- Comment #7 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 424205
  --> https://bugs.webkit.org/attachment.cgi?id=424205
Patch

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

I'm going to say r- until it is better understood what the benefits are.

> Source/WebCore/css/CSSBasicShapes.h:141
> -    CSSPrimitiveValue* centerX() const { return m_centerX.get(); }
> -    CSSPrimitiveValue* centerY() const { return m_centerY.get(); }
> +    CSSValue* centerX() const { return m_centerX.get(); }
> +    CSSValue* centerY() const { return m_centerY.get(); }
>      CSSPrimitiveValue* radius() const { return m_radius.get(); }
>  
> -    void setCenterX(Ref<CSSPrimitiveValue>&& centerX) { m_centerX =
WTFMove(centerX); }
> -    void setCenterY(Ref<CSSPrimitiveValue>&& centerY) { m_centerY =
WTFMove(centerY); }
> +    void setCenterX(Ref<CSSValue>&& centerX) { m_centerX = WTFMove(centerX);
}
> +    void setCenterY(Ref<CSSValue>&& centerY) { m_centerY = WTFMove(centerY);
}

The practical result of of this refactoring appears to be to weaken typing in
many places. This is not a desirable direction. 

Can you explain what concrete benefits there are that would outweigh this
regression?


More information about the webkit-reviews mailing list