[webkit-reviews] review granted: [Bug 212921] [IPC] Adopt enum class for IPC::CFType : [Attachment 401383] Patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 212921: [IPC] Adopt enum class for IPC::CFType
https://bugs.webkit.org/show_bug.cgi?id=212921

Attachment 401383: Patch v2

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




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

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

> Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:55
>      CFArray,

I understand your rationale for wanting to leave the CF prefix on these, but I
don’t agree that it’s better to have that mechanical rule and I would remove it
if it was me doing the work.

> Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:181
> +    case CFType::Unknown:

Seems like someone should later refactor to eliminate this value. Using
Optional, most likely. It’s funny to validate that these are good values, but
include one bad value.

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:210
> +    RetainPtr<CFDictionaryRef> dictionary =
createSerializableRepresentation(requestToSerialize.get(),
IPC::tokenNullptrTypeRef());

I’d use auto here.

> Source/WebKit/Shared/mac/WebCoreArgumentCodersMac.mm:234
> +    RetainPtr<NSURLRequest> nsURLRequest =
createNSURLRequestFromSerializableRepresentation(dictionary.get(),
IPC::tokenNullptrTypeRef());

And here.

> Source/WebKit/mac/WebKit2.order:919
> -__ZN7CoreIPC16tokenNullTypeRefEv
> +__ZN7CoreIPC19tokenNullptrTypeRefEv

Normally we don’t try to keep these up to date. Not even sure why we still have
them checked in.