Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(89)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2886943007: Introduce NavigatorWebDriver interface

Created:
3 years, 7 months ago by Sergey Shekyan
Modified:
3 years, 7 months ago
Reviewers:
bokan
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, blink-reviews-w3ctests_chromium.org, creis+watch_chromium.org, kinuko+watch, blink-reviews-api_chromium.org, nasko+codewatch_chromium.org, jam, shimazu+worker_chromium.org, kinuko+worker_chromium.org, dglazkov+blink, darin-cc_chromium.org, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, blink-reviews-frames_chromium.org, blink-worker-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Introduce NavigatorWebDriver interface Introduce NavigatorWebDriver interface that indicates if navigator is controlled by automation. BUG=723900 https://www.w3.org/TR/webdriver/#interface

Patch Set 1 #

Patch Set 2 : Introduce NavigatorWebDriver interface #

Total comments: 3

Patch Set 3 : wip: need more tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+101 lines, -0 lines) Patch
M content/browser/renderer_host/render_view_host_impl.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M content/public/common/common_param_traits_macros.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_switches.cc View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M content/public/common/web_preferences.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M content/public/common/web_preferences.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/interfaces/html.idl View 3 chunks +7 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/workers/interfaces.idl View 2 chunks +6 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/navigator_webdriver/navigator_webdriver.html View 1 2 1 chunk +22 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/core_idl_files.gni View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Navigator.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Navigator.cpp View 1 chunk +11 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Navigator.idl View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/core/frame/NavigatorWebDriver.idl View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/Settings.json5 View 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebSettingsImpl.cpp View 1 chunk +8 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebSettings.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (2 generated)
Sergey Shekyan
Guidance on how to organize tests round this is highly appreciated.
3 years, 7 months ago (2017-05-18 00:09:29 UTC) #2
bokan
3 years, 7 months ago (2017-05-19 16:43:23 UTC) #4 
See https://www.chromium.org/blink/launching-features

This is a fairly small feature but we are changing a public API so you'll need
to send out an intent to (implement and ship) to blink-dev and get lgtms + some
of the other process (I think you can skip TAG review in this case).

As for testing, it'd be nice to test end-to-end adding the command line flag and
running the JS but I don't think we have good support for that and I'm not sure
where to put it. Instead, I'd add a Blink SimTest in WebFrameTest.cpp (see
WebFrameSimTest), change the setting with WebView()->GetSettings() and check
that navigator.webdriver returns the correct thing. For testing the setting on
the other side of Chrome/Blink, see TEST_F(RenderViewImplBlinkSettingsTest,
CommandLine) in render_view_browsertest. Actually, it looks like you might be
able to do that and then load some HTML/JS and check the value to test
end-to-end, give that a try.

https://codereview.chromium.org/2886943007/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_view_host_impl.cc (right):

https://codereview.chromium.org/2886943007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_view_host_impl.cc:110: const char
kEnableAutomation[] = "enable-automation";
This should go in content_switches.cc/h

https://codereview.chromium.org/2886943007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_view_host_impl.cc:563: prefs.headless =
true;
Would it make sense to just set automation_controller = true if kHeadless is on
the command line, rather than adding a separate setting? It seems like you just
want headless to imply automation_controlled.

https://codereview.chromium.org/2886943007/diff/20001/content/public/common/w...
File content/public/common/web_preferences.h (right):

https://codereview.chromium.org/2886943007/diff/20001/content/public/common/w...
content/public/common/web_preferences.h:292: bool automation_controlled;
Please add some comments to describe what these do and give some context.

Powered by Google App Engine
This is Rietveld 408576698