[Webkit-unassigned] [Bug 143652] Network Cache: Deduplicate body data

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #250632|review?                     |review+
              Flags|                            |

--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 250632
  --> https://bugs.webkit.org/attachment.cgi?id=250632
patch

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

> Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.cpp:155
> +    return stat.st_nlink - 1;

Any danger of using more links to a single file than the file system supports?

> Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.h:35
> +#include <wtf/text/WTFString.h>

Don’t need this include.

> Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.h:50
> +    // These are all synchronous and should not be used from the main thread.

This comment applies to the constructor of the class too!

> Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.h:66
> +    const CString m_blobDirectoryPath;

Don’t we have thread safety issues with the reference count of this data member? I don’t understand the threading restrictions on this class. Maybe the only function that is safe to call on another thread is approximateSize()?

> Source/WebKit2/NetworkProcess/cache/NetworkCacheData.h:135
> +Data mapFile(const CString& path);

Unusual style to take a const CString&; wouldn’t we normally take const char* instead?

> Source/WebKit2/NetworkProcess/cache/NetworkCacheDataCocoa.mm:148
> +    if (a.isNull() || b.isNull())
> +        return false;

So null != null? Is that really what we want?

-- 
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/20150413/d46db749/attachment.html>