[webkit-reviews] review granted: [Bug 48781] Add a resourceload delegate method to query if WebCore should paint the default broken image for failed images. : [Attachment 77846] New patch addressing comments.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yongjun Zhang
<yongjun_zhang at apple.com>'s request for review:
Bug 48781: Add a resourceload delegate method to query if WebCore should paint
the default broken image for failed images.
https://bugs.webkit.org/show_bug.cgi?id=48781

Attachment 77846: New patch addressing comments.
https://bugs.webkit.org/attachment.cgi?id=77846&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=77846&action=review

> Tools/DumpRenderTree/mac/ResourceLoadDelegate.mm:245
> +- (BOOL)webView: (WebView*)wv shouldPaintBrokenImage:(NSURL*)imageURL

Extras space here. Also, I would use a longer variable name rather than "wv".

> WebKit/mac/WebView/WebResourceLoadDelegatePrivate.h:63
> +/*!
> + @method webView:shouldPaintBrokenImage:(NSURL*)imageURL
> + @abstract This message is sent when an image cannot be decoded or
displayed.
> + @param imageURL The url of the offending image.
> + @result return YES if WebKit should paint the default broken image.
> + */
> +- (BOOL)webView:(WebView*)sender shouldPaintBrokenImage:(NSURL*)imageURL;
>  @end

I would think this should be named shouldPaintBrokenImageForURL: given the
usual Objective-C naming scheme. In non-ObjC code, there is no need to include
the argument name in the function name, but in ObjC method names we normally
do.