[webkit-reviews] review granted: [Bug 108851] FileSystem mods: Changes to snapshot file creation to reduce dependencies on blobs. : [Attachment 189344] patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Michael Nordman
<michaeln at google.com>'s request for review:
Bug 108851: FileSystem mods: Changes to snapshot file creation to reduce
dependencies on blobs.
https://bugs.webkit.org/show_bug.cgi?id=108851

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

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


> Source/WebCore/ChangeLog:10
> +	   No new tests (OOPS!).

This line will stop your patch from being landed.  You should either add a test
of an explanation of why this change doesn't need a test.

> Source/WebCore/platform/network/BlobData.cpp:117
> +BlobDataHandle::~BlobDataHandle()
> +{
> +    ThreadableBlobRegistry::unregisterBlobURL(m_internalURL);
> +}

I take is that ThreadableBlobRegistry is safe to call on any thread.

> Source/WebCore/platform/network/BlobData.h:218
> +    KURL m_internalURL;

This object is ThreadSafeRefCounted but presumably KURL itself isn't actually
thread safe.  What's the thread-safety story here?