[webkit-reviews] review granted: [Bug 207618] Shrink CachedResource : [Attachment 390500] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 207618: Shrink CachedResource
https://bugs.webkit.org/show_bug.cgi?id=207618

Attachment 390500: Patch

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




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 390500
  --> https://bugs.webkit.org/attachment.cgi?id=390500
Patch

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

> Source/WebCore/ChangeLog:9
> +	   For each enum class, we define `bitsOfXXX` value, which indicates #
of bits to represent it. And using

Let's replace bitsOfXXX with XXXBitWidth to be consistent with
https://bugs.webkit.org/show_bug.cgi?id=207616.

> Source/WTF/wtf/Markable.h:150
> +    Optional<T> asOptional() const
> +    {
> +	   if (bool(*this))
> +	       return m_value;
> +	   return WTF::nullopt;
> +    }

This is identical to the cast operator above.  Can you implement this in terms
of the cast operator or better yet, implement the cast operator in term of
asOptional()?

> Source/WebCore/loader/ResourceLoaderOptions.h:49
> +static constexpr unsigned bitsOfSendCallbackPolicy = 1;

Let's call this sendCallbackPolicyBitWidth.

> Source/WebCore/loader/ResourceLoaderOptions.h:57
> +static constexpr unsigned bitsOfContentSniffingPolicy = 1;

Ditto: contentSniffingPolicyBitWidth.

> Source/WebCore/loader/ResourceLoaderOptions.h:63
> +static constexpr unsigned bitsOfDataBufferingPolicy = 1;

Ditto here and the others below.

> Source/WebCore/loader/cache/CachedResource.h:94
> +    static constexpr unsigned bitsOfType = 5;
> +    static_assert(static_cast<unsigned>(Type::LastType) <= ((1U <<
bitsOfType) - 1));

nit: Let's call this typeBitWidth instead.

> Source/WebCore/loader/cache/CachedResource.h:104
> +    static constexpr unsigned bitsOfStatus = 3;
> +    static_assert(static_cast<unsigned>(DecodeError) <= ((1ULL <<
bitsOfStatus) - 1));

Ditto: statusBitWidth.

> Source/WebCore/loader/cache/CachedResource.h:150
> +    static constexpr unsigned bitsOfPreloadResult = 2;

Ditto: preloadResultBitWidth.

> Source/WebCore/platform/network/ResourceLoadPriority.h:40
> +static constexpr unsigned bitsOfResourceLoadPriority = 3;

Ditto: resourceLoadPriorityBitWidth.

> Source/WebCore/platform/network/ResourceResponseBase.h:51
> +    static constexpr unsigned bitsOfType = 3;

Ditto: typeBitWidth.

> Source/WebCore/platform/network/ResourceResponseBase.h:53
> +    static constexpr unsigned bitsOfTainting = 2;

Ditto: taintingBitWidth.

> Source/WebCore/platform/network/StoredCredentialsPolicy.h:35
> +static constexpr unsigned bitsOfStoredCredentialsPolicy = 2;

Ditto: storedCredentialsPolicyBitWidth.