[webkit-reviews] review granted: [Bug 101640] Have NetworkProcess do the actual loading of subresources : [Attachment 173114] Patch v1 - Do basic subresource loading

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 101640: Have NetworkProcess do the actual loading of subresources
https://bugs.webkit.org/show_bug.cgi?id=101640

Attachment 173114: Patch v1 - Do basic subresource loading
https://bugs.webkit.org/attachment.cgi?id=173114&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173114&action=review


> Source/WebKit2/ChangeLog:8
> +	   Additional information of the change such as approach, rationale.
Please add per-function descriptions below (OOPS!).

OOPS.

> Source/WebKit2/ChangeLog:65
> +	   Add an implementation of WebCore::ResourceBuffer that wraps a
ShareableResource instead of a SharedBuffer:

Hmm, it's very much not cool that we still have an m_sharedBuffer data member.

> Source/WebKit2/Shared/ShareableResource.cpp:34
> +ShareableResource::Handle::Handle()
> +{}

Braces go on separate lines.

> Source/WebKit2/Shared/ShareableResource.cpp:61
> +    // Create the shared memory.

Does this comment help? I'm not sure if it's accurate.

> Source/WebKit2/Shared/ShareableResource.cpp:75
> +    ASSERT(m_sharedMemory);
> +    ASSERT(m_offset + m_size <= m_sharedMemory->size());

Should these checks be made in production builds, too?