Convert StartPageService and ZoomController to use the new navigation callbacks.

Convert StartPageService and ZoomController to use the new navigation callbacks.

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

[jam, 2017-01-27 01:28:13]

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

[commit-bot: I haz the power, 2017-01-27 01:29:36]

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

[jam, 2017-01-27 01:30:46]

Description was changed from

==========
Convert StartPageService and ZoomController to use the new navigation callbacks.

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

to

==========
Convert StartPageService and ZoomController to use the new navigation callbacks.

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

[jam, 2017-01-27 01:30:47]

jam@chromium.org changed reviewers:
+ dbeam@chromium.org, xiyuan@chromium.org

[commit-bot: I haz the power, 2017-01-27 02:34:55]

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

[commit-bot: I haz the power, 2017-01-27 02:34:56]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jam, 2017-01-27 04:34:54]

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

[commit-bot: I haz the power, 2017-01-27 04:35:24]

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-27 05:20:48]

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

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

Dry run: This issue passed the CQ dry run.

[jam, 2017-01-27 06:24:36]

dbeam: components
xiyuan: the rest

I had to do these together because of the comment in StartPageService

[xiyuan, 2017-01-27 15:31:50]

lgtm

[Dan Beam, 2017-01-27 20:33:21]

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/start_page_service.cc (left):

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/start_page_service.cc:576: void
StartPageService::DidFailProvisionalLoad(
did this only apply to the main page?  did this get called without a commit?

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/start_page_service.cc (right):

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/start_page_service.cc:563: if
(!navigation_handle->IsInMainFrame() || !navigation_handle->HasCommitted())
should this !IsInMainFrame() be after the error check if DidFailProvisionalLoad
applied to all frames, not just the main one?

[jam, 2017-01-27 21:28:57]

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/start_page_service.cc (left):

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/start_page_service.cc:576: void
StartPageService::DidFailProvisionalLoad(
On 2017/01/27 20:33:21, Dan Beam wrote:
> did this only apply to the main page?

The chrome apps page has no frames, so the IsInMainFrame() doesn't matter.

>  did this get called without a commit?

In the old api, a navigation is provisional until it either commits or fails.
I'm not sure what your question means?

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/start_page_service.cc (right):

https://codereview.chromium.org/2662503002/diff/20001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/start_page_service.cc:563: if
(!navigation_handle->IsInMainFrame() || !navigation_handle->HasCommitted())
On 2017/01/27 20:33:21, Dan Beam wrote:
> should this !IsInMainFrame() be after the error check if
DidFailProvisionalLoad
> applied to all frames, not just the main one?

per the other comment, these two methods realy only deal with the main frame.

[Dan Beam, 2017-01-27 21:55:55]

lgtm then, just wanted to make sure these are equivalent as possible to the
previous code (because i certainly don't understand them super well nor am even
sure how to test them manually)

[jam, 2017-01-27 23:04:59]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2017-01-27 23:05:56]

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-27 23:14:22]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485558299332510,
"parent_rev": "506c250363a28b39338e69aaa50bc83707fe5767", "commit_rev":
"e1f39841ab7e989b300f08d90f480b247a08ccf6"}

[commit-bot: I haz the power, 2017-01-27 23:14:53]

Description was changed from

==========
Convert StartPageService and ZoomController to use the new navigation callbacks.

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

to

==========
Convert StartPageService and ZoomController to use the new navigation callbacks.

BUG=682002

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

[commit-bot: I haz the power, 2017-01-27 23:14:55]

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