[Webkit-unassigned] [Bug 144380] [SOUP] Network Cache: Implement ShareableResource for Soup and enable it for GTK platform

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144380

--- Comment #2 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 251937
  --> https://bugs.webkit.org/attachment.cgi?id=251937
Patch

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

> Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.cpp:130
> -    auto mappedData = Data::adoptMap(map, size);
> +    auto mappedData = Data::adoptMap(map, size, fd);

It would be good to encapsulate the whole "create disk-backed map from existing Data" operation and make it a Data function.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheData.h:42
>  namespace WebKit {
> +class SharedMemory;

empty line after namespace

> Source/WebKit2/NetworkProcess/cache/NetworkCacheDataCocoa.mm:112
>  Data mapFile(int fd, size_t offset, size_t size)
>  {
> -    if (!size)
> +    if (!size) {
> +        close(fd);
>          return Data::empty();
> +    }

It is surprising this function now closes the file. At minimum it should be renamed to something like adoptAndMapFile. There are a bunch of clients that call this and close the file manually.

We really need better abstactions though.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheDataCocoa.mm:157
> +RefPtr<SharedMemory> Data::createSharedMemory() const

tryCreateSharedMemory?

> Source/WebKit2/Shared/soup/ShareableResourceSoup.cpp:43
> +PassRefPtr<SharedBuffer> ShareableResource::Handle::tryWrapInSharedBuffer() const

Adding this Soup-specific stuff is going to make it harder to get rid of these ugly abstractions. :(

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150504/1b0fc339/attachment.html>