[System Health] Tidy up stories for Chrome's special pages

[System Health] Tidy up stories for Chrome's special pages

We now keep all of:

- load:chrome:blank
- browse:chrome:newtab
- browse:chrome:omnibox

on the same chrome_stories.py

Also move and rename search:portal:google to replace the older
browse:search:google.

BUG=725094
Review-Url: https://codereview.chromium.org/2902513002
Cr-Commit-Position: refs/heads/master@{#473891}
Committed: https://chromium.googlesource.com/chromium/src/+/d57a7acae403b7436284e6f15fe85786eb657e54

[perezju, 2017-05-22 15:50:25]

PTAL.

I kept changes to the minimum, mainly moving around code between files.

If all looks fine I'll re-record and upload "browse:search:google" (was
"search:portal:google"). I guess that's easier than trying to patch the wpr
files with the story name change?

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

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:251: class
GoogleDesktopStory(_ArticleBrowsingStory):
This story now removed. Replaced with the old "search:portal:google" which works
on all platforms and is not disabled on any.

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/chrome_stories.py (left):

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/chrome_stories.py:15: class
SearchGoogleStory(system_health_story.SystemHealthStory):
This story now moved to browsing_stories.py

[perezju, 2017-05-22 15:50:41]

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

[perezju, 2017-05-22 15:50:41]

PTAL.

I kept changes to the minimum, mainly moving around code between files.

If all looks fine I'll re-record and upload "browse:search:google" (was
"search:portal:google"). I guess that's easier than trying to patch the wpr
files with the story name change?

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

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:251: class
GoogleDesktopStory(_ArticleBrowsingStory):
This story now removed. Replaced with the old "search:portal:google" which works
on all platforms and is not disabled on any.

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
File tools/perf/page_sets/system_health/chrome_stories.py (left):

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/chrome_stories.py:15: class
SearchGoogleStory(system_health_story.SystemHealthStory):
This story now moved to browsing_stories.py

[perezju, 2017-05-22 16:06:08]

On 2017/05/22 15:50:41, perezju wrote:
> PTAL.
> 
> I kept changes to the minimum, mainly moving around code between files.
> 
> If all looks fine I'll re-record and upload "browse:search:google" (was
> "search:portal:google"). I guess that's easier than trying to patch the wpr
> files with the story name change?
> 
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> File tools/perf/page_sets/system_health/browsing_stories.py (left):
> 
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> tools/perf/page_sets/system_health/browsing_stories.py:251: class
> GoogleDesktopStory(_ArticleBrowsingStory):
> This story now removed. Replaced with the old "search:portal:google" which
works
> on all platforms and is not disabled on any.
> 
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> File tools/perf/page_sets/system_health/chrome_stories.py (left):
> 
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> tools/perf/page_sets/system_health/chrome_stories.py:15: class
> SearchGoogleStory(system_health_story.SystemHealthStory):
> This story now moved to browsing_stories.py

Btw, how does per-platform recording work? Do I need to record for mobile and
desktop separately?

[nednguyen, 2017-05-22 19:52:35]

On 2017/05/22 16:06:08, perezju wrote:
> On 2017/05/22 15:50:41, perezju wrote:
> > PTAL.
> > 
> > I kept changes to the minimum, mainly moving around code between files.
> > 
> > If all looks fine I'll re-record and upload "browse:search:google" (was
> > "search:portal:google"). I guess that's easier than trying to patch the wpr
> > files with the story name change?
> > 
> >
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> > File tools/perf/page_sets/system_health/browsing_stories.py (left):
> > 
> >
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> > tools/perf/page_sets/system_health/browsing_stories.py:251: class
> > GoogleDesktopStory(_ArticleBrowsingStory):
> > This story now removed. Replaced with the old "search:portal:google" which
> works
> > on all platforms and is not disabled on any.
> > 
> >
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> > File tools/perf/page_sets/system_health/chrome_stories.py (left):
> > 
> >
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> > tools/perf/page_sets/system_health/chrome_stories.py:15: class
> > SearchGoogleStory(system_health_story.SystemHealthStory):
> > This story now moved to browsing_stories.py
> 
> Btw, how does per-platform recording work? Do I need to record for mobile and
> desktop separately?

Yes, you need to record for mobile & desktop separately.

[nednguyen, 2017-05-22 19:52:40]

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

https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
tools/perf/page_sets/system_health/browsing_stories.py:251: class
GoogleDesktopStory(_ArticleBrowsingStory):
On 2017/05/22 15:50:25, perezju wrote:
> This story now removed. Replaced with the old "search:portal:google" which
works
> on all platforms and is not disabled on any.

Why removing this instead of removing the old "search:portal:google" ? This is
added after the old "search:portal:google"  & is more complex (since it include
the back nav).

[perezju, 2017-05-23 09:28:53]

On 2017/05/22 19:52:40, nednguyen wrote:
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> File tools/perf/page_sets/system_health/browsing_stories.py (left):
> 
>
https://codereview.chromium.org/2902513002/diff/1/tools/perf/page_sets/system...
> tools/perf/page_sets/system_health/browsing_stories.py:251: class
> GoogleDesktopStory(_ArticleBrowsingStory):
> On 2017/05/22 15:50:25, perezju wrote:
> > This story now removed. Replaced with the old "search:portal:google" which
> works
> > on all platforms and is not disabled on any.
> 
> Why removing this instead of removing the old "search:portal:google" ? This is
> added after the old "search:portal:google"  & is more complex (since it
include
> the back nav).

You are right. Not sure why I thought it was the other way around. I'll keep
the current browse:search:google and see if I can also record it for mobile.

[nednguyen, 2017-05-23 13:06:51]

lgtm

[perezju, 2017-05-23 13:14:18]

On 2017/05/23 13:06:51, nednguyen wrote:
> lgtm

I'm having problems trying to record on mobile. The flow goes:

  google -> (click link) -> wikipedia -> (navigate back) -> about:blank (???)

Not sure why navigating back makes it go all the way to about:blank. Have you
seen
this issue before?

I'm thinking to land this as-is (no new recordings needed), and try again on a
follow up CL. What do you think?

[nednguyen, 2017-05-23 13:24:51]

On 2017/05/23 13:14:18, perezju wrote:
> On 2017/05/23 13:06:51, nednguyen wrote:
> > lgtm
> 
> I'm having problems trying to record on mobile. The flow goes:
> 
>   google -> (click link) -> wikipedia -> (navigate back) -> about:blank (???)
> 
> Not sure why navigating back makes it go all the way to about:blank. Have you
> seen
> this issue before?
> 
> I'm thinking to land this as-is (no new recordings needed), and try again on a
> follow up CL. What do you think?

+1

[perezju, 2017-05-23 13:27:56]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2017-05-23 13:28:19]

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-05-23 14:45:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495546076972420,
"parent_rev": "1dc490d564adebf5ed17d275ac5654eb9d63606e", "commit_rev":
"d57a7acae403b7436284e6f15fe85786eb657e54"}

[commit-bot: I haz the power, 2017-05-23 14:45:13]

Description was changed from

==========
[System Health] Tidy up stories for Chrome's special pages

We now keep all of:

- load:chrome:blank
- browse:chrome:newtab
- browse:chrome:omnibox

on the same chrome_stories.py

Also move and rename search:portal:google to replace the older
browse:search:google.

BUG=725094
==========

to

==========
[System Health] Tidy up stories for Chrome's special pages

We now keep all of:

- load:chrome:blank
- browse:chrome:newtab
- browse:chrome:omnibox

on the same chrome_stories.py

Also move and rename search:portal:google to replace the older
browse:search:google.

BUG=725094

Review-Url: https://codereview.chromium.org/2902513002
Cr-Commit-Position: refs/heads/master@{#473891}
Committed:
https://chromium.googlesource.com/chromium/src/+/d57a7acae403b7436284e6f15fe8...
==========

[commit-bot: I haz the power, 2017-05-23 14:45:15]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d57a7acae403b7436284e6f15fe8...