[webkit-reviews] review granted: [Bug 223701] Port FontDescriptionKey::computeHash() from legacy IntegerHasher to Hasher : [Attachment 424165] 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 223701: Port FontDescriptionKey::computeHash() from legacy IntegerHasher to
Hasher
https://bugs.webkit.org/show_bug.cgi?id=223701

Attachment 424165: Patch

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




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

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

Some nice features of Hasher that this does not take advantage of, that I would
like you to be aware of for the future:

1) Can pass an entire collection, like a std::array, and Hasher knows how to
hash all the items. Works for any object that has a begin function and that
works with a range-based for loop.

2) Can list more than one item in a single call to add, rather than using
separate calls to add each thing.

3) Can handle Optional, so no need to write valueOr.

4) Can pass a tuple and it will hash all the items in the tuple.

5) Can hash whole objects, not just integers, so can avoid doing hashes of
hashes in most cases.

6) Can write add function overloads for use in combination with the above.

> Source/WebCore/platform/graphics/FontCache.h:115
> +	   add(hasher, m_fontSelectionRequest.weight);
> +	   add(hasher, m_fontSelectionRequest.width);
> +	   add(hasher,
m_fontSelectionRequest.slope.valueOr(normalItalicValue()));

Could just write this:

    add(hasher, m_fontSelectionRequest.tied());

> Source/WebCore/platform/graphics/FontCache.h:116
> +	   add(hasher, m_locale.existingHash());

Could just write this:

    add(hasher, m_locale);

> Source/WebCore/platform/graphics/FontCache.h:117
>	   for (unsigned flagItem : m_flags)

Could just write this:

    add(hasher, m_flags);

Instead of the loop.

> Source/WebCore/platform/graphics/FontCache.h:120
> +	   add(hasher, m_featureSettings.hash());
> +	   add(hasher, m_variationSettings.hash());

With very little work we could refactor so this doesn’t make a hash out of
hashes. Likely just need to make an add(Hasher&, x) overload for
FontTaggedSetting.

No need to do that now.