PlzNavigate: Prevent tests to end too early.

PlzNavigate: Prevent tests to end too early.

This patch prevents the FrameLoader to send 'complete' notification when
there is still a navigation handled by the client ongoing.

This bug caused several tests to end too early and fail.
In contrast, there were tests that passed previously, but actually fail.

R=japhet@chromium.org
BUG=576261
Committed: https://crrev.com/2b898321fc29eb3993944fdeb19d9b5a32667dec
Cr-Commit-Position: refs/heads/master@{#418746}

[arthursonzogni, 2016-08-22 14:57:57]

Hi Nate,
Could you review this CL ?

Basically, I just add a boolean in FrameLoader to prevent it to send 'complete'
notifications when there is still a browser-side navigation ongoing. Before this
patch, the test runner finished too early and some tests didn't have time to
finish. 

There was 4 tests marked 'PASS'. They don't pass anymore, they were
false-positive. I identified what is the problem for these ones.(Ctrl+click
doesn't work properly with PlzNavigate yet)

Thanks,
Arthur

[arthursonzogni, 2016-08-22 15:00:09]

CC to clamy@

[Nate Chapin, 2016-09-08 18:44:23]

Sorry, this patch slipped through the cracks.

I'm not super familiar with PlzNavigate's details, so I could use a bit of
background. Is there a case where m_isNavigationHandledByClient could get stuck
true? A navigation is sent to the browser process, but it errors and we never
get a response back? How does this work if the navigation swaps processes?

[clamy, 2016-09-09 10:20:45]

clamy@chromium.org changed reviewers:
+ clamy@chromium.org

[clamy, 2016-09-09 10:20:45]

We ensure in the browser that if the navigation stops or is transferred to
another process we send a Stop IPC to inform the renderer the navigation stopped
(note we may eventually have a specific IPC for that, right now I juste resused
the Stop one).

[Nate Chapin, 2016-09-09 17:44:49]

I don't love adding another bool to FrameLoader, but this is probably the
cleanest way to handle this case. LGTM with a question about the expectations
change.

https://codereview.chromium.org/2269653002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
(right):

https://codereview.chromium.org/2269653002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation:31:
editing/pasteboard/paste-when-over-link.html [ Failure ]
Why are these failures being added here?

[arthursonzogni, 2016-09-15 00:15:59]

Thanks Nate!
Yes it is not really beautiful. But I don't have a cleanest way of doing it for
the moment...

https://codereview.chromium.org/2269653002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
(right):

https://codereview.chromium.org/2269653002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation:31:
editing/pasteboard/paste-when-over-link.html [ Failure ]
On 2016/09/09 17:44:49, Nate Chapin wrote:
> Why are these failures being added here?

Opening link in a new tab / new window was not working with PlzNavigate.
I made a fix for it :
https://codereview.chromium.org/2323153003/

Instead of opening the link in a new tabs, it is opened in the same tab. It
causes a mismatch with the expected output (bug #1)
But Because the test runner ends too early (bug #2), the browser don't have time
to open the new page, the test passes anyway. These are two bugs that
compensate.

[arthursonzogni, 2016-09-15 00:16:13]

The CQ bit was checked by arthursonzogni@chromium.org

[arthursonzogni, 2016-09-15 00:16:13]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org
Link to the patchset: https://codereview.chromium.org/2269653002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-15 00:16:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-15 01:36:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-15 01:39:31]

Description was changed from

==========
PlzNavigate: Prevent tests to end too early.

This patch prevents the FrameLoader to send 'complete' notification when
there is still a navigation handled by the client ongoing.

This bug caused several tests to end too early and fail.
In contrast, there were tests that passed previously, but actually fail.

R=japhet@chromium.org
BUG=576261
==========

to

==========
PlzNavigate: Prevent tests to end too early.

This patch prevents the FrameLoader to send 'complete' notification when
there is still a navigation handled by the client ongoing.

This bug caused several tests to end too early and fail.
In contrast, there were tests that passed previously, but actually fail.

R=japhet@chromium.org
BUG=576261

Committed: https://crrev.com/2b898321fc29eb3993944fdeb19d9b5a32667dec
Cr-Commit-Position: refs/heads/master@{#418746}
==========

[commit-bot: I haz the power, 2016-09-15 01:39:32]

Patchset 2 (id:??) landed as
https://crrev.com/2b898321fc29eb3993944fdeb19d9b5a32667dec
Cr-Commit-Position: refs/heads/master@{#418746}