Convert SessionCrashedBubbleView to use the new navigation callbacks.

Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed. Note that also DidStartNavigationToPendingEntry is skipped for the initial NTP navigation since it happens before this WebContentObserver is added to the first tab. However DidStartNavigation has different timing and would be reached for the NTP.

BUG=682002
Review-Url: https://codereview.chromium.org/2641813004
Cr-Commit-Position: refs/heads/master@{#444993}
Committed: https://chromium.googlesource.com/chromium/src/+/aa90ea1cec4f6bcc64a494c9a150e16f0e01c508

[jam, 2017-01-18 23:28:04]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[jam, 2017-01-18 23:28:43]

Description was changed from

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

BUG=682002
==========

to

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed.

BUG=682002
==========

[commit-bot: I haz the power, 2017-01-18 23:28:49]

Dry run: 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, 2017-01-19 00:13:08]

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

[commit-bot: I haz the power, 2017-01-19 00:13:08]

Dry run: This issue passed the CQ dry run.

[jam, 2017-01-19 00:19:33]

Description was changed from

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed.

BUG=682002
==========

to

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed. Note
that also DidStartNavigationToPendingEntry is skipped for the initial NTP
navigation since it happens before this WebContentObserver is added to the first
tab. However DidStartNavigation has different timing and would be reached for
the NTP.

BUG=682002
==========

[jam, 2017-01-19 00:19:33]

jam@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2017-01-19 00:39:42]

https://codereview.chromium.org/2641813004/diff/1/chrome/browser/ui/views/ses...
File chrome/browser/ui/views/session_crashed_bubble_view.cc (right):

https://codereview.chromium.org/2641813004/diff/1/chrome/browser/ui/views/ses...
chrome/browser/ui/views/session_crashed_bubble_view.cc:341: if
(render_frame_host->GetParent())
How come you early out if there is a parent? Isn't there always a parent? A
comment would help.

[jam, 2017-01-19 01:05:56]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[jam, 2017-01-19 01:06:13]

https://codereview.chromium.org/2641813004/diff/1/chrome/browser/ui/views/ses...
File chrome/browser/ui/views/session_crashed_bubble_view.cc (right):

https://codereview.chromium.org/2641813004/diff/1/chrome/browser/ui/views/ses...
chrome/browser/ui/views/session_crashed_bubble_view.cc:341: if
(render_frame_host->GetParent())
On 2017/01/19 00:39:42, sky wrote:
> How come you early out if there is a parent? Isn't there always a parent? A
> comment would help.

It's to maintain the same behavior as before, which only considered main frame
navigations. DidStartNavigationToPendingEntry didn't fire for subframes. So
without this check now, the NTP's frames loading would close the bubble.

Added a comment

[commit-bot: I haz the power, 2017-01-19 01:06:34]

Dry run: 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, 2017-01-19 02:03:25]

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

[commit-bot: I haz the power, 2017-01-19 02:03:26]

Dry run: This issue passed the CQ dry run.

[sky, 2017-01-19 15:51:29]

Got it! LGTM

[jam, 2017-01-20 05:29:50]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-01-20 05:30:05]

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, 2017-01-20 06:08:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484890190201960,
"parent_rev": "c3db3992faefec9a170da7150ee80ee5567c9a44", "commit_rev":
"aa90ea1cec4f6bcc64a494c9a150e16f0e01c508"}

[commit-bot: I haz the power, 2017-01-20 06:09:44]

Description was changed from

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed. Note
that also DidStartNavigationToPendingEntry is skipped for the initial NTP
navigation since it happens before this WebContentObserver is added to the first
tab. However DidStartNavigation has different timing and would be reached for
the NTP.

BUG=682002
==========

to

==========
Convert SessionCrashedBubbleView to use the new navigation callbacks.

In this specific case, I didn't see a reason that two callbacks are needed. Note
that also DidStartNavigationToPendingEntry is skipped for the initial NTP
navigation since it happens before this WebContentObserver is added to the first
tab. However DidStartNavigation has different timing and would be reached for
the NTP.

BUG=682002

Review-Url: https://codereview.chromium.org/2641813004
Cr-Commit-Position: refs/heads/master@{#444993}
Committed:
https://chromium.googlesource.com/chromium/src/+/aa90ea1cec4f6bcc64a494c9a150...
==========

[commit-bot: I haz the power, 2017-01-20 06:09:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/aa90ea1cec4f6bcc64a494c9a150...