Use local pages for webrtc telemetry tests.

Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
NOTRY=True

Review-Url: https://codereview.chromium.org/2761163003
Cr-Commit-Position: refs/heads/master@{#460078}
Committed: https://chromium.googlesource.com/chromium/src/+/78eee2ed541a59974558681e9165a18a0fe5028b

[ehmaldonado_chromium, 2017-03-21 15:01:28]

Description was changed from

==========
Downlead pages.

BUG=
==========

to

==========
Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
R=nednguyen@google.com
==========

[ehmaldonado_chromium, 2017-03-21 15:01:28]

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

[ehmaldonado_chromium, 2017-03-21 15:01:36]

Patchset #1 (id:1) has been deleted

[ehmaldonado_chromium, 2017-03-21 15:02:09]

Please take a look.

[nednguyen, 2017-03-21 15:05:14]

https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
File tools/perf/page_sets/webrtc_cases.py (right):

https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
tools/perf/page_sets/webrtc_cases.py:12: def __init__(self, url, page_set,
name):
Add assertion that webrtc apge must be starting with "file://"

https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
tools/perf/page_sets/webrtc_cases.py:146:
action_runner.ClickElement('button[id="startButton"]')
why are you changing the action to run on the page?

[ehmaldonado_chromium, 2017-03-21 15:20:00]

On 2017/03/21 15:05:14, nednguyen wrote:
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> File tools/perf/page_sets/webrtc_cases.py (right):
> 
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> tools/perf/page_sets/webrtc_cases.py:12: def __init__(self, url, page_set,
> name):
> Add assertion that webrtc apge must be starting with "file://"
> 
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> tools/perf/page_sets/webrtc_cases.py:146:
> action_runner.ClickElement('button[id="startButton"]')
> why are you changing the action to run on the page?

I had to modify the page for our repo.

[nednguyen, 2017-03-21 15:24:32]

On 2017/03/21 15:20:00, ehmaldonado_chromium wrote:
> On 2017/03/21 15:05:14, nednguyen wrote:
> >
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> > File tools/perf/page_sets/webrtc_cases.py (right):
> > 
> >
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> > tools/perf/page_sets/webrtc_cases.py:12: def __init__(self, url, page_set,
> > name):
> > Add assertion that webrtc apge must be starting with "file://"
> > 
> >
>
https://codereview.chromium.org/2761163003/diff/20001/tools/perf/page_sets/we...
> > tools/perf/page_sets/webrtc_cases.py:146:
> > action_runner.ClickElement('button[id="startButton"]')
> > why are you changing the action to run on the page?
> 
> I had to modify the page for our repo.

lgtm but please add the assert that webrtc page must be using "file://" & have
another webrtc engineer who is familiar with this to review your change.

[ehmaldonado_chromium, 2017-03-21 15:43:12]

> lgtm but please add the assert that webrtc page must be using "file://" & have
> another webrtc engineer who is familiar with this to review your change.

Done. 
Also, the only other webrtc engineer who's familiar with this is on parental
leave until September...

I'm getting presubmit warnings telling me to modify the Copyright notice of the
.js files. I'm just ignoring them, is it a problem?

[nednguyen, 2017-03-21 16:11:53]

On 2017/03/21 15:43:12, ehmaldonado_chromium wrote:
> > lgtm but please add the assert that webrtc page must be using "file://" &
have
> > another webrtc engineer who is familiar with this to review your change.
> 
> Done. 
> Also, the only other webrtc engineer who's familiar with this is on parental
> leave until September...
> 
> I'm getting presubmit warnings telling me to modify the Copyright notice of
the
> .js files. I'm just ignoring them, is it a problem?

Please don't ignore the Presubmit warning.

[kjellander_chromium, 2017-03-21 16:46:15]

kjellander@chromium.org changed reviewers:
+ kjellander@chromium.org

[kjellander_chromium, 2017-03-21 16:46:16]

A couple of questions:
1. How did you pull down the files from github? The css files with (1) in the
name doesn't seem to exist in the GitHub repo. I'd like to have these files as
similar as possible.
2. Would it make sense to write a python script that clones the repo and then
copies + transforms (updates license header and so on) the files instead? Then
we could easily keep them up to date.
3. Don't we need to pass some flag to Chrome to be able to use pages from
file:// URLs?

[ehmaldonado_chromium, 2017-03-21 16:48:56]

> Please don't ignore the Presubmit warning.

I don't think it's worth it to make the adapter-latest.js files' lines have less
than 80 characters, for example.
Is there a way to disable them?

[ehmaldonado_chromium, 2017-03-21 16:51:22]

On 2017/03/21 16:46:16, kjellander_chromium wrote:
> A couple of questions:
> 1. How did you pull down the files from github? The css files with (1) in the
> name doesn't seem to exist in the GitHub repo. I'd like to have these files as
> similar as possible.
I navigated to them, right click, and saved them.

> 2. Would it make sense to write a python script that clones the repo and then
> copies + transforms (updates license header and so on) the files instead? Then
> we could easily keep them up to date.
I don't think it's worth the effort to keep them up to date. I'm not sure
they're kept up to date today.

> 3. Don't we need to pass some flag to Chrome to be able to use pages from
> file:// URLs?
Nope.

[kjellander_chromium, 2017-03-21 18:14:06]

On 2017/03/21 16:51:22, ehmaldonado_chromium wrote:
> On 2017/03/21 16:46:16, kjellander_chromium wrote:
> > A couple of questions:
> > 1. How did you pull down the files from github? The css files with (1) in
the
> > name doesn't seem to exist in the GitHub repo. I'd like to have these files
as
> > similar as possible.
> I navigated to them, right click, and saved them.
> 
> > 2. Would it make sense to write a python script that clones the repo and
then
> > copies + transforms (updates license header and so on) the files instead?
Then
> > we could easily keep them up to date.
> I don't think it's worth the effort to keep them up to date. I'm not sure
> they're kept up to date today.

Hmm, I don't agree. adapter.js is changed frequently so we'll probably have to
keep it up to date every now and then. There's also a fair amount of work
planned for Q2 to make Chrome more spec compliant with WebRTC 1.0, so I think
that might also require us to update these pages.
I think the reason they're not kept up to date more frequently is mainly caused
by the fact that there's no easy way to do it. A script that clones the github
repo (or just updates an existing clone) then copies a selected set of files
(maintaining the same hierarchy), is the best way to do this in a maintainable
way.

> > 3. Don't we need to pass some flag to Chrome to be able to use pages from
> > file:// URLs?
> Nope.

Great!

[nednguyen, 2017-03-21 18:17:07]

On 2017/03/21 18:14:06, kjellander_chromium wrote:
> On 2017/03/21 16:51:22, ehmaldonado_chromium wrote:
> > On 2017/03/21 16:46:16, kjellander_chromium wrote:
> > > A couple of questions:
> > > 1. How did you pull down the files from github? The css files with (1) in
> the
> > > name doesn't seem to exist in the GitHub repo. I'd like to have these
files
> as
> > > similar as possible.
> > I navigated to them, right click, and saved them.
> > 
> > > 2. Would it make sense to write a python script that clones the repo and
> then
> > > copies + transforms (updates license header and so on) the files instead?
> Then
> > > we could easily keep them up to date.
> > I don't think it's worth the effort to keep them up to date. I'm not sure
> > they're kept up to date today.
> 
> Hmm, I don't agree. adapter.js is changed frequently so we'll probably have to
> keep it up to date every now and then. There's also a fair amount of work
> planned for Q2 to make Chrome more spec compliant with WebRTC 1.0, so I think
> that might also require us to update these pages.
> I think the reason they're not kept up to date more frequently is mainly
caused
> by the fact that there's no easy way to do it. A script that clones the github
> repo (or just updates an existing clone) then copies a selected set of files
> (maintaining the same hierarchy), is the best way to do this in a maintainable
> way.
> 
> > > 3. Don't we need to pass some flag to Chrome to be able to use pages from
> > > file:// URLs?
> > Nope.
> 
> Great!

IIRC, there is a file somewhere that you can include to blacklist license of
files given that you don't need to ship them with the browser. IMO, I recommend
adding license since Telemetry performance tests are also used by other partners
like Sam Sung, Nvidia, etc..

[ehmaldonado_chromium, 2017-03-23 16:16:51]

Ned: 
This doesn't seem to work for android. And neither does
media.android.tough_video_cases [1] which also uses local files [2].
Am I missing something, or this approach doesn't work on Android?


[1] https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
[2]
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...

[nednguyen, 2017-03-23 16:21:33]

On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> Ned: 
> This doesn't seem to work for android. And neither does
> media.android.tough_video_cases [1] which also uses local files [2].
> Am I missing something, or this approach doesn't work on Android?
> 
> 
> [1] https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> [2]
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...

How are other webrtc benchmark works on Android?

TO see how local file works on Android, you can try to run s.t like:
blink_perf.bindings

[ehmaldonado_chromium, 2017-03-23 16:28:00]

On 2017/03/23 16:21:33, nednguyen wrote:
> On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > Ned: 
> > This doesn't seem to work for android. And neither does
> > media.android.tough_video_cases [1] which also uses local files [2].
> > Am I missing something, or this approach doesn't work on Android?
> > 
> > 
> > [1]
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > [2]
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> 
> How are other webrtc benchmark works on Android?
> 
> TO see how local file works on Android, you can try to run s.t like:
> blink_perf.bindings

I ran:

tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
--browser=android-chromium

And I got:

This site can't be reached
127.0.0.1 refused to connect.

[nednguyen, 2017-03-23 16:32:01]

On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> On 2017/03/23 16:21:33, nednguyen wrote:
> > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > Ned: 
> > > This doesn't seem to work for android. And neither does
> > > media.android.tough_video_cases [1] which also uses local files [2].
> > > Am I missing something, or this approach doesn't work on Android?
> > > 
> > > 
> > > [1]
> https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > [2]
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > 
> > How are other webrtc benchmark works on Android?
> > 
> > TO see how local file works on Android, you can try to run s.t like:
> > blink_perf.bindings
> 
> I ran:
> 
> tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> --browser=android-chromium
> 
> And I got:
> 
> This site can't be reached
> 127.0.0.1 refused to connect.

Hmhh, I think that may mean you need to setup your test device. Please see:
go/telemetry-device-setup

[ehmaldonado_chromium, 2017-03-23 17:00:17]

On 2017/03/23 16:32:01, nednguyen wrote:
> On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > On 2017/03/23 16:21:33, nednguyen wrote:
> > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > Ned: 
> > > > This doesn't seem to work for android. And neither does
> > > > media.android.tough_video_cases [1] which also uses local files [2].
> > > > Am I missing something, or this approach doesn't work on Android?
> > > > 
> > > > 
> > > > [1]
> > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > [2]
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > 
> > > How are other webrtc benchmark works on Android?
> > > 
> > > TO see how local file works on Android, you can try to run s.t like:
> > > blink_perf.bindings
> > 
> > I ran:
> > 
> > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > --browser=android-chromium
> > 
> > And I got:
> > 
> > This site can't be reached
> > 127.0.0.1 refused to connect.
> 
> Hmhh, I think that may mean you need to setup your test device. Please see:
> go/telemetry-device-setup

Still the same.

[ehmaldonado_chromium, 2017-03-23 20:03:09]

On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> On 2017/03/23 16:32:01, nednguyen wrote:
> > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > Ned: 
> > > > > This doesn't seem to work for android. And neither does
> > > > > media.android.tough_video_cases [1] which also uses local files [2].
> > > > > Am I missing something, or this approach doesn't work on Android?
> > > > > 
> > > > > 
> > > > > [1]
> > > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > [2]
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > 
> > > > How are other webrtc benchmark works on Android?
> > > > 
> > > > TO see how local file works on Android, you can try to run s.t like:
> > > > blink_perf.bindings
> > > 
> > > I ran:
> > > 
> > > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > > --browser=android-chromium
> > > 
> > > And I got:
> > > 
> > > This site can't be reached
> > > 127.0.0.1 refused to connect.
> > 
> > Hmhh, I think that may mean you need to setup your test device. Please see:
> > go/telemetry-device-setup
> 
> Still the same.

Does it work for you?
Is it possible to run it on swarming, to check that it works?

[nednguyen, 2017-03-23 20:17:59]

On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > On 2017/03/23 16:32:01, nednguyen wrote:
> > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > Ned: 
> > > > > > This doesn't seem to work for android. And neither does
> > > > > > media.android.tough_video_cases [1] which also uses local files [2].
> > > > > > Am I missing something, or this approach doesn't work on Android?
> > > > > > 
> > > > > > 
> > > > > > [1]
> > > >
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > [2]
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > 
> > > > > How are other webrtc benchmark works on Android?
> > > > > 
> > > > > TO see how local file works on Android, you can try to run s.t like:
> > > > > blink_perf.bindings
> > > > 
> > > > I ran:
> > > > 
> > > > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > > > --browser=android-chromium
> > > > 
> > > > And I got:
> > > > 
> > > > This site can't be reached
> > > > 127.0.0.1 refused to connect.
> > > 
> > > Hmhh, I think that may mean you need to setup your test device. Please
see:
> > > go/telemetry-device-setup
> > 
> > Still the same.
> 
> Does it work for you?
> Is it possible to run it on swarming, to check that it works?

You can run trybot
(https://www.chromium.org/developers/telemetry/performance-try-bots), though I
also suggest working with other engineers to make sure that you can run
telemetry benchmark locally on Android. Otherwise, in the future you will have
bad time debugging the failures.

[nednguyen, 2017-03-24 00:37:42]

On 2017/03/23 20:17:59, nednguyen wrote:
> On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > Ned: 
> > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > media.android.tough_video_cases [1] which also uses local files
[2].
> > > > > > > Am I missing something, or this approach doesn't work on Android?
> > > > > > > 
> > > > > > > 
> > > > > > > [1]
> > > > >
> https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > [2]
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > 
> > > > > > How are other webrtc benchmark works on Android?
> > > > > > 
> > > > > > TO see how local file works on Android, you can try to run s.t like:
> > > > > > blink_perf.bindings
> > > > > 
> > > > > I ran:
> > > > > 
> > > > > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > > > > --browser=android-chromium
> > > > > 
> > > > > And I got:
> > > > > 
> > > > > This site can't be reached
> > > > > 127.0.0.1 refused to connect.
> > > > 
> > > > Hmhh, I think that may mean you need to setup your test device. Please
> see:
> > > > go/telemetry-device-setup
> > > 
> > > Still the same.
> > 
> > Does it work for you?
> > Is it possible to run it on swarming, to check that it works?
> 
> You can run trybot
> (https://www.chromium.org/developers/telemetry/performance-try-bots), though I
> also suggest working with other engineers to make sure that you can run
> telemetry benchmark locally on Android. Otherwise, in the future you will have
> bad time debugging the failures.

Actually the problem you are facing may be due to
https://bugs.chromium.org/p/chromium/issues/detail?id=704782

[ehmaldonado_chromium, 2017-03-24 14:37:40]

Patchset #3 (id:40001) has been deleted

[ehmaldonado_chromium, 2017-03-24 18:21:25]

On 2017/03/24 00:37:42, nednguyen wrote:
> On 2017/03/23 20:17:59, nednguyen wrote:
> > On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > > Ned: 
> > > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > > media.android.tough_video_cases [1] which also uses local files
> [2].
> > > > > > > > Am I missing something, or this approach doesn't work on
Android?
> > > > > > > > 
> > > > > > > > 
> > > > > > > > [1]
> > > > > >
> > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > > [2]
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > > 
> > > > > > > How are other webrtc benchmark works on Android?
> > > > > > > 
> > > > > > > TO see how local file works on Android, you can try to run s.t
like:
> > > > > > > blink_perf.bindings
> > > > > > 
> > > > > > I ran:
> > > > > > 
> > > > > > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > > > > > --browser=android-chromium
> > > > > > 
> > > > > > And I got:
> > > > > > 
> > > > > > This site can't be reached
> > > > > > 127.0.0.1 refused to connect.
> > > > > 
> > > > > Hmhh, I think that may mean you need to setup your test device. Please
> > see:
> > > > > go/telemetry-device-setup
> > > > 
> > > > Still the same.
> > > 
> > > Does it work for you?
> > > Is it possible to run it on swarming, to check that it works?
> > 
> > You can run trybot
> > (https://www.chromium.org/developers/telemetry/performance-try-bots), though
I
> > also suggest working with other engineers to make sure that you can run
> > telemetry benchmark locally on Android. Otherwise, in the future you will
have
> > bad time debugging the failures.
> 
> Actually the problem you are facing may be due to
> https://bugs.chromium.org/p/chromium/issues/detail?id=704782

Do the green trybots mean this CL would work on Android on the bots?

[nednguyen, 2017-03-24 18:29:08]

On 2017/03/24 18:21:25, ehmaldonado_chromium wrote:
> On 2017/03/24 00:37:42, nednguyen wrote:
> > On 2017/03/23 20:17:59, nednguyen wrote:
> > > On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > > > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > > > Ned: 
> > > > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > > > media.android.tough_video_cases [1] which also uses local
files
> > [2].
> > > > > > > > > Am I missing something, or this approach doesn't work on
> Android?
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > [1]
> > > > > > >
> > > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > > > [2]
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > > > 
> > > > > > > > How are other webrtc benchmark works on Android?
> > > > > > > > 
> > > > > > > > TO see how local file works on Android, you can try to run s.t
> like:
> > > > > > > > blink_perf.bindings
> > > > > > > 
> > > > > > > I ran:
> > > > > > > 
> > > > > > > tools/perf/run_benchmark -v blink_perf.bindings --device=06df61c3
> > > > > > > --browser=android-chromium
> > > > > > > 
> > > > > > > And I got:
> > > > > > > 
> > > > > > > This site can't be reached
> > > > > > > 127.0.0.1 refused to connect.
> > > > > > 
> > > > > > Hmhh, I think that may mean you need to setup your test device.
Please
> > > see:
> > > > > > go/telemetry-device-setup
> > > > > 
> > > > > Still the same.
> > > > 
> > > > Does it work for you?
> > > > Is it possible to run it on swarming, to check that it works?
> > > 
> > > You can run trybot
> > > (https://www.chromium.org/developers/telemetry/performance-try-bots),
though
> I
> > > also suggest working with other engineers to make sure that you can run
> > > telemetry benchmark locally on Android. Otherwise, in the future you will
> have
> > > bad time debugging the failures.
> > 
> > Actually the problem you are facing may be due to
> > https://bugs.chromium.org/p/chromium/issues/detail?id=704782
> 
> Do the green trybots mean this CL would work on Android on the bots?

What was your command? I don't see anything in the log:
https://uberchromegw.corp.google.com/i/tryserver.chromium.perf/builders/andro...

[ehmaldonado_chromium, 2017-03-24 18:40:12]

On 2017/03/24 18:29:08, nednguyen wrote:
> On 2017/03/24 18:21:25, ehmaldonado_chromium wrote:
> > On 2017/03/24 00:37:42, nednguyen wrote:
> > > On 2017/03/23 20:17:59, nednguyen wrote:
> > > > On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > > > > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > > > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > > > > Ned: 
> > > > > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > > > > media.android.tough_video_cases [1] which also uses local
> files
> > > [2].
> > > > > > > > > > Am I missing something, or this approach doesn't work on
> > Android?
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > [1]
> > > > > > > >
> > > >
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > > > > [2]
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > > > > 
> > > > > > > > > How are other webrtc benchmark works on Android?
> > > > > > > > > 
> > > > > > > > > TO see how local file works on Android, you can try to run s.t
> > like:
> > > > > > > > > blink_perf.bindings
> > > > > > > > 
> > > > > > > > I ran:
> > > > > > > > 
> > > > > > > > tools/perf/run_benchmark -v blink_perf.bindings
--device=06df61c3
> > > > > > > > --browser=android-chromium
> > > > > > > > 
> > > > > > > > And I got:
> > > > > > > > 
> > > > > > > > This site can't be reached
> > > > > > > > 127.0.0.1 refused to connect.
> > > > > > > 
> > > > > > > Hmhh, I think that may mean you need to setup your test device.
> Please
> > > > see:
> > > > > > > go/telemetry-device-setup
> > > > > > 
> > > > > > Still the same.
> > > > > 
> > > > > Does it work for you?
> > > > > Is it possible to run it on swarming, to check that it works?
> > > > 
> > > > You can run trybot
> > > > (https://www.chromium.org/developers/telemetry/performance-try-bots),
> though
> > I
> > > > also suggest working with other engineers to make sure that you can run
> > > > telemetry benchmark locally on Android. Otherwise, in the future you
will
> > have
> > > > bad time debugging the failures.
> > > 
> > > Actually the problem you are facing may be due to
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=704782
> > 
> > Do the green trybots mean this CL would work on Android on the bots?
> 
> What was your command? I don't see anything in the log:
>
https://uberchromegw.corp.google.com/i/tryserver.chromium.perf/builders/andro...

Step 20 has nested logs.
See step 20.16:

https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Ftryserver.chromium.perf%2F...

My command was
$ tools/perf/run_benchmark try all webrtc.peerconnection

[nednguyen, 2017-03-24 18:44:27]

On 2017/03/24 18:40:12, ehmaldonado_chromium wrote:
> On 2017/03/24 18:29:08, nednguyen wrote:
> > On 2017/03/24 18:21:25, ehmaldonado_chromium wrote:
> > > On 2017/03/24 00:37:42, nednguyen wrote:
> > > > On 2017/03/23 20:17:59, nednguyen wrote:
> > > > > On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > > > > > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > > > > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > > > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > > > > > Ned: 
> > > > > > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > > > > > media.android.tough_video_cases [1] which also uses local
> > files
> > > > [2].
> > > > > > > > > > > Am I missing something, or this approach doesn't work on
> > > Android?
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > [1]
> > > > > > > > >
> > > > >
> https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > > > > > [2]
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > > > > > 
> > > > > > > > > > How are other webrtc benchmark works on Android?
> > > > > > > > > > 
> > > > > > > > > > TO see how local file works on Android, you can try to run
s.t
> > > like:
> > > > > > > > > > blink_perf.bindings
> > > > > > > > > 
> > > > > > > > > I ran:
> > > > > > > > > 
> > > > > > > > > tools/perf/run_benchmark -v blink_perf.bindings
> --device=06df61c3
> > > > > > > > > --browser=android-chromium
> > > > > > > > > 
> > > > > > > > > And I got:
> > > > > > > > > 
> > > > > > > > > This site can't be reached
> > > > > > > > > 127.0.0.1 refused to connect.
> > > > > > > > 
> > > > > > > > Hmhh, I think that may mean you need to setup your test device.
> > Please
> > > > > see:
> > > > > > > > go/telemetry-device-setup
> > > > > > > 
> > > > > > > Still the same.
> > > > > > 
> > > > > > Does it work for you?
> > > > > > Is it possible to run it on swarming, to check that it works?
> > > > > 
> > > > > You can run trybot
> > > > > (https://www.chromium.org/developers/telemetry/performance-try-bots),
> > though
> > > I
> > > > > also suggest working with other engineers to make sure that you can
run
> > > > > telemetry benchmark locally on Android. Otherwise, in the future you
> will
> > > have
> > > > > bad time debugging the failures.
> > > > 
> > > > Actually the problem you are facing may be due to
> > > > https://bugs.chromium.org/p/chromium/issues/detail?id=704782
> > > 
> > > Do the green trybots mean this CL would work on Android on the bots?
> > 
> > What was your command? I don't see anything in the log:
> >
>
https://uberchromegw.corp.google.com/i/tryserver.chromium.perf/builders/andro...
> 
> Step 20 has nested logs.
> See step 20.16:
> 
>
https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Ftryserver.chromium.perf%2F...
> 
> My command was
> $ tools/perf/run_benchmark try all webrtc.peerconnection

Ah, this log is correct. It seems like this is good to go  then

[ehmaldonado_chromium, 2017-03-24 18:46:00]

On 2017/03/24 18:44:27, nednguyen wrote:
> On 2017/03/24 18:40:12, ehmaldonado_chromium wrote:
> > On 2017/03/24 18:29:08, nednguyen wrote:
> > > On 2017/03/24 18:21:25, ehmaldonado_chromium wrote:
> > > > On 2017/03/24 00:37:42, nednguyen wrote:
> > > > > On 2017/03/23 20:17:59, nednguyen wrote:
> > > > > > On 2017/03/23 20:03:09, ehmaldonado_chromium wrote:
> > > > > > > On 2017/03/23 17:00:17, ehmaldonado_chromium wrote:
> > > > > > > > On 2017/03/23 16:32:01, nednguyen wrote:
> > > > > > > > > On 2017/03/23 16:28:00, ehmaldonado_chromium wrote:
> > > > > > > > > > On 2017/03/23 16:21:33, nednguyen wrote:
> > > > > > > > > > > On 2017/03/23 16:16:51, ehmaldonado_chromium wrote:
> > > > > > > > > > > > Ned: 
> > > > > > > > > > > > This doesn't seem to work for android. And neither does
> > > > > > > > > > > > media.android.tough_video_cases [1] which also uses
local
> > > files
> > > > > [2].
> > > > > > > > > > > > Am I missing something, or this approach doesn't work on
> > > > Android?
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > [1]
> > > > > > > > > >
> > > > > >
> > https://cs.chromium.org/chromium/src/tools/perf/benchmarks/media.py?l=111
> > > > > > > > > > > > [2]
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/tools/perf/page_sets/tough_video_cases.p...
> > > > > > > > > > > 
> > > > > > > > > > > How are other webrtc benchmark works on Android?
> > > > > > > > > > > 
> > > > > > > > > > > TO see how local file works on Android, you can try to run
> s.t
> > > > like:
> > > > > > > > > > > blink_perf.bindings
> > > > > > > > > > 
> > > > > > > > > > I ran:
> > > > > > > > > > 
> > > > > > > > > > tools/perf/run_benchmark -v blink_perf.bindings
> > --device=06df61c3
> > > > > > > > > > --browser=android-chromium
> > > > > > > > > > 
> > > > > > > > > > And I got:
> > > > > > > > > > 
> > > > > > > > > > This site can't be reached
> > > > > > > > > > 127.0.0.1 refused to connect.
> > > > > > > > > 
> > > > > > > > > Hmhh, I think that may mean you need to setup your test
device.
> > > Please
> > > > > > see:
> > > > > > > > > go/telemetry-device-setup
> > > > > > > > 
> > > > > > > > Still the same.
> > > > > > > 
> > > > > > > Does it work for you?
> > > > > > > Is it possible to run it on swarming, to check that it works?
> > > > > > 
> > > > > > You can run trybot
> > > > > >
(https://www.chromium.org/developers/telemetry/performance-try-bots),
> > > though
> > > > I
> > > > > > also suggest working with other engineers to make sure that you can
> run
> > > > > > telemetry benchmark locally on Android. Otherwise, in the future you
> > will
> > > > have
> > > > > > bad time debugging the failures.
> > > > > 
> > > > > Actually the problem you are facing may be due to
> > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=704782
> > > > 
> > > > Do the green trybots mean this CL would work on Android on the bots?
> > > 
> > > What was your command? I don't see anything in the log:
> > >
> >
>
https://uberchromegw.corp.google.com/i/tryserver.chromium.perf/builders/andro...
> > 
> > Step 20 has nested logs.
> > See step 20.16:
> > 
> >
>
https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Ftryserver.chromium.perf%2F...
> > 
> > My command was
> > $ tools/perf/run_benchmark try all webrtc.peerconnection
> 
> Ah, this log is correct. It seems like this is good to go  then

Henrik: Can you take a look at the update script, please?

[kjellander_chromium, 2017-03-24 19:51:25]

Thanks for writing the script, it's exactly what I was having in mind. I just
have a few concerns about it - I think it could be simpler and a little more
future-proof.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
File tools/perf/page_sets/update_webrtc_cases.py (right):

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:41: STRIPPED_TAGS = ['<meta',
'<link', '<script']
Do we really want to remove all <script> tags? It seems risky in the future.

Can't you preserve the directory hierarchy on
https://github.com/webrtc/samples/tree/gh-pages/src instead? Like:
tools/perf/page_sets/js/common.js (and thus you can remove line 29).
tools/perf/page_sets/content/datachannel/datatransfer
tools/perf/page_sets/content/getusermedia/resolution
tools/perf/page_sets/content/peerconnection/constraints
tools/perf/page_sets/content/peerconnection/audio

or is there something I'm missing? 
Removing the <link> tags and the <script> tag for ga.js makes sense, but I'm not
sure about the rest.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:65: contents = [JS_COPYRIGHT_NOTICE]
+ contents[8:]
I guess 8 comes from the WebRTC license header which is currently added. It
seems a bit risky but I'm fine since we'll review each update and that tests
should fail badly if the wrong things are removed.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:75: if any(tag in line for tag in
STRIPPED_TAGS):
This seems to assume each of the stripped tags are located on one line only,
which seems like a risky assumption. Can you use multi-line regexp instead?

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:97: p.wait()
How is the case when git clone fails handled? Does it fail in a way that makes
it easy to figure out what went wrong?

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:113: 
+1 blank line

[nednguyen, 2017-03-24 20:12:37]

I didn't notice your script to update webrtc pages, can you put it in
webrtc_cases/ folder?

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
File tools/perf/page_sets/update_webrtc_cases.py (right):

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:1: # Copyright 2017 The Chromium
Authors. All rights reserved.
Our convention is script has no .py tail. Also please add --help about what this
script does, how to use it.

[ehmaldonado_chromium, 2017-03-24 20:13:53]

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
File tools/perf/page_sets/update_webrtc_cases.py (right):

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:41: STRIPPED_TAGS = ['<meta',
'<link', '<script']
On 2017/03/24 19:51:25, kjellander_chromium wrote:
> Do we really want to remove all <script> tags? It seems risky in the future.
> 
> Can't you preserve the directory hierarchy on
> https://github.com/webrtc/samples/tree/gh-pages/src instead? Like:
> tools/perf/page_sets/js/common.js (and thus you can remove line 29).
> tools/perf/page_sets/content/datachannel/datatransfer
> tools/perf/page_sets/content/getusermedia/resolution
> tools/perf/page_sets/content/peerconnection/constraints
> tools/perf/page_sets/content/peerconnection/audio
> 
> or is there something I'm missing? 
> Removing the <link> tags and the <script> tag for ga.js makes sense, but I'm
not
> sure about the rest.

I tried to preserve the directory structure, but it looks like levels above the
test page are not accessible (so from peerconnection/getusermedia/index.html I
couldn't access adapter.js

You're right. It also makes sense to copy common.js. about the other scripts,
I'm not sure.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:65: contents = [JS_COPYRIGHT_NOTICE]
+ contents[8:]
On 2017/03/24 19:51:25, kjellander_chromium wrote:
> I guess 8 comes from the WebRTC license header which is currently added. It
> seems a bit risky but I'm fine since we'll review each update and that tests
> should fail badly if the wrong things are removed.

I could also try to delete the first (multiline) comment. I don't know if it's
safer, though.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:75: if any(tag in line for tag in
STRIPPED_TAGS):
On 2017/03/24 19:51:25, kjellander_chromium wrote:
> This seems to assume each of the stripped tags are located on one line only,
> which seems like a risky assumption. Can you use multi-line regexp instead?

Will do.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:97: p.wait()
On 2017/03/24 19:51:25, kjellander_chromium wrote:
> How is the case when git clone fails handled? Does it fail in a way that makes
> it easy to figure out what went wrong?

Yes. Git's output is printed to console.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:113: 
On 2017/03/24 19:51:25, kjellander_chromium wrote:
> +1 blank line

Will do.

[ehmaldonado_chromium, 2017-03-24 20:16:15]

On 2017/03/24 20:12:37, nednguyen wrote:
> I didn't notice your script to update webrtc pages, can you put it in
> webrtc_cases/ folder?
> 
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> File tools/perf/page_sets/update_webrtc_cases.py (right):
> 
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> tools/perf/page_sets/update_webrtc_cases.py:1: # Copyright 2017 The Chromium
> Authors. All rights reserved.
> Our convention is script has no .py tail. Also please add --help about what
this
> script does, how to use it.

I tried putting it in the webrtc_cases folder, but it complains that there is no
webrtc_cases module in the presubmit check.

[kjellander_chromium, 2017-03-24 20:18:30]

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
File tools/perf/page_sets/update_webrtc_cases.py (right):

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:41: STRIPPED_TAGS = ['<meta',
'<link', '<script']
On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > Do we really want to remove all <script> tags? It seems risky in the future.
> > 
> > Can't you preserve the directory hierarchy on
> > https://github.com/webrtc/samples/tree/gh-pages/src instead? Like:
> > tools/perf/page_sets/js/common.js (and thus you can remove line 29).
> > tools/perf/page_sets/content/datachannel/datatransfer
> > tools/perf/page_sets/content/getusermedia/resolution
> > tools/perf/page_sets/content/peerconnection/constraints
> > tools/perf/page_sets/content/peerconnection/audio
> > 
> > or is there something I'm missing? 
> > Removing the <link> tags and the <script> tag for ga.js makes sense, but I'm
> not
> > sure about the rest.
> 
> I tried to preserve the directory structure, but it looks like levels above
the
> test page are not accessible (so from peerconnection/getusermedia/index.html I
> couldn't access adapter.js
> 
> You're right. It also makes sense to copy common.js. about the other scripts,
> I'm not sure.

OK, if there's a limitation in how the files are served and so on, I'm fine with
the change directory structure. I was mostly wondering since it seemed simpler
to just preserve it to a higher degree.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:65: contents = [JS_COPYRIGHT_NOTICE]
+ contents[8:]
On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > I guess 8 comes from the WebRTC license header which is currently added. It
> > seems a bit risky but I'm fine since we'll review each update and that tests
> > should fail badly if the wrong things are removed.
> 
> I could also try to delete the first (multiline) comment. I don't know if it's
> safer, though.

I think it should be, but no big deal since this is just run manually +
reviewed.

https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
tools/perf/page_sets/update_webrtc_cases.py:97: p.wait()
On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > How is the case when git clone fails handled? Does it fail in a way that
makes
> > it easy to figure out what went wrong?
> 
> Yes. Git's output is printed to console.

That's fine then, since this will be run interactively.

[ehmaldonado_chromium, 2017-03-27 11:44:39]

Patchset #4 (id:80001) has been deleted

[ehmaldonado_chromium, 2017-03-27 11:45:50]

On 2017/03/24 20:18:30, kjellander_chromium wrote:
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> File tools/perf/page_sets/update_webrtc_cases.py (right):
> 
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> tools/perf/page_sets/update_webrtc_cases.py:41: STRIPPED_TAGS = ['<meta',
> '<link', '<script']
> On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> > On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > > Do we really want to remove all <script> tags? It seems risky in the
future.
> > > 
> > > Can't you preserve the directory hierarchy on
> > > https://github.com/webrtc/samples/tree/gh-pages/src instead? Like:
> > > tools/perf/page_sets/js/common.js (and thus you can remove line 29).
> > > tools/perf/page_sets/content/datachannel/datatransfer
> > > tools/perf/page_sets/content/getusermedia/resolution
> > > tools/perf/page_sets/content/peerconnection/constraints
> > > tools/perf/page_sets/content/peerconnection/audio
> > > 
> > > or is there something I'm missing? 
> > > Removing the <link> tags and the <script> tag for ga.js makes sense, but
I'm
> > not
> > > sure about the rest.
> > 
> > I tried to preserve the directory structure, but it looks like levels above
> the
> > test page are not accessible (so from peerconnection/getusermedia/index.html
I
> > couldn't access adapter.js
> > 
> > You're right. It also makes sense to copy common.js. about the other
scripts,
> > I'm not sure.
> 
> OK, if there's a limitation in how the files are served and so on, I'm fine
with
> the change directory structure. I was mostly wondering since it seemed simpler
> to just preserve it to a higher degree.
> 
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> tools/perf/page_sets/update_webrtc_cases.py:65: contents =
[JS_COPYRIGHT_NOTICE]
> + contents[8:]
> On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> > On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > > I guess 8 comes from the WebRTC license header which is currently added.
It
> > > seems a bit risky but I'm fine since we'll review each update and that
tests
> > > should fail badly if the wrong things are removed.
> > 
> > I could also try to delete the first (multiline) comment. I don't know if
it's
> > safer, though.
> 
> I think it should be, but no big deal since this is just run manually +
> reviewed.
> 
>
https://codereview.chromium.org/2761163003/diff/60001/tools/perf/page_sets/up...
> tools/perf/page_sets/update_webrtc_cases.py:97: p.wait()
> On 2017/03/24 20:13:53, ehmaldonado_chromium wrote:
> > On 2017/03/24 19:51:25, kjellander_chromium wrote:
> > > How is the case when git clone fails handled? Does it fail in a way that
> makes
> > > it easy to figure out what went wrong?
> > 
> > Yes. Git's output is printed to console.
> 
> That's fine then, since this will be run interactively.

PTAL :)

[nednguyen, 2017-03-27 12:21:06]

lgtm

[kjellander_chromium, 2017-03-27 13:20:04]

Thanks for all the work on the update script. 

lgtm with one comment.

https://codereview.chromium.org/2761163003/diff/100001/tools/perf/page_sets/u...
File tools/perf/page_sets/update_webrtc_cases (right):

https://codereview.chromium.org/2761163003/diff/100001/tools/perf/page_sets/u...
tools/perf/page_sets/update_webrtc_cases:117: '  * Add a copyright notice on top
of the HTML and JS files.\n'
Add a line about how the files are renamed:
* index.html -> testname.html
* main.js -> testname.js

[ehmaldonado_chromium, 2017-03-27 14:56:58]

ehmaldonado@chromium.org changed reviewers:
+ dpranke@google.com, phajdan.jr@chromium.org

[ehmaldonado_chromium, 2017-03-27 14:56:59]

Dirk, Phajdan: Can you review the changes in PRESUBMIT.py?
Thanks!

https://codereview.chromium.org/2761163003/diff/100001/tools/perf/page_sets/u...
File tools/perf/page_sets/update_webrtc_cases (right):

https://codereview.chromium.org/2761163003/diff/100001/tools/perf/page_sets/u...
tools/perf/page_sets/update_webrtc_cases:117: '  * Add a copyright notice on top
of the HTML and JS files.\n'
On 2017/03/27 13:20:04, kjellander_chromium wrote:
> Add a line about how the files are renamed:
> * index.html -> testname.html
> * main.js -> testname.js

Done.

[Paweł Hajdan Jr., 2017-03-28 09:29:45]

https://codereview.chromium.org/2761163003/diff/120001/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/2761163003/diff/120001/PRESUBMIT.py#newcode29
PRESUBMIT.py:29:
r"tools[\\\/]perf[\\\/]page_sets[\\\/]webrtc_cases[\\\/]adapter.js$",
Why is this needed?

[ehmaldonado_chromium, 2017-03-28 09:56:36]

PTAL

https://codereview.chromium.org/2761163003/diff/120001/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/2761163003/diff/120001/PRESUBMIT.py#newcode29
PRESUBMIT.py:29:
r"tools[\\\/]perf[\\\/]page_sets[\\\/]webrtc_cases[\\\/]adapter.js$",
On 2017/03/28 09:29:45, Paweł Hajdan Jr. wrote:
> Why is this needed?

It has lines longer than 80 characters. We want to ignore it because those are
test pages, rather than code.

[Paweł Hajdan Jr., 2017-03-28 09:57:35]

LGTM

[ehmaldonado_chromium, 2017-03-28 09:58:49]

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

[commit-bot: I haz the power, 2017-03-28 09:59:01]

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

[commit-bot: I haz the power, 2017-03-28 11:56:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 11:56:25]

Dry run: This issue passed the CQ dry run.

[ehmaldonado_chromium, 2017-03-28 11:57:13]

Description was changed from

==========
Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
R=nednguyen@google.com
==========

to

==========
Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
NOTRY=True
==========

[ehmaldonado_chromium, 2017-03-28 13:11:38]

The CQ bit was checked by ehmaldonado@chromium.org

[ehmaldonado_chromium, 2017-03-28 13:11:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, kjellander@chromium.org
Link to the patchset: https://codereview.chromium.org/2761163003/#ps140001
(title: "Exclude all of webrtc_cases in PRESUBMIT.py and add a comment
explaining it is because these are te…")

[commit-bot: I haz the power, 2017-03-28 13:11:50]

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

[commit-bot: I haz the power, 2017-03-28 13:16:58]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1490706698149360,
"parent_rev": "0990801e0a56421e48fda78935487f8c1e670931", "commit_rev":
"78eee2ed541a59974558681e9165a18a0fe5028b"}

[commit-bot: I haz the power, 2017-03-28 13:17:53]

Description was changed from

==========
Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
NOTRY=True
==========

to

==========
Use local pages for webrtc telemetry tests.

This is to avoid hitting HTTPS errors.

BUG=chromium:702201
NOTRY=True

Review-Url: https://codereview.chromium.org/2761163003
Cr-Commit-Position: refs/heads/master@{#460078}
Committed:
https://chromium.googlesource.com/chromium/src/+/78eee2ed541a59974558681e9165...
==========

[commit-bot: I haz the power, 2017-03-28 13:17:56]

Committed patchset #6 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/78eee2ed541a59974558681e9165...