PlzNavigate: Fix crash when loading unreachable URL.

PlzNavigate: Fix crash when loading unreachable URL.

In RenderFrameImpl::OnFailedNavigation, when loading an unreachable
URL, the condition : ShouldDisplayErrorPageForFailedLoad(..) return true the
first time. But the auto_reload feature ask for navigating again some time
later multiple times. These times, ShouldDisplayErrorPageForFailedLoad(..)
returns false because auto_reload_in_flight_ is true and we don't want to
display the error page again.

Thus, putting here the Macro NOTREACHED() causes crashs for every
failed navigation.

I restored the behavior found in RenderFrameImpl::DidFailProvisionalLoad, which
is the non-PlzNavigate counterpart of this function and stop the navigation.

TEST=Try to navigate to an unexisting url  (unreachableurl.com for instance)
and see if it crashs. Use --enable-browser-side-navigation.
R=clamy@chromium.org
BUG=

Committed: https://crrev.com/3c2d8ddde185e0290387df2e7abeb1175370dd29
Cr-Commit-Position: refs/heads/master@{#412807}

[arthursonzogni, 2016-08-16 15:08:25]

clamy@ PTAL.

Thanks.

[clamy, 2016-08-17 13:17:07]

Thanks!

https://codereview.chromium.org/2248013003/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2248013003/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:4882: didStopLoading();
We should send the IPC directly, as this is more of acommunication issue with
the renderer than something coming from blink.

Also add a comment like:
// The browser expects this frame to be loading an error page. Inform it that
the load stopped.

[arthursonzogni, 2016-08-17 13:45:09]

https://codereview.chromium.org/2248013003/diff/1/content/renderer/render_fra...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2248013003/diff/1/content/renderer/render_fra...
content/renderer/render_frame_impl.cc:4882: didStopLoading();
On 2016/08/17 13:17:06, clamy wrote:
> We should send the IPC directly, as this is more of acommunication issue with
> the renderer than something coming from blink.
> 
> Also add a comment like:
> // The browser expects this frame to be loading an error page. Inform it that
> the load stopped.

Done.

[clamy, 2016-08-17 14:27:13]

https://codereview.chromium.org/2248013003/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2248013003/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:4884: Send(new
FrameHostMsg_DidStopLoading(routing_id_));
We should only send the IPC if the frame is not loading already (see what is
done at the end of NavigateInternal).

[arthursonzogni, 2016-08-17 15:57:01]

I think it is okay now.

https://codereview.chromium.org/2248013003/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2248013003/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:4884: Send(new
FrameHostMsg_DidStopLoading(routing_id_));
On 2016/08/17 14:27:13, clamy wrote:
> We should only send the IPC if the frame is not loading already (see what is
> done at the end of NavigateInternal).

Done.

[clamy, 2016-08-18 12:15:24]

Thanks! Lgtm.

[arthursonzogni, 2016-08-18 12:21:48]

The CQ bit was checked by arthursonzogni@chromium.org

[commit-bot: I haz the power, 2016-08-18 12:22:02]

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-08-18 13:16:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-18 13:18:20]

Description was changed from

==========
PlzNavigate: Fix crash when loading unreachable URL.

In RenderFrameImpl::OnFailedNavigation, when loading an unreachable
URL, the condition : ShouldDisplayErrorPageForFailedLoad(..) return true the
first time. But the auto_reload feature ask for navigating again some time
later multiple times. These times, ShouldDisplayErrorPageForFailedLoad(..)
returns false because auto_reload_in_flight_ is true and we don't want to
display the error page again.

Thus, putting here the Macro NOTREACHED() causes crashs for every
failed navigation.

I restored the behavior found in RenderFrameImpl::DidFailProvisionalLoad, which
is the non-PlzNavigate counterpart of this function and stop the navigation.

TEST=Try to navigate to an unexisting url  (unreachableurl.com for instance)
and see if it crashs. Use --enable-browser-side-navigation.
R=clamy@chromium.org
BUG=
==========

to

==========
PlzNavigate: Fix crash when loading unreachable URL.

In RenderFrameImpl::OnFailedNavigation, when loading an unreachable
URL, the condition : ShouldDisplayErrorPageForFailedLoad(..) return true the
first time. But the auto_reload feature ask for navigating again some time
later multiple times. These times, ShouldDisplayErrorPageForFailedLoad(..)
returns false because auto_reload_in_flight_ is true and we don't want to
display the error page again.

Thus, putting here the Macro NOTREACHED() causes crashs for every
failed navigation.

I restored the behavior found in RenderFrameImpl::DidFailProvisionalLoad, which
is the non-PlzNavigate counterpart of this function and stop the navigation.

TEST=Try to navigate to an unexisting url  (unreachableurl.com for instance)
and see if it crashs. Use --enable-browser-side-navigation.
R=clamy@chromium.org
BUG=

Committed: https://crrev.com/3c2d8ddde185e0290387df2e7abeb1175370dd29
Cr-Commit-Position: refs/heads/master@{#412807}
==========

[commit-bot: I haz the power, 2016-08-18 13:18:21]

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