[webkit-reviews] review granted: [Bug 92997] [Chromium] Add a stub for WebView::getTouchHighlightQuads() : [Attachment 156104] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Peter Beverloo
<peter at chromium.org>'s request for review:
Bug 92997: [Chromium] Add a stub for WebView::getTouchHighlightQuads()
https://bugs.webkit.org/show_bug.cgi?id=92997

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=156104&action=review


> Source/Platform/chromium/public/WebTouchCandidatesInfo.h:48
> +    WebTouchCandidatesInfo()
> +	   : numberOfCandidates(0),
> +	   : unitedBounds(),
> +	   : smallestDimension(0)

This isn't quite right.  Try:

WebTouchCandidatesInfo()
    : numberOfCandidates(0)
    , smallestDimension(0)

> Source/WebKit/chromium/public/WebView.h:63
> +#if defined(OS_ANDROID)

These ifdefs aren't needed.

> Source/WebKit/chromium/public/WebView.h:474
> +    // Touch
----------------------------------------------------------------

Two blank lines above section headings.

> Source/WebKit/chromium/public/WebView.h:476
> +#if defined(OS_ANDROID)

This #ifdef isn't needed.

> Source/WebKit/chromium/public/WebView.h:483
> +    virtual WebVector<WebFloatQuad> getTouchHighlightQuads(const WebPoint&,
> +							      int padding,

Should we use a WebRect instead?

> Source/WebKit/chromium/public/WebView.h:485
> +							     
WebTouchCandidatesInfo* outTouchInfo,
> +							      WebColor&
outTapHighlightColor) = 0;

WebTouchCandidatesInfo* -> WebTouchCandidatesInfo& (we use non-const reference
types for out parameters.