[webkit-reviews] review granted: [Bug 69365] [WK2] WebKitTestRunner needs LayoutTestController.dumpConfigurationForViewport : [Attachment 109809] patch 1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chang Shu <cshu at webkit.org>'s request
for review:
Bug 69365: [WK2] WebKitTestRunner needs
LayoutTestController.dumpConfigurationForViewport
https://bugs.webkit.org/show_bug.cgi?id=69365

Attachment 109809: patch 1
https://bugs.webkit.org/attachment.cgi?id=109809&action=review

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


> Source/WebKit2/WebProcess/WebPage/WebPage.h:437
> +    // For Testing purpose

This has grammar and formatting problems and should say:

    // For testing purposes.

Or:

    // For testing purposes:

> Source/WebKit2/WebProcess/WebPage/WebPage.h:441
> +    String viewportAsText(int deviceDPI, int deviceWidth, int deviceHeight,
int availableWidth, int availableHeight);

This is not a good name for the function because it does not return “the
viewport” it returns “viewport attributes”. So it should be named
viewportAttributesAsText or viewportConfigurationAsText. Leaving out the AsText
suffix might be OK too.