If the renderer drops a navigation, make sure the spinner doesn't spin endlessly.

If the renderer drops a navigation, make sure the spinner doesn't spin endlessly.

BUG=431853
TEST=none

Committed: https://crrev.com/1a89b7149ee39637f63d791aad7ae84e4b4e768c
Cr-Commit-Position: refs/heads/master@{#307567}

[Avi (use Gerrit), 2014-12-09 17:16:29]

avi@chromium.org changed reviewers:
+ creis@chromium.org, nasko@chromium.org

[Avi (use Gerrit), 2014-12-09 17:16:30]

Charlie for review, Nasko for IPC

[nasko, 2014-12-09 18:13:33]

IPC LGTM and one generic question.

https://codereview.chromium.org/790843002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/790843002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_delegate.h:67: // The top-level
RenderFrame stopped loading a page. This corresponds to
Is this going to be only for top-level frames? I don't see this enforced in
code. I also imagine we will have frames report start/stop loading events, since
browser side will know the full picture, so I doubt longer term this will be
isolated to top-level only.

[Charlie Reis, 2014-12-09 19:28:15]

Great!

Can we verify that the new IPC is sent in
RenderViewImplTest.StaleNavigationsIgnored?

Otherwise LGTM with a nit.

https://codereview.chromium.org/790843002/diff/20001/content/common/frame_mes...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/790843002/diff/20001/content/common/frame_mes...
content/common/frame_messages.h:581: // Sent when the renderer drops a
navigation.
nit: drops a navigation -> decides to ignore a navigation request.

(Just wanted to elaborate a bit in case someone didn't understand the name of
the message.)

[Avi (use Gerrit), 2014-12-09 20:45:27]

https://codereview.chromium.org/790843002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/790843002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_delegate.h:67: // The top-level
RenderFrame stopped loading a page. This corresponds to
On 2014/12/09 18:13:33, nasko wrote:
> Is this going to be only for top-level frames? I don't see this enforced in
> code. I also imagine we will have frames report start/stop loading events,
since
> browser side will know the full picture, so I doubt longer term this will be
> isolated to top-level only.

No, but the comment was stolen from DidStartLoading, and _that_ is called from
RenderFrameHostImpl::Navigate for all frames _also_. Will fix the comments.

https://codereview.chromium.org/790843002/diff/20001/content/common/frame_mes...
File content/common/frame_messages.h (right):

https://codereview.chromium.org/790843002/diff/20001/content/common/frame_mes...
content/common/frame_messages.h:581: // Sent when the renderer drops a
navigation.
On 2014/12/09 19:28:15, Charlie Reis wrote:
> nit: drops a navigation -> decides to ignore a navigation request.
> 
> (Just wanted to elaborate a bit in case someone didn't understand the name of
> the message.)

Done.

[Avi (use Gerrit), 2014-12-09 20:45:43]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2014-12-09 20:46:59]

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

[Avi (use Gerrit), 2014-12-09 21:09:39]

The CQ bit was unchecked by avi@chromium.org

[Charlie Reis, 2014-12-09 21:17:16]

Test LGTM.  Thanks!

[Avi (use Gerrit), 2014-12-09 21:17:38]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2014-12-09 21:17:50]

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

[commit-bot: I haz the power, 2014-12-09 22:25:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-09 22:26:14]

Patchset 4 (id:??) landed as
https://crrev.com/1a89b7149ee39637f63d791aad7ae84e4b4e768c
Cr-Commit-Position: refs/heads/master@{#307567}