[webkit-reviews] review granted: [Bug 122942] PingLoader objects unnecessarily pass through OwnPtr : [Attachment 214425] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 122942: PingLoader objects unnecessarily pass through OwnPtr
https://bugs.webkit.org/show_bug.cgi?id=122942

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

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


OK as is. Room for improvement.

> Source/WebCore/loader/PingLoader.cpp:71
> +    PingLoader* pingLoader = new PingLoader(frame, request);
> +    UNUSED_PARAM(pingLoader);

I don’t think we need the variable at all. It’s also strange to use
UNUSED_PARAM for this. The word is “delete”, not “kill”.

Also, I think we should have a helper member function to hide the use of "new".
I don’t like having these calls to new with the comments in three different
places in the file. It should just be one small inline function.

> Source/WebCore/loader/PingLoader.cpp:102
> +    // No need to free the PingLoader object or manage it via a smart
pointer - it will kill itself as soon as it receives a response.
> +    PingLoader* pingLoader = new PingLoader(frame, request);
> +    UNUSED_PARAM(pingLoader);

Ditto.

> Source/WebCore/loader/PingLoader.cpp:122
> +    // No need to free the PingLoader object or manage it via a smart
pointer - it will kill itself as soon as it receives a response.
> +    PingLoader* pingLoader = new PingLoader(frame, request);
> +    UNUSED_PARAM(pingLoader);

Ditto.