[Telemetry] Re-add API to tear down state after each story run

[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this new API
and restricts them from using startup_url.

BUG=chromium:594952
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/2951114f88d4ec6d9ed53527f73baa302ebcae12

[Zhen Wang, 2016-03-16 18:01:16]

Description was changed from

==========
[Telemetry] Re-add API to tear down state after each story run

BUG=594952
BUG=chromium:594952
==========

to

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change the how PageTest benchmarks restart 
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this and
restricts them from using startup_url.

BUG=chromium:594952
==========

[Zhen Wang, 2016-03-16 18:01:54]

Description was changed from

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change the how PageTest benchmarks restart 
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this and
restricts them from using startup_url.

BUG=chromium:594952
==========

to

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this and
restricts them from using startup_url.

BUG=chromium:594952
==========

[Zhen Wang, 2016-03-16 18:02:29]

Description was changed from

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this and
restricts them from using startup_url.

BUG=chromium:594952
==========

to

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this new API
and restricts them from using startup_url.

BUG=chromium:594952
==========

[Zhen Wang, 2016-03-16 18:06:02]

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

[Zhen Wang, 2016-03-16 18:06:02]

ptal

[nednguyen, 2016-03-16 18:34:22]

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
File telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
telemetry/telemetry/page/shared_page_state.py:232: if
self._test.RestartBrowserBeforeEachPage() or page.startup_url:
we need to add "if self.browser and ..<the rest>.." here, otherwise on the first
run, we don't restart browser but these statements inside the block do get
called.

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
telemetry/telemetry/page/shared_page_state.py:233: if
self.platform.tracing_controller:
all platform has tracing_controller, so this check is not needed

[Zhen Wang, 2016-03-16 19:29:13]

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
File telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
telemetry/telemetry/page/shared_page_state.py:232: if
self._test.RestartBrowserBeforeEachPage() or page.startup_url:
On 2016/03/16 18:34:21, nednguyen wrote:
> we need to add "if self.browser and ..<the rest>.." here, otherwise on the
first
> run, we don't restart browser but these statements inside the block do get
> called.

Oh, right. Done.

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
telemetry/telemetry/page/shared_page_state.py:233: if
self.platform.tracing_controller:
On 2016/03/16 18:34:21, nednguyen wrote:
> all platform has tracing_controller, so this check is not needed

I added this check for testBrowserStartupURLSetCorrectly:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...

Should I just set it in the test?

[Zhen Wang, 2016-03-16 19:47:05]

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
File telemetry/telemetry/page/shared_page_state.py (right):

https://codereview.chromium.org/1806023002/diff/1/telemetry/telemetry/page/sh...
telemetry/telemetry/page/shared_page_state.py:233: if
self.platform.tracing_controller:
On 2016/03/16 18:34:21, nednguyen wrote:
> all platform has tracing_controller, so this check is not needed

Removed the check

[nednguyen, 2016-03-16 19:56:05]

lgtm

[Zhen Wang, 2016-03-16 20:48:56]

Fixed the tests. Ned, can you take another look?

[nednguyen, 2016-03-16 21:13:33]

lgtm

[nednguyen, 2016-03-16 21:14:58]

https://codereview.chromium.org/1806023002/diff/60001/telemetry/telemetry/pag...
File telemetry/telemetry/page/page_run_end_to_end_unittest.py (right):

https://codereview.chromium.org/1806023002/diff/60001/telemetry/telemetry/pag...
telemetry/telemetry/page/page_run_end_to_end_unittest.py:417: # 1. browser
restarts after page 1 run
So basically we have 1 less browser close call because the first call no longer
happens due to "self._browser " check?

[Zhen Wang, 2016-03-16 21:27:23]

https://codereview.chromium.org/1806023002/diff/60001/telemetry/telemetry/pag...
File telemetry/telemetry/page/page_run_end_to_end_unittest.py (right):

https://codereview.chromium.org/1806023002/diff/60001/telemetry/telemetry/pag...
telemetry/telemetry/page/page_run_end_to_end_unittest.py:417: # 1. browser
restarts after page 1 run
On 2016/03/16 21:14:57, nednguyen wrote:
> So basically we have 1 less browser close call because the first call no
longer
> happens due to "self._browser " check?

Yes

[Zhen Wang, 2016-03-16 21:27:28]

The CQ bit was checked by zhenw@chromium.org

[commit-bot: I haz the power, 2016-03-16 21:27:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1806023002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1806023002/60001

[commit-bot: I haz the power, 2016-03-16 21:43:05]

Description was changed from

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this new API
and restricts them from using startup_url.

BUG=chromium:594952
==========

to

==========
[Telemetry] Re-add API to tear down state after each story run

This CL re-adds the API to tear down state after each story 
run for TBM benchmarks. This API was introduced in 
https://crrev.com/1417133002. That CL was reverted because 
it tried to change how PageTest benchmarks restart the
browser. Some of them (startup) require sharing profile 
states, which should not be tear down after each story run.

This CL does not touch how PageTest benchmarks restart the
browser. It just allows TBM benchmarks to use this new API
and restricts them from using startup_url.

BUG=chromium:594952

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-03-16 21:43:06]

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