[webkit-reviews] review granted: [Bug 235958] Add support for sharing Shared Workers (including across WebProcesses) : [Attachment 450954] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 235958: Add support for sharing Shared Workers (including across
WebProcesses)
https://bugs.webkit.org/show_bug.cgi?id=235958

Attachment 450954: Patch

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




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

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

Looks good, I didn’t spot any mistakes.

> Source/WebCore/workers/shared/SharedWorkerKey.h:52
> +    Hasher hasher;
> +    add(hasher, origin.hash());
> +    add(hasher, url);
> +    add(hasher, name);
> +    return hasher.hash();

Instead, we should write:

    return computeHash(origin.hash(), url, name);

That does the same thing. But also, it’s not good to hash a hash. Instead, we
should write these functions:

    inline add(Hasher& hasher, const SharedWorkerKey& key)
    {
	hasher.add(key.origin, key.url, key.name);
    }
    inline add(Hasher& hasher, const ClientOrigin& origin)
    {
	add(hasher, origin.topOrigin, origin.clientOrigin);
    }
    inline add(Hasher& hasher, const SecurityOriginData& data)
    {
	add(hasher, data.protocol, data.host, data.port);
    }

These don’t have to be inlined, and of course they should go in the appropriate
headers. Once those functions are all defined, we can call computeHash on any
of these types, without any hashing of a hash getting involved. If we want, we
can still define SharedWorkerKey::hash, but it should just call computeHash:

    unsigned hash() const { return computeHash(*this); }