Ref BlobResourceHandle before calling doNotifyFinish() on main thread

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Nate Chapin]

In another patch, I changed WebCore timings slightly and found that we don't seem to be guaranteeing that a BlobResourceHandle will survive until doNotfyFinish() gets called on the appropriate thread.

I haven't found a way to trigger this without modifying the loader rather substantially, but I do have a fix for the issue I saw.

[Nate Chapin]

Created attachment 95217 [details]
patch #2

[Jian Li]

Comment on attachment 95217 [details]
patch #2

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.

[Nate Chapin]

Created attachment 95220 [details]
Addressing comments

[Jian Li]

Comment on attachment 95220 [details]
Addressing comments

View in context: https://bugs.webkit.org/attachment.cgi?id=95220&action=review

> Source/WebCore/ChangeLog:7
> +        I triggered this crash in fast/files/file-reader-abort.html during a

Please move this line and the line below to place after the bug link.

[Nate Chapin]

Created attachment 95221 [details]
Moved test line in ChangeLog

[WebKit Commit Bot]

Comment on attachment 95221 [details]
Moved test line in ChangeLog

Clearing flags on attachment: 95221

Committed r87595: <http://trac.webkit.org/changeset/87595>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.