Convert NavigationController unit and browser tests to use the new navigation callbacks.

Convert NavigationController unit and browser tests to use the new navigation callbacks.

BUG=682002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2682193002
Cr-Commit-Position: refs/heads/master@{#449205}
Committed: https://chromium.googlesource.com/chromium/src/+/a0e5e40a1f939430bf21f60045bdf5bc28100f97

[jam, 2017-02-08 21:07:29]

Description was changed from

==========
Convert NavigationController unit and browser tests to use the new navigation
callbacks.

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

to

==========
Convert NavigationController unit and browser tests to use the new navigation
callbacks.

BUG=682002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2017-02-08 21:08:27]

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

[commit-bot: I haz the power, 2017-02-08 21:09:22]

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

[jam, 2017-02-08 22:32:54]

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

[commit-bot: I haz the power, 2017-02-08 23:28:07]

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

[commit-bot: I haz the power, 2017-02-08 23:28:08]

Dry run: This issue passed the CQ dry run.

[nasko, 2017-02-09 00:29:12]

LGTM with a few comments and nits.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/interstitial_page_navigator_impl.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/interstitial_page_navigator_impl.cc:53:
NAVIGATION_TYPE_UNKNOWN,
Interstitials navigate to a new page, so it seems more fitting to use
NAVIGATION_TYPE_NEW_PAGE.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:666:
std::vector<NavigationType> types_;
nit: navigation_types_

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_unittest.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_unittest.cc:339:
NavigationType type_;
nit: navigation_type_

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_handle_impl.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:435:
DidCommitNavigation(params, false, GURL(), NAVIGATION_TYPE_UNKNOWN,
I think the safer default is NAVIGATION_TYPE_NEW_PAGE for tests.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_handle_impl.h (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.h:319: NavigationType
navigation_type() { return navigation_type_; }
Since this is set at commit time, shouldn't we DCHECK the state to ensure
callers don't do it earlier?

[jam, 2017-02-09 01:13:41]

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

[commit-bot: I haz the power, 2017-02-09 01:14:37]

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

[jam, 2017-02-09 01:15:04]

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/interstitial_page_navigator_impl.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/interstitial_page_navigator_impl.cc:53:
NAVIGATION_TYPE_UNKNOWN,
On 2017/02/09 00:29:11, nasko (very slow) wrote:
> Interstitials navigate to a new page, so it seems more fitting to use
> NAVIGATION_TYPE_NEW_PAGE.

Done.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:666:
std::vector<NavigationType> types_;
On 2017/02/09 00:29:11, nasko (very slow) wrote:
> nit: navigation_types_

Done.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_unittest.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_unittest.cc:339:
NavigationType type_;
On 2017/02/09 00:29:11, nasko (very slow) wrote:
> nit: navigation_type_

Done.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_handle_impl.cc (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.cc:435:
DidCommitNavigation(params, false, GURL(), NAVIGATION_TYPE_UNKNOWN,
On 2017/02/09 00:29:11, nasko (very slow) wrote:
> I think the safer default is NAVIGATION_TYPE_NEW_PAGE for tests.

Done.

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_handle_impl.h (right):

https://codereview.chromium.org/2682193002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_handle_impl.h:319: NavigationType
navigation_type() { return navigation_type_; }
On 2017/02/09 00:29:11, nasko (very slow) wrote:
> Since this is set at commit time, shouldn't we DCHECK the state to ensure
> callers don't do it earlier?

Done.

[jam, 2017-02-09 01:15:09]

The CQ bit was unchecked by jam@chromium.org

[jam, 2017-02-09 01:15:15]

The CQ bit was checked by jam@chromium.org

[jam, 2017-02-09 01:15:15]

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

[jam, 2017-02-09 01:15:16]

The CQ bit was unchecked by jam@chromium.org

[jam, 2017-02-09 01:15:18]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-02-09 01:15:32]

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-02-09 01:17:15]

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

[commit-bot: I haz the power, 2017-02-09 01:17:16]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jam, 2017-02-09 01:26:37]

The CQ bit was checked by jam@chromium.org

[jam, 2017-02-09 01:26:37]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2682193002/#ps40001
(title: "merge")

[commit-bot: I haz the power, 2017-02-09 01:27:25]

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-02-09 05:20:11]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486603597385690,
"parent_rev": "6423d6b3eea01831f79139b2d9408dc565a45077", "commit_rev":
"a0e5e40a1f939430bf21f60045bdf5bc28100f97"}

[commit-bot: I haz the power, 2017-02-09 05:21:34]

Description was changed from

==========
Convert NavigationController unit and browser tests to use the new navigation
callbacks.

BUG=682002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Convert NavigationController unit and browser tests to use the new navigation
callbacks.

BUG=682002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2017-02-09 05:21:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a0e5e40a1f939430bf21f60045bd...