From perezju:

I was also hoping to avoid the problem of "having to rename twice", but the fact that the API can be currently accessed through different layers (mainly action_runner and web_contents, but also a few others internally) with similarly named methods but slightly different interfaces was making it really hard to keep track and distinguish clients that need fixing from those that have been already migrated.

Browsing through the uses of the API in ChromeOS, it looks like this is mostly done through web_contents (i.e. tab) objects. If that sounds correct we might try to arrange something special to avoid having you rename everything twice.

The main changes you would have to do are:
For Execute/EvaluateJavaScript, make sure that the optional timeout argument is always given with an explicit keyword (e.g. timeout=60), rather ran relying on it's position on the list of arguments.
Rename WaitForJavaScriptExpression to WaitForJavaScriptCondition (and make sure to use the explicit timeout keyword when needed).
Optionally, you could also make use of the new API facilities to provide more robust interpolation of values in JS code. But for this you will have to wait until we finish the second renaming on our side, when the new features will become available to you under the old method names.

On the other hand, if you do have a few uses of the API via action_runner scattered around, you might want to consider whether the two step renaming would be a safer option.

Yup, we only use the web_contents API in ChromeOS. We have a few places where we use a timeout, but we always use the explicit timeout keyword. 

WaitForJavaScriptExpression is used less frequently, and we can rename this when the new API is available. The JS we care about is pretty simple and the new python string interpolation is not likely to come into play.

We don't use action_runner anywhere that I'm aware of.

From my query [1], there seem to be several clients, most (all?) of them in third_party/autotest.

On those results I see plenty of examples where the timeout keyword is not given explicitly, e.g.: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/client/cros/networking/chrome_testing/chrome_networking_test_context.py#77

Those should be fixed or will break after our API update.

Are there other places I should be looking? Or are those all of the client calls we need to worry about?

[1]: https://cs.corp.google.com/search/?q=%5C.%5Cw%2BJavaScript%5Cw*%5C(+package:%5Echromeos_public$&type=cs
(sorry for internal link, is there a public equivalent available?)

Yup, all the clients are in third_party/autotest, and yes, these are all the client calls we should worry about. 

I hadn't looked hard enough - I'll look over all the invocations and add the timeout arg name.

There is no public equivalent unfortunately.

Good, so we have a plan.

I'll take this one for now to prepare the interface changes you need from our end, basically I think just add a WaitForJavaScriptCondition (not Expression) since that will be the final name of that method, and assign back to you when that's ready.

sg, thank you!

List of files using JS apis in third_party/autotest/files:

client/bin/vm_sanity.py
client/common_lib/cros/arc_util.py
client/common_lib/cros/cfm_util.py
client/common_lib/cros/chrome.py
client/common_lib/cros/enrollment.py
client/common_lib/cros/interactive_xmlrpc_server.py
client/common_lib/cros/kiosk_utils.py
client/cros/a11y/a11y_test_base.py
client/cros/enterprise/enterprise_policy_base.py
client/cros/multimedia/audio_extension_handler.py
client/cros/multimedia/cfm_facade_native.py
client/cros/multimedia/display_facade_native.py
client/cros/multimedia/facade_resource.py
client/cros/networking/chrome_testing/chrome_networking_test_context.py
client/cros/semiauto_framework.py
client/cros/touch_playback_test_base.py
client/cros/video/histogram_verifier.py
client/cros/video/native_html5_player.py
client/cros/video/vimeo_player.py
client/cros/video/youtube_helper.py
client/site_tests/audio_ActiveStreamStress/audio_ActiveStreamStress.py
client/site_tests/audio_AudioCorruption/audio_AudioCorruption.py
client/site_tests/audio_CrasSanity/audio_CrasSanity.py
client/site_tests/audio_PlaybackPower/audio_PlaybackPower.py
client/site_tests/audio_SeekAudioFeedback/audio_SeekAudioFeedback.py
client/site_tests/desktopui_AudioFeedback/desktopui_AudioFeedback.py
client/site_tests/desktopui_CameraApp/desktopui_CameraApp.py
client/site_tests/desktopui_ConnectivityDiagnostics/desktopui_ConnectivityDiagnostics.py
client/site_tests/desktopui_MediaAudioFeedback/desktopui_MediaAudioFeedback.py
client/site_tests/desktopui_ScreenLocker/desktopui_ScreenLocker.py
client/site_tests/documentscan_AppTestWithFakeLorgnette/documentscan_AppTestWithFakeLorgnette.py
client/site_tests/enterprise_KioskEnrollment/enterprise_KioskEnrollment.py
client/site_tests/enterprise_RemoraRequisition/enterprise_RemoraRequisition.py
client/site_tests/graphics_Stress/graphics_Stress.py
client/site_tests/graphics_WebGLAquarium/graphics_WebGLAquarium.py
client/site_tests/graphics_WebGLClear/graphics_WebGLClear.py
client/site_tests/graphics_WebGLManyPlanetsDeep/graphics_WebGLManyPlanetsDeep.py
client/site_tests/graphics_WebGLPerformance/graphics_WebGLPerformance.py
client/site_tests/logging_AsanCrash/logging_AsanCrash.py
client/site_tests/logging_FeedbackReport/logging_FeedbackReport.py
client/site_tests/login_GaiaLogin/login_GaiaLogin.py
client/site_tests/login_OobeLocalization/login_OobeLocalization.py
client/site_tests/network_FirewallHolePunch/network_FirewallHolePunch.py
client/site_tests/platform_InputBrowserNav/platform_InputBrowserNav.py
client/site_tests/policy_AutoFillEnabled/policy_AutoFillEnabled.py
client/site_tests/policy_EditBookmarksEnabled/policy_EditBookmarksEnabled.py
client/site_tests/policy_ForceYouTubeSafetyMode/policy_ForceYouTubeSafetyMode.py
client/site_tests/policy_ImagesBlockedForUrls/policy_ImagesBlockedForUrls.py
client/site_tests/policy_JavaScriptBlockedForUrls/policy_JavaScriptBlockedForUrls.py
client/site_tests/policy_ManagedBookmarks/policy_ManagedBookmarks.py
client/site_tests/policy_NotificationsAllowedForUrls/policy_NotificationsAllowedForUrls.py
client/site_tests/policy_PluginsBlockedForUrls/policy_PluginsBlockedForUrls.py
client/site_tests/policy_PopupsBlockedForUrls/policy_PopupsBlockedForUrls.py
client/site_tests/policy_URLBlacklist/policy_URLBlacklist.py
client/site_tests/policy_URLWhitelist/policy_URLWhitelist.py
client/site_tests/power_Consumption/power_Consumption.py
client/site_tests/power_FlashVideoSuspend/power_FlashVideoSuspend.py
client/site_tests/power_LoadTest/power_LoadTest.py
client/site_tests/power_VideoSuspend/power_VideoSuspend.py
client/site_tests/security_BundledExtensions/security_BundledExtensions.py
client/site_tests/security_SandboxStatus/security_SandboxStatus.py
client/site_tests/telemetry_AFDOGenerateClient/telemetry_AFDOGenerateClient.py
client/site_tests/touch_TabSwitch/touch_TabSwitch.py
client/site_tests/video_ChromeRTCHWEncodeUsed/video_ChromeRTCHWEncodeUsed.py
client/site_tests/video_MultiplePlayback/video_MultiplePlayback.py
client/site_tests/video_PlaybackPerf/video_PlaybackPerf.py
client/site_tests/video_VideoDecodeMemoryUsage/video_VideoDecodeMemoryUsage.py
client/site_tests/video_VideoSeek/video_VideoSeek.py
client/site_tests/video_VimeoVideo/video_VimeoVideo.py
client/site_tests/video_WebRtcCamera/video_WebRtcCamera.py
client/site_tests/video_WebRtcMediaRecorder/video_WebRtcMediaRecorder.py
client/site_tests/video_WebRtcPeerConnectionWithCamera/video_WebRtcPeerConnectionWithCamera.py
client/site_tests/video_WebRtcPerf/video_WebRtcPerf.py
client/site_tests/video_WebRtcSanity/video_WebRtcSanity.py
client/site_tests/video_YouTubeHTML5/video_YouTubeHTML5.py
client/site_tests/video_YouTubeMseEme/video_YouTubeMseEme.py
client/site_tests/video_YouTubePage/video_YouTubePage.py

Most of the omissions were in WaitForJavaScriptExpression
https://chromium-review.googlesource.com/#/c/431959/

Files using WaitForJavaScriptExpression (which will need a rename):
client/common_lib/cros/arc_util.py
client/common_lib/cros/cfm_util.py
client/common_lib/cros/enrollment.py
client/common_lib/cros/kiosk_utils.py
client/cros/multimedia/display_facade_native.py
client/cros/multimedia/facade_resource.py
client/cros/networking/chrome_testing/chrome_networking_test_context.py
client/cros/video/native_html5_player.py
client/site_tests/graphics_WebGLPerformance/graphics_WebGLPerformance.py
client/site_tests/policy_EditBookmarksEnabled/policy_EditBookmarksEnabled.py
client/site_tests/policy_ManagedBookmarks/policy_ManagedBookmarks.py
client/site_tests/power_FlashVideoSuspend/power_FlashVideoSuspend.py
client/site_tests/telemetry_AFDOGenerateClient/telemetry_AFDOGenerateClient.py
client/site_tests/video_MultiplePlayback/video_MultiplePlayback.py
client/site_tests/video_VimeoVideo/video_VimeoVideo.py
client/site_tests/video_YouTubeMseEme/video_YouTubeMseEme.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/eb5db992b43406802e561e7940866185eb996073

commit eb5db992b43406802e561e7940866185eb996073
Author: Achuith Bhandarkar <achuith@chromium.org>
Date: Tue Jan 24 21:18:23 2017

autotest: Use explicit timeout keyword in JavaScript calls.

BUG= chromium:682812 
TEST=trybots

Change-Id: I1ae37ef0625d29d299878698b9cda8468eb430bd
Reviewed-on: https://chromium-review.googlesource.com/431959
Commit-Ready: Achuith Bhandarkar <achuith@chromium.org>
Tested-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>

[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/video_MultiplePlayback/video_MultiplePlayback.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/cros/networking/chrome_testing/chrome_networking_test_context.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/common_lib/cros/arc_util.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/telemetry_AFDOGenerateClient/telemetry_AFDOGenerateClient.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/graphics_WebGLPerformance/graphics_WebGLPerformance.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/video_VimeoVideo/video_VimeoVideo.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/policy_ManagedBookmarks/policy_ManagedBookmarks.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/common_lib/cros/enrollment.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/common_lib/cros/cfm_util.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/policy_EditBookmarksEnabled/policy_EditBookmarksEnabled.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/cros/multimedia/facade_resource.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/power_FlashVideoSuspend/power_FlashVideoSuspend.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/common_lib/cros/kiosk_utils.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/site_tests/video_YouTubeMseEme/video_YouTubeMseEme.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/cros/video/native_html5_player.py
[modify] https://crrev.com/eb5db992b43406802e561e7940866185eb996073/client/cros/multimedia/display_facade_native.py

https://codereview.chromium.org/2661683002/ has just landed. As soon as that rolls into your project feel free to proceed with the rest of the migration.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee272d6c846e19f2019bf8a0ba09316c1d561469

commit ee272d6c846e19f2019bf8a0ba09316c1d561469
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Fri Jan 27 22:45:32 2017

Roll src/third_party/catapult/ 3c9b30e0c..9907db54e (5 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/3c9b30e0cc0d..9907db54ee40

$ git log 3c9b30e0c..9907db54e --date=short --no-merges --format='%ad %ae %s'
2017-01-27 perezju [Telemetry] Add web_contents.WaitForJavaScriptCondition method
2017-01-27 charliea Add HistogramSet.getHistogramNamed and use it where applicable
2017-01-27 benjhayden Move Scalar to base.
2017-01-27 charliea Rename HistogramSet.getValuesNamed to HistogramSet.getHistogramsNamed
2017-01-27 eakuefner [StyleGuide] Allow let in JavaScript code

BUG= 682812 

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2659123002
Cr-Commit-Position: refs/heads/master@{#446805}

[modify] https://crrev.com/ee272d6c846e19f2019bf8a0ba09316c1d561469/DEPS

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/0309dc99e4f2ffae6211c80e4e46a62579807486

commit 0309dc99e4f2ffae6211c80e4e46a62579807486
Author: Achuith Bhandarkar <achuith@chromium.org>
Date: Sat Feb 04 04:01:50 2017

autotest: Replace WaitForJavaScriptExpression with WaitForJavaScriptCondition.

BUG= chromium:682812 
TEST=trybot

Change-Id: I8a71fe24c04f31d401a1925251c5d221accb4d46
Reviewed-on: https://chromium-review.googlesource.com/436528
Commit-Ready: Achuith Bhandarkar <achuith@chromium.org>
Tested-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>

[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/video_MultiplePlayback/video_MultiplePlayback.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/cros/networking/chrome_testing/chrome_networking_test_context.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/common_lib/cros/arc_util.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/telemetry_AFDOGenerateClient/telemetry_AFDOGenerateClient.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/graphics_WebGLPerformance/graphics_WebGLPerformance.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/video_VimeoVideo/video_VimeoVideo.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/policy_ManagedBookmarks/policy_ManagedBookmarks.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/common_lib/cros/enrollment.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/common_lib/cros/cfm_util.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/policy_EditBookmarksEnabled/policy_EditBookmarksEnabled.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/cros/multimedia/facade_resource.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/power_FlashVideoSuspend/power_FlashVideoSuspend.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/common_lib/cros/kiosk_utils.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/site_tests/video_YouTubeMseEme/video_YouTubeMseEme.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/cros/video/native_html5_player.py
[modify] https://crrev.com/0309dc99e4f2ffae6211c80e4e46a62579807486/client/cros/multimedia/display_facade_native.py

Juan - I believe I've switched all instances of WaitForJavaScriptExpression over to WaitForJavaScriptCondition.

Thanks, sounds great! On our side we're about half way though the migration. I think we can close this bug now and I'll follow up on telemetry-announce and the main tracking bug when we switch over these methods to their new implementations.

Shouldn't affect you anymore, but just to make sure that the transition goes smoothly.