[webkit-reviews] review granted: [Bug 211527] Use CocoaColor in more places instead of platform defines : [Attachment 398665] Fix bug title

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 211527: Use CocoaColor in more places instead of platform defines
https://bugs.webkit.org/show_bug.cgi?id=211527

Attachment 398665: Fix bug title

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 398665
  --> https://bugs.webkit.org/attachment.cgi?id=398665
Fix bug title

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

Nice clean up. This is the kind of abstraction I really like.

Not sure each of these (CocoaColor, CocoaFont/Descriptor, CocoaImage) should
have its own header. Seems like they could share.

> Source/WebKit/Platform/cocoa/CocoaFont.h:29
> + at class NSFont, NSFontDescriptor;

Didn’t even know you could use commas in these! Not sure I would have.

> Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm:103
> +    if ([object isKindOfClass:[CocoaColor class]])

I’m tempted in new code to write ".class" instead.

> Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm:434
> +	   encodeColorInternal(encoder, static_cast<CocoaColor *>(object));

Seems like for the future we need a Objective-C pointer cast that, at least in
debug builds, checks the class.

> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.h:48
>  using ViewType = NSView;
>  using RectType = NSRect;

Other candidates.

> Source/WebKit/UIProcess/Cocoa/WKSafeBrowsingWarning.mm:57
>  using TextViewType = NSTextView;
>  using ButtonType = NSButton;
>  using AlignmentType = NSLayoutAttribute;
>  using SizeType = NSSize;

Other candidates.