[Telemetry] Fix JavaScript interpolation in telemetry actions

[Telemetry] Fix JavaScript interpolation in telemetry actions

Telemetry actions expose API's for clients to interact with elements on
a page (e.g. scroll or tap). Some of these take parameters (e.g. a CSS
selector) which were naively interpolated into JavaScript snippets, but
this is brittle when special characters are involved.

In this CL we migrate all of these to use the js_template helper, which
takes care of safely inserting Python values into the JavaScript
snippets.

BUG=catapult:#3028

Review-Url: https://codereview.chromium.org/2559503002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/c78686903901261433d5abf72d94f0052524b5c7

[perezju, 2016-12-06 16:11:21]

The CQ bit was checked by perezju@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 16:11:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[perezju, 2016-12-06 16:26:40]

perezju@chromium.org changed reviewers:
+ nednguyen@google.com

[perezju, 2016-12-06 16:26:41]

https://codereview.chromium.org/2559503002/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/actions/drag.py (right):

https://codereview.chromium.org/2559503002/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/actions/drag.py:70: callback='function() {
window.__dragActionDone = true; }')
This idiom is used in several files. Not sure exactly why since the "callback"
can be typed directly into the snippet. I guess the reason was readability?

Anyway, I tried to keep the intention of the original, but would also be happy
to remove the template rendering stuff since it's not really needed in these
cases.

https://codereview.chromium.org/2559503002/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/actions/seek.py (right):

https://codereview.chromium.org/2559503002/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/actions/seek.py:48: seconds=str(self._seconds),  # 
TODO: Is string convertion intended?
Looking at
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...
I think the answer is "no", since element.currentTime ought to be a number. But
maybe I'm missing some subtle js thing?

Not sure if "fixing" this here (i.e. remove the extra "str"), wait for a follow
up CL, or just live it alone (it appears to be working anyway).

[perezju, 2016-12-09 11:43:23]

ping

[nednguyen, 2016-12-09 12:29:08]

lgtm with nits

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/drag.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/drag.py:69: window.__dragAction = new
__DragAction({{ @callback }});''',
not sure why we don't just inline the callback code

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/mouse_click.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/mouse_click.py:23: callback='function() {
window.__mouseClickActionDone = true; }')
same here,  just inline the callback

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/pinch.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/pinch.py:43: callback='function() {
window.__pinchActionDone = true; }')
ditto

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/swipe.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/swipe.py:52: callback='function() {
window.__swipeActionDone = true; }')
ditto

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/tap.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/tap.py:38: callback='function() {
window.__tapActionDone = true; }')
ditto

[perezju, 2016-12-14 12:18:10]

The CQ bit was checked by perezju@chromium.org

[perezju, 2016-12-14 12:18:11]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2559503002/#ps60001
(title: "inline js literals")

[commit-bot: I haz the power, 2016-12-14 12:18:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[perezju, 2016-12-14 12:18:21]

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/drag.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/drag.py:69: window.__dragAction = new
__DragAction({{ @callback }});''',
On 2016/12/09 12:29:07, nednguyen wrote:
> not sure why we don't just inline the callback code

Done. Yeah, not sure why it was done that way either, the code is even cleaner
when inlined.

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/mouse_click.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/mouse_click.py:23: callback='function() {
window.__mouseClickActionDone = true; }')
On 2016/12/09 12:29:07, nednguyen wrote:
> same here,  just inline the callback

Done.

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/pinch.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/pinch.py:43: callback='function() {
window.__pinchActionDone = true; }')
On 2016/12/09 12:29:07, nednguyen wrote:
> ditto

Done.

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/swipe.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/swipe.py:52: callback='function() {
window.__swipeActionDone = true; }')
On 2016/12/09 12:29:07, nednguyen wrote:
> ditto

Done.

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/actions/tap.py (right):

https://codereview.chromium.org/2559503002/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/actions/tap.py:38: callback='function() {
window.__tapActionDone = true; }')
On 2016/12/09 12:29:07, nednguyen wrote:
> ditto

Done.

[commit-bot: I haz the power, 2016-12-14 12:48:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481717890655430,
"parent_rev": "041b58513c9679b8d609c6e67f50e37ed7e893d3", "commit_rev":
"c78686903901261433d5abf72d94f0052524b5c7"}

[commit-bot: I haz the power, 2016-12-14 12:48:46]

Description was changed from

==========
[Telemetry] Fix JavaScript interpolation in telemetry actions

Telemetry actions expose API's for clients to interact with elements on
a page (e.g. scroll or tap). Some of these take parameters (e.g. a CSS
selector) which were naively interpolated into JavaScript snippets, but
this is brittle when special characters are involved.

In this CL we migrate all of these to use the js_template helper, which
takes care of safely inserting Python values into the JavaScript
snippets.

BUG=catapult:#3028
==========

to

==========
[Telemetry] Fix JavaScript interpolation in telemetry actions

Telemetry actions expose API's for clients to interact with elements on
a page (e.g. scroll or tap). Some of these take parameters (e.g. a CSS
selector) which were naively interpolated into JavaScript snippets, but
this is brittle when special characters are involved.

In this CL we migrate all of these to use the js_template helper, which
takes care of safely inserting Python values into the JavaScript
snippets.

BUG=catapult:#3028

Review-Url: https://codereview.chromium.org/2559503002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-12-14 12:48:47]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...