Replace TabModel with WebStateList in GoogleLandingController.

Replace TabModel with WebStateList in GoogleLandingController.

BUG=694750
Review-Url: https://codereview.chromium.org/2833513002
Cr-Commit-Position: refs/heads/master@{#466542}
Committed: https://chromium.googlesource.com/chromium/src/+/892edbbf57698e574b6b2b15481239af1c5c4ede

[justincohen, 2017-04-19 16:59:34]

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

[justincohen, 2017-04-19 16:59:35]

Dependant CL, removes tabModel

[rohitrao (ping after 24h), 2017-04-19 19:26:33]

lgtm

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:1462: _tabCount =
tabCount;
Why do we need to cache this in an ivar?  Does headerView only sometimes exist? 
Is tabCount sometimes called before the view is loaded?

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:157: _webStateListObserver
= base::MakeUnique<WebStateListObserverBridge>(self);
Louis has an example CL that uses a ScopedObserver, if you want to look at that
and see if you like it.

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:166:
_webStateList->RemoveObserver(_webStateListObserver.get());
dealloc might be too late, if this object has its lifetime artificially extended
by a block.  We're working on developing a consistent pattern to use here.

[justincohen, 2017-04-19 21:12:23]

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_controller.mm (right):

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_controller.mm:1462: _tabCount =
tabCount;
On 2017/04/19 19:26:33, rohitrao wrote:
> Why do we need to cache this in an ivar?  Does headerView only sometimes
exist? 
> Is tabCount sometimes called before the view is loaded?

headerView is created later.

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
File ios/chrome/browser/ui/ntp/google_landing_mediator.mm (right):

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:157: _webStateListObserver
= base::MakeUnique<WebStateListObserverBridge>(self);
On 2017/04/19 19:26:33, rohitrao wrote:
> Louis has an example CL that uses a ScopedObserver, if you want to look at
that
> and see if you like it.

Acknowledged.

https://codereview.chromium.org/2833513002/diff/1/ios/chrome/browser/ui/ntp/g...
ios/chrome/browser/ui/ntp/google_landing_mediator.mm:166:
_webStateList->RemoveObserver(_webStateListObserver.get());
On 2017/04/19 19:26:33, rohitrao wrote:
> dealloc might be too late, if this object has its lifetime artificially
extended
> by a block.  We're working on developing a consistent pattern to use here.

I think the view's life can be extended, but not the controller?

[justincohen, 2017-04-22 05:53:52]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2017-04-22 05:53:53]

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/2833513002/#ps70001
(title: "Rebase onto master")

[commit-bot: I haz the power, 2017-04-22 05:54:03]

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-22 07:09:10]

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

[commit-bot: I haz the power, 2017-04-22 07:09:12]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[justincohen, 2017-04-22 13:04:11]

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

[commit-bot: I haz the power, 2017-04-22 13:04:17]

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-22 13:11:14]

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

[commit-bot: I haz the power, 2017-04-22 13:11:15]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[justincohen, 2017-04-22 13:39:23]

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

[commit-bot: I haz the power, 2017-04-22 13:39:28]

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-22 14:47:21]

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

[commit-bot: I haz the power, 2017-04-22 14:47:21]

Dry run: This issue passed the CQ dry run.

[justincohen, 2017-04-22 14:49:16]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2017-04-22 14:49:16]

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/2833513002/#ps110001
(title: "Fix gn")

[commit-bot: I haz the power, 2017-04-22 14:49: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-04-22 19:14:58]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1492872556505720,
"parent_rev": "94561434e49d009b34dd000575f14537802ec6aa", "commit_rev":
"892edbbf57698e574b6b2b15481239af1c5c4ede"}

[commit-bot: I haz the power, 2017-04-22 19:15:09]

Description was changed from

==========
Replace TabModel with WebStateList in GoogleLandingController.

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

to

==========
Replace TabModel with WebStateList in GoogleLandingController.

BUG=694750

Review-Url: https://codereview.chromium.org/2833513002
Cr-Commit-Position: refs/heads/master@{#466542}
Committed:
https://chromium.googlesource.com/chromium/src/+/892edbbf57698e574b6b2b154812...
==========

[commit-bot: I haz the power, 2017-04-22 19:15:10]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/892edbbf57698e574b6b2b154812...

[justincohen, 2017-04-23 14:46:49]

A revert of this CL (patchset #7 id:110001) has been created in
https://codereview.chromium.org/2835033002/ by justincohen@chromium.org.

The reason for reverting is: Broke downstream eg tests, needs some minor fixes..