[webkit-reviews] review denied: [Bug 38982] [Chromium] Support icon loading for <input type=file> : [Attachment 56453] Proposed patch (rev.2)

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 38982: [Chromium] Support icon loading for <input type=file>
https://bugs.webkit.org/show_bug.cgi?id=38982

Attachment 56453: Proposed patch (rev.2)
https://bugs.webkit.org/attachment.cgi?id=56453&action=review

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

Looks reasonable.  R- for nits.  We'll need fishd for Chromium WebKit API
review.

> WebKit/chromium/public/WebIconLoadingCompletion.h:45
> +    virtual void didLoadIcon(const WebData&) = 0;

Space after this line.

> WebKit/chromium/src/WebIconLoadingCompletionImpl.cpp:57
> +    delete this;

:(

> WebKit/chromium/src/WebIconLoadingCompletionImpl.h:37
> +// FIXME: These relative paths are a temporary hack to support using this
> +// header from webkit/glue.
> +#include "../public/WebData.h"
> +#include "../public/WebIconLoadingCompletion.h"

Can't we just do the right thing?

> WebKit/chromium/src/WebIconLoadingCompletionImpl.h:52
> +    virtual void didLoadIcon(const WebData&);

Blank line after this line.