Change profile_creator to not subclass page_test

Change profile_creator to not subclass page_test.

No intended behavioral change. This CL is purely a refactor.

Previously, common logic for profile generation was held in
profile_generator.py, and subclasses of profile_creator were merely different
page_tests. This structure has two problems:

- It relies on the assumption that the logic for generating profiles is
relatively similar, modulo some differences in page navigation. There are
currently only 2 subclasses of profile_creator, and they are already diverging
in behavior. I will soon be adding more.

- It doesn't allow for composition of profile generation. In the near future,
there will be a profile creator that takes an existing profile, and adds 25
extensions to it. Another profile creator will take that profile, and add 100
bookmarks. Another one will then perform 1000 navigations.

BUG=444230, 442546
Committed: https://crrev.com/8c2882777864cfbe31941bb9c610c4f2f97b3c9f
Cr-Commit-Position: refs/heads/master@{#310856}

[erikchen, 2014-12-23 22:33:30]

Patchset #1 (id:1) has been deleted

[erikchen, 2014-12-23 22:36:07]

erikchen@chromium.org changed reviewers:
+ dtu@chromium.org

[erikchen, 2014-12-23 22:36:08]

dtu: Please review.

I considered moving profile_creator.py and profile_generator.py out of
tools/telemetry/telemetry/page entirely (and into tools/perf/profile_creators)
but I didn't understand why they were in tools/telemetry/telemetry/page to begin
with. I left them there, on the assumption that I am missing information about
the intended structuring of the telemetry code.

[dtu, 2015-01-09 01:23:48]

On 2014/12/23 22:36:08, erikchen wrote:
> dtu: Please review.
> 
> I considered moving profile_creator.py and profile_generator.py out of
> tools/telemetry/telemetry/page entirely (and into tools/perf/profile_creators)
> but I didn't understand why they were in tools/telemetry/telemetry/page to
begin
> with. I left them there, on the assumption that I am missing information about
> the intended structuring of the telemetry code.

I think in general this is a good idea, it's similar to how Benchmarks wrap
PageTests to become something that's Runnable and composable.

I talked with Tony, and we both think profile_creator.py and
profile_generator.py can move into tools/perf/. I guess originally we/the code
author were more opinionated about what a profile generator does, so the common
code went into telemetry/, but here we're looking to generalize it. That also
resolves some of the dependency issues, like referencing perf/page_sets/ from
telemetry/.

[dtu, 2015-01-09 01:23:58]

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
File tools/perf/profile_creators/extensions_profile_creator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
tools/perf/profile_creators/extensions_profile_creator.py:90: class
PageTest(page_test.PageTest):
I'm finding the nested classes hard to read. Can you move it to the top-level
and precede the name with an underscore?

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
tools/perf/profile_creators/extensions_profile_creator.py:145: self._page_test =
ExtensionsProfileCreator.PageTest()
Why create a side-effect on the class if you're using this entirely within
Run()?

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/profile_creator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_creator.py:11: pass
not needed

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_creator.py:28: pass
This is an abstract class, right? raise NotImplementedError()

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/profile_generator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_generator.py:81: return 1
I prefer re-raising the exception here, so we get the exception type and stack.
In general it's bad to catch Exception unless you re-raise, as you can catch
things like SyntaxError.

[erikchen, 2015-01-09 03:01:37]

On 2015/01/09 01:23:48, dtu wrote:
> On 2014/12/23 22:36:08, erikchen wrote:
> > dtu: Please review.
> > 
> > I considered moving profile_creator.py and profile_generator.py out of
> > tools/telemetry/telemetry/page entirely (and into
tools/perf/profile_creators)
> > but I didn't understand why they were in tools/telemetry/telemetry/page to
> begin
> > with. I left them there, on the assumption that I am missing information
about
> > the intended structuring of the telemetry code.
> 
> I think in general this is a good idea, it's similar to how Benchmarks wrap
> PageTests to become something that's Runnable and composable.
> 
> I talked with Tony, and we both think profile_creator.py and
> profile_generator.py can move into tools/perf/. I guess originally we/the code
> author were more opinionated about what a profile generator does, so the
common
> code went into telemetry/, but here we're looking to generalize it. That also
> resolves some of the dependency issues, like referencing perf/page_sets/ from
> telemetry/.

I filed a bug against myself to write up a follow-up change to move the files.
https://code.google.com/p/chromium/issues/detail?id=447457

[erikchen, 2015-01-09 03:09:22]

Patchset #2 (id:40001) has been deleted

[erikchen, 2015-01-09 03:12:42]

dtu: PTAL

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
File tools/perf/profile_creators/extensions_profile_creator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
tools/perf/profile_creators/extensions_profile_creator.py:90: class
PageTest(page_test.PageTest):
On 2015/01/09 01:23:58, dtu wrote:
> I'm finding the nested classes hard to read. Can you move it to the top-level
> and precede the name with an underscore?

Done. I renamed it to _ExtensionPageTest just to make it a little more
differentiable from PageTest.

https://codereview.chromium.org/825703003/diff/20001/tools/perf/profile_creat...
tools/perf/profile_creators/extensions_profile_creator.py:145: self._page_test =
ExtensionsProfileCreator.PageTest()
On 2015/01/09 01:23:58, dtu wrote:
> Why create a side-effect on the class if you're using this entirely within
> Run()?

I removed the member, and made it a local variable.

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/profile_creator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_creator.py:11: pass
On 2015/01/09 01:23:58, dtu wrote:
> not needed

Removed.

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_creator.py:28: pass
On 2015/01/09 01:23:58, dtu wrote:
> This is an abstract class, right? raise NotImplementedError()

Done.

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/profile_generator.py (right):

https://codereview.chromium.org/825703003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/profile_generator.py:81: return 1
On 2015/01/09 01:23:58, dtu wrote:
> I prefer re-raising the exception here, so we get the exception type and
stack.
> In general it's bad to catch Exception unless you re-raise, as you can catch
> things like SyntaxError.

Sounds reasonable to me. Done. I re-raised the exception, and got rid of the
return statement, on the premise that returning a non-zero exit value is
strictly less useful than having a top-level uncaught exception.

[dtu, 2015-01-09 18:46:34]

lgtm

[erikchen, 2015-01-09 18:48:50]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-01-09 18:50:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/825703003/80001

[commit-bot: I haz the power, 2015-01-09 18:55:10]

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

[commit-bot: I haz the power, 2015-01-09 18:55:11]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[erikchen, 2015-01-09 19:12:44]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-01-09 19:14:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/825703003/100001

[commit-bot: I haz the power, 2015-01-09 21:16:09]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-01-09 21:17:28]

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