[webkit-reviews] review denied: [Bug 99083] [chromium] Pass canPaintLCDText to WebContentLayerClient::paintContents : [Attachment 171310] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Alok Priyadarshi
<alokp at chromium.org>'s request for review:
Bug 99083: [chromium] Pass canPaintLCDText to
WebContentLayerClient::paintContents
https://bugs.webkit.org/show_bug.cgi?id=99083

Attachment 171310: Patch
https://bugs.webkit.org/attachment.cgi?id=171310&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171310&action=review


> Source/Platform/chromium/public/WebContentLayerClient.h:51
> +    // FIXME: Remove this version of paintContents after chromium starts
using the
> +    // new version with canPaintLCDText.
> +    virtual void paintContents(WebCanvas* canvas, const WebRect& clip,
WebFloatRect& opaque)
> +    {
> +	   paintContents(canvas, clip, false, opaque);
> +    }

Now this is just weird - you're using implementation inheritance.  I really
think you should add the new parameter with a guard and do the rolls that way
instead of trying to rely on the default implementation.  Any code that calls
the 4-arg version of paintContents() will silently break chromium code that
implements only the 3-arg version today.