Kill page_test.NavigateToPage

Kill page_test.NavigateToPage

BUG=455391, 470147
Committed: https://crrev.com/60c3db53394bdba955667eaecc312695f65c8766
Cr-Commit-Position: refs/heads/master@{#330623}

[nednguyen, 2015-05-18 22:41:30]

Patchset #1 (id:1) has been deleted

[nednguyen, 2015-05-18 22:42:03]

nednguyen@google.com changed reviewers:
+ bolian@chromium.org, sullivan@chromium.org

[sullivan, 2015-05-19 13:57:38]

lgtm

[nednguyen, 2015-05-19 14:52:02]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2015-05-19 14:52:35]

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

[nednguyen, 2015-05-19 14:52:58]

The CQ bit was unchecked by nednguyen@google.com

[nednguyen, 2015-05-19 14:54:55]

nednguyen@google.com changed reviewers:
+ megjablon@chromium.org
- bolian@chromium.org

[nednguyen, 2015-05-19 16:34:35]

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

[nednguyen, 2015-05-19 16:39:16]

On 2015/05/19 14:52:35, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1135173007/20001

@chrome_proxy OWNERs, can you please take a look? Thanks!

[bengr, 2015-05-19 19:12:19]

bengr@chromium.org changed reviewers:
+ sclittle@chromium.org
- bengr@chromium.org

[sclittle, 2015-05-19 19:58:57]

Have you tried running the safebrowsing_on test, to ensure that it still works?

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/comm...
File tools/chrome_proxy/common/chrome_proxy_measurements.py (right):

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/comm...
tools/chrome_proxy/common/chrome_proxy_measurements.py:23: self._expect_timeout
= False
If you're moving |_expect_timeout| to the Page, then please remove it from here.

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/comm...
tools/chrome_proxy/common/chrome_proxy_measurements.py:53: if
self._expect_timeout:
This is syntactically incorrect without the "try", and the method would have
already returned by this point (see the return statements above).

What are you trying to do here?

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/inte...
File tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py (right):

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/inte...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:93: def
CreateUserStorySet(self, options):
Why is this benchmark being changed to not use |page_set|, but the other
benchmarks aren't?

[nednguyen, 2015-05-19 20:18:01]

I run "chrome_proxy_benchmark.safebrowsing_off.safebrowsing " &
chrome_proxy_benchmark.safebrowsing_on.safebrowsing  benchmarks to make sure
both of them work.

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/comm...
File tools/chrome_proxy/common/chrome_proxy_measurements.py (right):

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/comm...
tools/chrome_proxy/common/chrome_proxy_measurements.py:53: if
self._expect_timeout:
On 2015/05/19 19:58:57, sclittle wrote:
> This is syntactically incorrect without the "try", and the method would have
> already returned by this point (see the return statements above).
> 
> What are you trying to do here?

Err, silly me. I am about to remove the whole RunNavigateSteps method.

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/inte...
File tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py (right):

https://codereview.chromium.org/1135173007/diff/40001/tools/chrome_proxy/inte...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:93: def
CreateUserStorySet(self, options):
On 2015/05/19 19:58:57, sclittle wrote:
> Why is this benchmark being changed to not use |page_set|, but the other
> benchmarks aren't?

Because we need to instantiate the pagesets.SafebrowsingPageSet. By default,
telemetry benchmark just directly construct the page_set object.

[sclittle, 2015-05-19 20:28:05]

LGTM, thanks for the clarification.

If you've run the tests and they pass, then I think this should be fine.

[nednguyen, 2015-05-19 20:49:56]

The CQ bit was checked by nednguyen@google.com

[nednguyen, 2015-05-19 20:49:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
sullivan@chromium.org
Link to the patchset: https://codereview.chromium.org/1135173007/#ps60001
(title: "Address sclittle's comment")

[commit-bot: I haz the power, 2015-05-19 20:51:00]

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

[commit-bot: I haz the power, 2015-05-19 21:54:16]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-05-19 21:55:21]

Patchset 3 (id:??) landed as
https://crrev.com/60c3db53394bdba955667eaecc312695f65c8766
Cr-Commit-Position: refs/heads/master@{#330623}