[webkit-reviews] review granted: [Bug 42693] WebKitTestRunner needs layoutTestController.setPrinting : [Attachment 201651] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 42693: WebKitTestRunner needs layoutTestController.setPrinting
https://bugs.webkit.org/show_bug.cgi?id=42693

Attachment 201651: Patch
https://bugs.webkit.org/attachment.cgi?id=201651&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=201651&action=review


> Tools/ChangeLog:17
> +	   (TestRunner):

Bogus line added by prepare-ChangeLog.

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:879
> +	       WKRetainPtr<WKStringRef> text(AdoptWK,
WKBundlePageCopyRenderTreeExternalRepresentationForPrinting(m_page));
> +	       stringBuilder.append(toWTFString(text));

Should use adoptWK function, not the AdoptWK constructor, here. Also probably
reads better without a local variable, like this:

   
stringBuilder.append(toWTFString(adoptWK(WKBundlePageCopyRenderTreeExternalRepr
esentationForPrinting(m_page)).get()));

> Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:882
> +	       WKRetainPtr<WKStringRef> text(AdoptWK,
WKBundlePageCopyRenderTreeExternalRepresentation(m_page));
> +	       stringBuilder.append(toWTFString(text));        

Ditto.