[webkit-reviews] review denied: [Bug 61669] Ref BlobResourceHandle before calling doNotifyFinish() on main thread : [Attachment 95217] patch #2

[bugzilla-daemon at webkit.org]

Jian Li <jianli at chromium.org> has denied Nate Chapin <japhet at chromium.org>'s
request for review:
Bug 61669: Ref BlobResourceHandle before calling doNotifyFinish() on main
thread
https://bugs.webkit.org/show_bug.cgi?id=61669

Attachment 95217: patch #2
https://bugs.webkit.org/attachment.cgi?id=95217&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95217&action=review

> Source/WebCore/ChangeLog:5
> +	   ref() BlobResourceHandle before calling doNotifyFinish()

The title is somewhat confusing to me. How about: Keep a reference to
BlobResourceHandle before calling doNotifyFinish asynchronously to ensure it's
still safe in the main thread.

Which test covers this? Please point it out here.

> Source/WebCore/platform/network/BlobResourceHandle.cpp:599
> +    if (!handle->aborted())

We should say:
    if (!handle->aborted() && handle->client())

> Source/WebCore/platform/network/BlobResourceHandle.cpp:601
> +    handle->deref();

Please add a comment for deref, like: Unbalance the deref in notifyFinish.

> Source/WebCore/platform/network/BlobResourceHandle.cpp:607
> +    ref();

Please add an empty line here.