[webkit-reviews] review denied: [Bug 171997] navigator.webdriver should return false if the page is not controlled by automation : [Attachment 310918] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied Sergey Shekyan <shekyan at gmail.com>'s
request for review:
Bug 171997: navigator.webdriver should return false if the page is not
controlled by automation
https://bugs.webkit.org/show_bug.cgi?id=171997

Attachment 310918: Patch

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




--- Comment #14 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 310918
  --> https://bugs.webkit.org/attachment.cgi?id=310918
Patch

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

Patch looks good otherwise.

> Source/WebCore/ChangeLog:5
> +	   Per WebDriver Specification at
https://www.w3.org/TR/webdriver/#interface

The description should come *after* the reviewed by line.

> Source/WebCore/Modules/webdriver/NavigatorWebDriver.cpp:52
> +    Frame* frame = navigator.frame();

auto*

> Source/WebCore/Modules/webdriver/NavigatorWebDriver.cpp:53
> +    if (frame || !frame->page())

This check seems wrong, should probably !frame.

> LayoutTests/js/dom/navigator-webdriver.html:8
> +description(

Just description("Check that navigator.webdriver has the right attributes.");

> LayoutTests/js/dom/navigator-webdriver.html:12
> +if (window.testRunner) {

We don't use curly brackets for one-liners.