[webkit-reviews] review denied: [Bug 62530] [Chromium] Remove dependencies on harfbuzz from FontPlatformDataLinux and FontLinux : [Attachment 98696] Patch V1

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Kenichi Ishibashi
<bashi at chromium.org>'s request for review:
Bug 62530: [Chromium] Remove dependencies on harfbuzz from
FontPlatformDataLinux and FontLinux
https://bugs.webkit.org/show_bug.cgi?id=62530

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98696&action=review

In general, looks fine.  Just some style nits.

> Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h:114
> +    const int offsetForPosition(int);
> +    const FloatRect selectionRect(const FloatPoint&, int, int, int);

I think morrita meant to put const at the end of the function (e.g., int
offsetForPosition(int) const).

Can you name the parameters in selectionRect()?  It's confusing when there are
multiple ints passed in.

> Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h:118
> +    const unsigned width() const { return m_pixelWidth; }

Remove the first const.

> Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h:133
> +    const int glyphIndexForXPositionInScriptRun(int targetX);

const at the end.

> Source/WebCore/platform/graphics/chromium/HarfbuzzSkia.h:56
> +    HarfbuzzFace(FontPlatformData*);

explicit