Remove 'navigate' attribute from wait_until

Remove 'navigate' attribute from wait_until.
Change PerformActionAndWaitForNavigate to WaitForNavigate.

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

[tonyg, 2014-06-05 16:54:23]

https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/backends/chrome/inspector_page.py (left):

https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome/inspector_page.py:82:
action_function()
I like the cleanup in this patch, but IIRC, performing the action here reduces
flake.

Perhaps the code has changed since I remember, but my recollection was that if
we perform the action that causes the navigation as a separate action prior to
this then there's a chance the action will complete prior to when we begin
listening for the Page.frameNavigated event. In that case, we'd just wait
forever for an event that already came.

Can you confirm whether that's still relevant?

[nednguyen, 2014-06-05 17:00:42]

https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/backends/chrome/inspector_page.py (left):

https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome/inspector_page.py:82:
action_function()
On 2014/06/05 16:54:24, tonyg wrote:
> I like the cleanup in this patch, but IIRC, performing the action here reduces
> flake.
> 
> Perhaps the code has changed since I remember, but my recollection was that if
> we perform the action that causes the navigation as a separate action prior to
> this then there's a chance the action will complete prior to when we begin
> listening for the Page.frameNavigated event. In that case, we'd just wait
> forever for an event that already came.
> 
> Can you confirm whether that's still relevant?
I think the code path should be almost the same. If we rely on the tiny
different of the telemetry code path to avoid flake, I think we were doing it
wrong.

[dtu, 2014-06-05 21:48:45]

On 2014/06/05 17:00:42, nednguyen wrote:
>
https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/core/backends/chrome/inspector_page.py (left):
> 
>
https://codereview.chromium.org/317913003/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/core/backends/chrome/inspector_page.py:82:
> action_function()
> On 2014/06/05 16:54:24, tonyg wrote:
> > I like the cleanup in this patch, but IIRC, performing the action here
reduces
> > flake.
> > 
> > Perhaps the code has changed since I remember, but my recollection was that
if
> > we perform the action that causes the navigation as a separate action prior
to
> > this then there's a chance the action will complete prior to when we begin
> > listening for the Page.frameNavigated event. In that case, we'd just wait
> > forever for an event that already came.
> > 
> > Can you confirm whether that's still relevant?
> I think the code path should be almost the same. If we rely on the tiny
> different of the telemetry code path to avoid flake, I think we were doing it
> wrong.

lgtm

We always handle page notifications synchronously, so we can guarantee that we
won't receive and process the frameNavigated event until we call
DispatchNotifications and enter a polling loop. We'll never wait for an event
that already happened, because even if it did, we haven't processed the event
yet.

I think in the past we might've called "Page.enable" at navigation time, which
would have the problem tonyg mentioned. But that's not how the code looks now,
so maybe the code's just changed since then.

We do process notifications asynchronously for tracing, but that uses a
different websocket connection, so there's no conflict. We just have to be
careful to never call StartAsyncDispatchNotifications on an InspectorBackend.

In any case, I'm glad for this cleanup. Yay!

[nednguyen, 2014-06-05 21:58:46]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-06-05 21:59:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nednguyen@google.com/317913003/20001

[commit-bot: I haz the power, 2014-06-06 02:18:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-06 02:23:09]

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

[commit-bot: I haz the power, 2014-06-06 02:23:10]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[nednguyen, 2014-06-06 02:59:16]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-06-06 03:01:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nednguyen@google.com/317913003/30001

[commit-bot: I haz the power, 2014-06-06 07:24:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)

[commit-bot: I haz the power, 2014-06-06 07:29:21]

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

[commit-bot: I haz the power, 2014-06-06 07:29:22]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[nednguyen, 2014-06-06 14:35:49]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-06-06 14:36:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nednguyen@google.com/317913003/30001

[commit-bot: I haz the power, 2014-06-06 14:47:46]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)

[commit-bot: I haz the power, 2014-06-06 14:51:20]

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

[commit-bot: I haz the power, 2014-06-06 14:51:21]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[nednguyen, 2014-06-06 15:18:22]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-06-06 15:19:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nednguyen@google.com/317913003/50001

[commit-bot: I haz the power, 2014-06-06 18:50:47]

Change committed as 275487