[webkit-reviews] review denied: [Bug 52934] Use WTF::StringHasher in WebCore : [Attachment 79823] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 52934: Use WTF::StringHasher in WebCore
https://bugs.webkit.org/show_bug.cgi?id=52934

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=79823&action=review

review- because of the WTF prefix issue. Other than this and the function
naming that we decided to handle outside the scope of this patch, this seems
fine.

> Source/JavaScriptCore/wtf/StringHasher.h:146
> +	   ASSERT(!(size % 4));

Where does the number 4 come from here? The hasher can handle any object with a
size that’s multiple of 2. It is true that 4 is the size of int and the size of
pointers on 32-bit systems, but it seems a bit arbitrary to specifically
support only ranges of memory that are multiples of 4.

> Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp:92
> -    return
AlreadyHashed::avoidDeletedValue(StringImpl::computeHash(url.string().character
s() + hostStart, hostEnd - hostStart));
> +    return
AlreadyHashed::avoidDeletedValue(WTF::StringHasher::createHash(url.string().cha
racters() + hostStart, hostEnd - hostStart));

There should be no need to use the namespace prefix WTF. The WTF header files
make use of using declarations, so files that use things defined in the WTF
namespace should almost never need to explicitly include the WTF:: prefix.