[System Health] Make pinterest/tumblr stories scroll to elements off screen.

[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
Committed: https://crrev.com/d26f4378989c7cb41cba9b215464c180bd46b7e7
Cr-Commit-Position: refs/heads/master@{#429475}

[rnephew (Reviews Here), 2016-10-14 22:52:52]

rnephew@chromium.org changed reviewers:
+ nednguyen@google.com, petrcermak@chromium.org

[rnephew (Reviews Here), 2016-10-14 22:52:52]

Still need to re-record the pageset, but want to get your approval on the rest
before doing that.

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:320: ITEMS_TO_VISIT = 8
Should I just use the default 15 here?

[petrcermak, 2016-10-17 08:51:37]

LGTM, but please wait for Ned since I'm not on the project any more.

Thanks,
Petr

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:320: ITEMS_TO_VISIT = 8
On 2016/10/14 22:52:52, rnephew (Reviews Here) wrote:
> Should I just use the default 15 here?

Unless it makes the story too long or less stable, I don't see any reason to use
a custom value ;-)

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:328:
action_runner.Wait(1) # To make browsing more realistic.
nit: There should be two spaces before a comment.

[rnephew (Reviews Here), 2016-10-17 18:48:20]

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/browsing_stories.py (right):

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:320: ITEMS_TO_VISIT = 8
On 2016/10/17 08:51:37, petrcermak wrote:
> On 2016/10/14 22:52:52, rnephew (Reviews Here) wrote:
> > Should I just use the default 15 here?
> 
> Unless it makes the story too long or less stable, I don't see any reason to
use
> a custom value ;-)

After some local experimenting it does seem to make it less stable. I will set
it to 8 like the youtube set.

Tumblr will randomly try to open some images as pages and some as popups. Its a
low rate of occurance, but 15 seems to trigger it at least once every run.

The pinterest page crashes during clock sync when its set to 15. It does not
when set to 8 though, so I'm guessing this has something to do with the
length/size of the trace.

https://codereview.chromium.org/2414153004/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:328:
action_runner.Wait(1) # To make browsing more realistic.
On 2016/10/17 08:51:37, petrcermak wrote:
> nit: There should be two spaces before a comment.

Done.

[rnephew (Reviews Here), 2016-10-17 18:49:51]

Patchset #2 (id:20001) has been deleted

[rnephew (Reviews Here), 2016-10-17 18:58:58]

Testing this locally it seems to still be using the old WPR recording. I will
re-record them one at a time and upload them and see if that works.

[nednguyen, 2016-10-17 19:22:41]

Description was changed from

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
==========

to

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
==========

[nednguyen, 2016-10-17 19:22:42]

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

[nednguyen, 2016-10-17 19:24:50]

https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
File tools/perf/page_sets/data/system_health_desktop.json (right):

https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
tools/perf/page_sets/data/system_health_desktop.json:114:
"system_health_desktop_035.wpr": [
Hmhh, this seems odd. Which wpr recording is "browse:media:pinterest" page
using? _034 or _035 wpr?

[nednguyen, 2016-10-17 19:30:44]

https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
File tools/perf/page_sets/data/system_health_desktop.json (right):

https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
tools/perf/page_sets/data/system_health_desktop.json:111:
"browse:media:pinterest",
IIRC, the record_wpr should erase these two entries & replace the
system_health_desktop.json with new mapping between wpr & page. If not, maybe
this is a bug

[rnephew (Reviews Here), 2016-10-17 20:05:50]

On 2016/10/17 19:30:44, nednguyen wrote:
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> File tools/perf/page_sets/data/system_health_desktop.json (right):
> 
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> tools/perf/page_sets/data/system_health_desktop.json:111:
> "browse:media:pinterest",
> IIRC, the record_wpr should erase these two entries & replace the
> system_health_desktop.json with new mapping between wpr & page. If not, maybe
> this is a bug

34 was me trying to be clever and record both of the new pages in one go by
changing their name to record_this_pinterest and record_this_tumblr then
running:
./record_wpr --story-filter=record_this_ desktop_memory_system_health
--pageset-repeat=1 --browser=system --upload
I then manually changed the .json file to have the right names. This didn't seem
to work though, it was still picking up the old recordings.

35 is just pinterest with the real name and 36 is just tumblr with the real
name.

Basically I think with 34 I made some bad assumptions with how WPR works.

[nednguyen, 2016-10-21 00:58:58]

On 2016/10/17 20:05:50, rnephew (Reviews Here) wrote:
> On 2016/10/17 19:30:44, nednguyen wrote:
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > File tools/perf/page_sets/data/system_health_desktop.json (right):
> > 
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > tools/perf/page_sets/data/system_health_desktop.json:111:
> > "browse:media:pinterest",
> > IIRC, the record_wpr should erase these two entries & replace the
> > system_health_desktop.json with new mapping between wpr & page. If not,
maybe
> > this is a bug
> 
> 34 was me trying to be clever and record both of the new pages in one go by
> changing their name to record_this_pinterest and record_this_tumblr then
> running:
> ./record_wpr --story-filter=record_this_ desktop_memory_system_health
> --pageset-repeat=1 --browser=system --upload
> I then manually changed the .json file to have the right names. This didn't
seem
> to work though, it was still picking up the old recordings.

Hmhh, I think this should work. If you want to debug further, you can add print
statement in
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...

> 
> 35 is just pinterest with the real name and 36 is just tumblr with the real
> name.
> 
> Basically I think with 34 I made some bad assumptions with how WPR works.

[rnephew (Reviews Here), 2016-10-24 16:35:12]

On 2016/10/21 00:58:58, nednguyen wrote:
> On 2016/10/17 20:05:50, rnephew (Reviews Here) wrote:
> > On 2016/10/17 19:30:44, nednguyen wrote:
> > >
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > > File tools/perf/page_sets/data/system_health_desktop.json (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > > tools/perf/page_sets/data/system_health_desktop.json:111:
> > > "browse:media:pinterest",
> > > IIRC, the record_wpr should erase these two entries & replace the
> > > system_health_desktop.json with new mapping between wpr & page. If not,
> maybe
> > > this is a bug
> > 
> > 34 was me trying to be clever and record both of the new pages in one go by
> > changing their name to record_this_pinterest and record_this_tumblr then
> > running:
> > ./record_wpr --story-filter=record_this_ desktop_memory_system_health
> > --pageset-repeat=1 --browser=system --upload
> > I then manually changed the .json file to have the right names. This didn't
> seem
> > to work though, it was still picking up the old recordings.
> 
> Hmhh, I think this should work. If you want to debug further, you can add
print
> statement in
>
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...
> 
> > 
> > 35 is just pinterest with the real name and 36 is just tumblr with the real
> > name.
> > 
> > Basically I think with 34 I made some bad assumptions with how WPR works.

After looking at that, and running both locally, its working as I expect.

[nednguyen, 2016-10-24 16:52:16]

On 2016/10/24 16:35:12, rnephew (Reviews Here) wrote:
> On 2016/10/21 00:58:58, nednguyen wrote:
> > On 2016/10/17 20:05:50, rnephew (Reviews Here) wrote:
> > > On 2016/10/17 19:30:44, nednguyen wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > > > File tools/perf/page_sets/data/system_health_desktop.json (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2414153004/diff/100001/tools/perf/page_sets/d...
> > > > tools/perf/page_sets/data/system_health_desktop.json:111:
> > > > "browse:media:pinterest",
> > > > IIRC, the record_wpr should erase these two entries & replace the
> > > > system_health_desktop.json with new mapping between wpr & page. If not,
> > maybe
> > > > this is a bug
> > > 
> > > 34 was me trying to be clever and record both of the new pages in one go
by
> > > changing their name to record_this_pinterest and record_this_tumblr then
> > > running:
> > > ./record_wpr --story-filter=record_this_ desktop_memory_system_health
> > > --pageset-repeat=1 --browser=system --upload
> > > I then manually changed the .json file to have the right names. This
didn't
> > seem
> > > to work though, it was still picking up the old recordings.
> > 
> > Hmhh, I think this should work. If you want to debug further, you can add
> print
> > statement in
> >
>
https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/...
> > 
> > > 
> > > 35 is just pinterest with the real name and 36 is just tumblr with the
real
> > > name.
> > > 
> > > Basically I think with 34 I made some bad assumptions with how WPR works.
> 
> After looking at that, and running both locally, its working as I expect.

So can we remove the "system_health_desktop_034.wpr" entry?

[rnephew (Reviews Here), 2016-10-24 17:03:44]

Patchset #6 (id:120001) has been deleted

[rnephew (Reviews Here), 2016-10-24 17:04:07]

> So can we remove the "system_health_desktop_034.wpr" entry?

Done.

[nednguyen, 2016-10-24 17:05:48]

The CQ bit was checked by nednguyen@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 17:06:27]

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, 2016-10-24 18:14:05]

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

[commit-bot: I haz the power, 2016-10-24 18:14:06]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[rnephew (Reviews Here), 2016-10-24 18:40:09]

Failures are in stories not affected by this CL.

[perezju, 2016-10-25 08:37:43]

lgtm

I ran the stories, and they do work as expected.

[perezju, 2016-10-25 08:38:15]

(but you still need Ned's stamp)

[nednguyen, 2016-10-28 00:36:51]

lgtm

[rnephew (Reviews Here), 2016-11-02 18:42:11]

On 2016/10/28 00:36:51, nednguyen wrote:
> lgtm

Sorry, these fell off my radar a little bit. I'm rebasing now and attempting to
commit.

[rnephew (Reviews Here), 2016-11-02 18:47:40]

The CQ bit was checked by rnephew@chromium.org

[rnephew (Reviews Here), 2016-11-02 18:47:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org, perezju@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2414153004/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-02 18:48:22]

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, 2016-11-02 19:49:43]

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

[commit-bot: I haz the power, 2016-11-02 19:49:44]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rnephew (Reviews Here), 2016-11-02 20:11:38]

On 2016/11/02 19:49:44, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

benchmarks.system_health_smoke_test.SystemHealthBenchmarkSmokeTest.system_health.memory_desktop.browse:media:youtube
failed. Unrelated to my CL. Trying again.

[rnephew (Reviews Here), 2016-11-02 20:11:48]

The CQ bit was checked by rnephew@chromium.org

[commit-bot: I haz the power, 2016-11-02 20:12:21]

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, 2016-11-02 21:21:48]

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

[commit-bot: I haz the power, 2016-11-02 21:21:49]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[nednguyen, 2016-11-02 21:25:04]

On 2016/11/02 21:21:49, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

Let disable "browse:media:youtube" with a bug filed to unblock landing this.

[rnephew (Reviews Here), 2016-11-02 21:39:22]

On 2016/11/02 21:25:04, nednguyen wrote:
> On 2016/11/02 21:21:49, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
> 
> Let disable "browse:media:youtube" with a bug filed to unblock landing this.

Done, PTAL.

[rnephew (Reviews Here), 2016-11-02 22:07:09]

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

[commit-bot: I haz the power, 2016-11-02 22:08:05]

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

[nednguyen, 2016-11-02 22:08:44]

lgtm

[commit-bot: I haz the power, 2016-11-02 23:25:37]

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

[commit-bot: I haz the power, 2016-11-02 23:25:38]

Dry run: This issue passed the CQ dry run.

[rnephew (Reviews Here), 2016-11-03 00:22:31]

The CQ bit was checked by rnephew@chromium.org

[rnephew (Reviews Here), 2016-11-03 00:22:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
perezju@chromium.org, petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/2414153004/#ps180001
(title: "Disable youtube browsing story")

[commit-bot: I haz the power, 2016-11-03 00:23:14]

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, 2016-11-03 00:31:02]

Description was changed from

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
==========

to

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
==========

[commit-bot: I haz the power, 2016-11-03 00:31:05]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2016-11-03 00:33:13]

Description was changed from

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392
==========

to

==========
[System Health] Make pinterest/tumblr stories scroll to elements off screen.

The system health stories for these pages require scrolling down to interact
with different elements that start off screen.

BUG=646392, 649392

Committed: https://crrev.com/d26f4378989c7cb41cba9b215464c180bd46b7e7
Cr-Commit-Position: refs/heads/master@{#429475}
==========

[commit-bot: I haz the power, 2016-11-03 00:33:15]

Patchset 8 (id:??) landed as
https://crrev.com/d26f4378989c7cb41cba9b215464c180bd46b7e7
Cr-Commit-Position: refs/heads/master@{#429475}