Wait for test script to be loaded before calling __get_timings. See detailed analysis in crbug.co...

Wait for test script to be loaded before calling __get_timings by setting a variable to indicate that the result report page is loaded. See detailed analysis in crbug.com/53140.

BUG=53140
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=101718

[hans, 2011-09-15 12:48:00]

This looks fine to me, but I'm not familiar with this framework so the other
reviewers should take a look too.

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:143: bool
WaitUntilTabURLPathEndWith(TabProxy* tab,
nit: it should probably be named "...EndsWith()"

[Paweł Hajdan Jr., 2011-09-15 17:59:34]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
Polling is _unacceptable_ . Please wait for an event.

[cmp, 2011-09-15 23:22:52]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
I don't think it's a big deal, really.  If you really wanted to avoid the
polling, you could just set a _navigated_to_report variable in the page cycler
data and then call WaitUntilCookieValue to wait for that value to be set.  Let's
make that change, and I can point you at where to make the data change (or I can
make it on your behalf).

[Johnny(Jianning) Ding, 2011-09-16 00:14:09]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
Thanks Chase! I have sent a patch to make the data change, please review it.

On 2011/09/15 23:22:52, cmp wrote:
> I don't think it's a big deal, really.  If you really wanted to avoid the
> polling, you could just set a _navigated_to_report variable in the page cycler
> data and then call WaitUntilCookieValue to wait for that value to be set. 
Let's
> make that change, and I can point you at where to make the data change (or I
can
> make it on your behalf).

[cmp, 2011-09-16 00:18:22]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
Reviewed. :)

[Johnny(Jianning) Ding, 2011-09-16 00:48:01]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
Thanks Chase! Just landed that patch. Base on that, I have updated patch v2,
please take a look.
Thanks again.
On 2011/09/16 00:18:22, cmp wrote:
> Reviewed. :)

[Johnny(Jianning) Ding, 2011-09-16 00:55:05]

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
File chrome/test/perf/page_cycler_test.cc (right):

http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
chrome/test/perf/page_cycler_test.cc:153:
base::PlatformThread::Sleep(automation::kSleepTime);
Please hold on the review. I need to change the patch. Sorry for the
inconvenience.

On 2011/09/16 00:48:01, Johnny(Jianning) Ding wrote:
> Thanks Chase! Just landed that patch. Base on that, I have updated patch v2,
> please take a look.
> Thanks again.
> On 2011/09/16 00:18:22, cmp wrote:
> > Reviewed. :)
>

[Johnny(Jianning) Ding, 2011-09-16 01:38:25]

Patch updated. Please reexamine it.

On 2011/09/16 00:55:05, Johnny(Jianning) Ding wrote:
>
http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
> File chrome/test/perf/page_cycler_test.cc (right):
> 
>
http://codereview.chromium.org/7904005/diff/1/chrome/test/perf/page_cycler_te...
> chrome/test/perf/page_cycler_test.cc:153:
> base::PlatformThread::Sleep(automation::kSleepTime);
> Please hold on the review. I need to change the patch. Sorry for the
> inconvenience.
> 
> On 2011/09/16 00:48:01, Johnny(Jianning) Ding wrote:
> > Thanks Chase! Just landed that patch. Base on that, I have updated patch v2,
> > please take a look.
> > Thanks again.
> > On 2011/09/16 00:18:22, cmp wrote:
> > > Reviewed. :)
> >

[Paweł Hajdan Jr., 2011-09-16 16:18:52]

LGTM