[webkit-reviews] review granted: [Bug 198010] [Mac] Add API test to make sure the system font is drawn just like in native code : [Attachment 370174] Patch

Fri May 17 20:33:26 PDT 2019

Daniel Bates <dbates at webkit.org> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 198010: [Mac] Add API test to make sure the system font is drawn just like
in native code
https://bugs.webkit.org/show_bug.cgi?id=198010

Attachment 370174: Patch

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

--- Comment #2 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 370174
  --> https://bugs.webkit.org/attachment.cgi?id=370174
Patch

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.html:2
> +<html lang="en-US">

Ok as is. Lang attribute doesn't seem required.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:35
> +    RetainPtr<WKWebViewConfiguration> configuration =
adoptNS([[WKWebViewConfiguration alloc] init]);

Auto?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:36
> +    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc]
initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);

Auto?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextWidth.mm:50
> +    NSAttributedString *attributedString = [[NSAttributedString alloc]
initWithString:@"This is a test string" attributes:@{NSFontAttributeName :
static_cast<NSFont *>(font.get())}];

Looks like a leak. If not, better idiom in my opinion is use retainPtr and call
.leak() at the call site <---- makes grepping easy . Grep code for examples.