[telemetry] Simplify the way PageRunner is called.

[telemetry] Simplify the way PageRunner is called.

Before:
results = page_test_results.PageTestResults()
with page_runner.PageRunner(ps):
  possible_browser = browser_finder.FindBrowser(options)
  runner.Run(options, possible_browser, test, results)

After:
results = page_runner.Run(test, ps, options)

Turns out, PageRunner keeps very little state on its own, and stores most of its state in RunState. Using a with statement does nothing because PageRunner.Close() just passes. Therefore, we can get rid of the PageRunner object and just have page_runner.Run() directly. The only exception was has_called_will_run_page_set, which I solved by getting rid of WillRunPageSet() :(

We can also move a lot of its setup methods to RunState, since those methods delve pretty deeply into RunState anyway. There's more work that could be done here in the future.

Finally, move the results object and possible_browser creation from page_test_runner to page_runner/page_test. This will make it easier for the future test_runner to call page_runner directly. The separation of concerns is that test_runner/page_test_runner only deal with parsing the command line, and page_runner does everything else. In the future, page_test_runner can be obsoleted.


BUG=237412
TEST=./run_tests, smoothness_measurement, and page_cycler with no failures.
R=nduca@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=206529

[dtu, 2013-06-11 01:16:08]

WIP. tonyg, what do you think? I'm having trouble with the existence of
has_called_will_run_page_set, since it doesn't fit in either RunState or
PageState.

[tonyg, 2013-06-11 16:14:29]

On 2013/06/11 01:16:08, Dave Tu wrote:
> WIP. tonyg, what do you think? I'm having trouble with the existence of
> has_called_will_run_page_set, since it doesn't fit in either RunState or
> PageState.

Looks like a very positive refactor to me. I'm not sure what exactly to do about
has_called_will_run_page_set. It certainly looks better in RunState than
PageState. Let me know when this is ready for a closer review.

[dtu, 2013-06-12 00:55:44]

PTAL

[nduca, 2013-06-14 23:03:57]

Lets rename WillRunPageSet to be exactly what it does: DidReconfigureHTTPServer.
Then LGTM.

[dtu, 2013-06-14 23:36:05]

Hey marja, I merged one of your changes. Can you make sure it's right? :P

[dtu, 2013-06-15 00:43:09]

Committed patchset #5 manually as r206529 (presubmit successful).

[marja, 2013-06-18 07:49:59]

the wpr appending behavior lgtm (sadly, there's no test for it, it's kinda
tricky to record anything in the unit tests when we shouldn't anyway be running
against real web sites)

[marja, 2013-06-18 13:06:00]

Hmmh, this broke record_wpr. I'll revert this because fixing it is non-trivial
(even with the API change, do I need to hack the options.output_format myself or
is there some getter funct that will create the right type of options for me...)

[marja, 2013-06-18 13:10:44]

On 2013/06/18 13:06:00, marja wrote:
> Hmmh, this broke record_wpr. I'll revert this because fixing it is non-trivial
> (even with the API change, do I need to hack the options.output_format myself
or
> is there some getter funct that will create the right type of options for
me...)

I take this back. It doesn't revert cleanly. dtu@, what's the right fix for
record_wpr?