Remove WebToolbarDelegate from GoogleLandingDataSource.

Remove WebToolbarDelegate from GoogleLandingDataSource.

BUG=694750
Review-Url: https://codereview.chromium.org/2832573002
Cr-Commit-Position: refs/heads/master@{#467980}
Committed: https://chromium.googlesource.com/chromium/src/+/394a4f2f148ad91c04bacb5df265248898844a3b

[justincohen, 2017-04-19 17:28:40]

justincohen@chromium.org changed reviewers:
+ rohitrao@chromium.org

[justincohen, 2017-04-19 17:28:41]

ptal, more removal.

[justincohen, 2017-04-22 06:00:13]

Next up on the refactor list.  PTAL!

[justincohen, 2017-04-25 12:30:07]

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

[commit-bot: I haz the power, 2017-04-25 12:30:19]

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-04-25 12:40:39]

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

[commit-bot: I haz the power, 2017-04-25 12:40:40]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-04-28 10:39:59]

lgtm

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:196: // |YES| if the
google landing toolbar can show the forward arrow.
Perhaps mention somewhere that these two values are cached because they need to
be pushed into the header view, and the header view may not currently exist.

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:180: if (webState) {
Should we forcibly disable the buttons if there is no webstate?  Do they always
default to the disabled state?  Can we guarantee that?

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:297: if (newWebState) {
Same question -- if newWebState is nil, should we disable the buttons?

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:372: -
(void)prepareToEnterTabSwitcher:(id)sender {
Can you document why this call is necessary?

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:373: [_webToolbarDelegate
prepareToEnterTabSwitcher:sender];
I thought you added this method to the DataSource protocol, not to
WebToolbarDelegate?

[justincohen, 2017-04-28 13:23:04]

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:196: // |YES| if the
google landing toolbar can show the forward arrow.
On 2017/04/28 10:39:59, rohitrao (ping after 24h) wrote:
> Perhaps mention somewhere that these two values are cached because they need
to
> be pushed into the header view, and the header view may not currently exist.

Done.

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:180: if (webState) {
On 2017/04/28 10:39:59, rohitrao (ping after 24h) wrote:
> Should we forcibly disable the buttons if there is no webstate?  Do they
always
> default to the disabled state?  Can we guarantee that?

No, it means this NTP is about to be destroyed.

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:297: if (newWebState) {
On 2017/04/28 10:39:59, rohitrao (ping after 24h) wrote:
> Same question -- if newWebState is nil, should we disable the buttons?

No, it means this NTP is about to go away.

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:372: -
(void)prepareToEnterTabSwitcher:(id)sender {
On 2017/04/28 10:39:59, rohitrao (ping after 24h) wrote:
> Can you document why this call is necessary?

Done.

https://codereview.chromium.org/2832573002/diff/120001/ios/chrome/browser/ui/...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:373: [_webToolbarDelegate
prepareToEnterTabSwitcher:sender];
On 2017/04/28 10:39:59, rohitrao (ping after 24h) wrote:
> I thought you added this method to the DataSource protocol, not to
> WebToolbarDelegate?

This is the data source protocol -- note that this goes away in the next
dispatcher CL

[justincohen, 2017-04-28 13:23:08]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2017-04-28 13:23:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2832573002/#ps140001
(title: "New comments")

[commit-bot: I haz the power, 2017-04-28 13:23:29]

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-04-28 13:35:01]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1493385788700690,
"parent_rev": "1b2b965b6d7588243a28803a4f5253f737e393e0", "commit_rev":
"394a4f2f148ad91c04bacb5df265248898844a3b"}

[commit-bot: I haz the power, 2017-04-28 13:35:14]

Description was changed from

==========
Remove WebToolbarDelegate from GoogleLandingDataSource.

BUG=694750
==========

to

==========
Remove WebToolbarDelegate from GoogleLandingDataSource.

BUG=694750

Review-Url: https://codereview.chromium.org/2832573002
Cr-Commit-Position: refs/heads/master@{#467980}
Committed:
https://chromium.googlesource.com/chromium/src/+/394a4f2f148ad91c04bacb5df265...
==========

[commit-bot: I haz the power, 2017-04-28 13:35:16]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/394a4f2f148ad91c04bacb5df265...