[webkit-reviews] review denied: [Bug 227502] [PlayStation] Don't assume 4KB block size when estimating Network Cache disk usage : [Attachment 432712] Fixing review comments

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied Christopher Reid
<chris.reid at sony.com>'s request for review:
Bug 227502: [PlayStation] Don't assume 4KB block size when estimating Network
Cache disk usage
https://bugs.webkit.org/show_bug.cgi?id=227502

Attachment 432712: Fixing review comments

https://bugs.webkit.org/attachment.cgi?id=432712&action=review




--- Comment #10 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 432712
  --> https://bugs.webkit.org/attachment.cgi?id=432712
Fixing review comments

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

r=me with comments.

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:187
> +#if !PLATFORM(PLAYSTATION)

Why is this needed? I would think that either:
1. The Playstation port doesn't build this file, in which case we don't need
this #if !PLATFORM(PLAYSTATION)
or
2. The Playstation port builds this file, in which case we don't this this #if
or the duplicate implementation in FileSystemPlayStation.cpp.

> Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp:270
> +    ,
m_volumeBlockSize(FileSystem::getVolumeFileBlockSize(baseDirectoryPath).value_o
r(4 * KB))

r- for this. This does a filesystem operation on the *main* thread, which is
never OK. You need to call this on the background thread.

> Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h:165
> +    size_t estimateRecordsSize(unsigned, unsigned);

Please don't omit the parameter names as they are not obvious.

Also, this function can probably be const.