[ios] TabCollectionItem

[ios] TabCollectionItem

In this CL, the collection view controller no longer uses an external 
data source protocol. The collection view controller now has its
own model, which is an array of TabCollectionItem. 

BUG=686770
Review-Url: https://codereview.chromium.org/2885983003
Cr-Commit-Position: refs/heads/master@{#472991}
Committed: https://chromium.googlesource.com/chromium/src/+/66fd39bba716cb6aa352b9274a54333af05b5322

[edchin, 2017-05-18 06:00:08]

Description was changed from

==========
[ios] TabCollectionItem

BUG=
==========

to

==========
[ios] TabCollectionItem

In this CL, the collection view controller no longer uses an external 
data source protocol. The collection view controller now has its
own model, which is an array of TabCollectionItem. 

BUG=686770
==========

[edchin, 2017-05-18 06:00:17]

Patchset #1 (id:1) has been deleted

[edchin, 2017-05-18 06:00:25]

Patchset #1 (id:20001) has been deleted

[edchin, 2017-05-18 06:00:31]

Patchset #1 (id:40001) has been deleted

[edchin, 2017-05-18 06:01:31]

Patchset #1 (id:60001) has been deleted

[edchin, 2017-05-18 06:01:51]

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

[commit-bot: I haz the power, 2017-05-18 06:04:10]

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-05-18 07:15:44]

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

[commit-bot: I haz the power, 2017-05-18 07:15:45]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[edchin, 2017-05-18 14:39:52]

edchin@chromium.org changed reviewers:
+ jif@chromium.org, sczs@chromium.org

[edchin, 2017-05-18 14:40:04]

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

[commit-bot: I haz the power, 2017-05-18 14:40:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sczs, 2017-05-18 16:11:02]

lgtm with some nits

Maybe add a little bit more info to the issue Subject

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h:8: #import
"ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h"
Can we forward declare it?

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h:28: //
Initializes tab collection with |items|.
initialize sounds like this would return a TabCollection object, WDYT about
populateItems or something like that?

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h (right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h:10: @interface
TabCollectionItem : NSObject
Can you add a comment for this Class

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator.mm
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator.mm:104: -
(TabCollectionItem*)itemFromWebState:(const web::WebState*)webState {
nit: WDYT about changing this to tabCollectionItemFromWebState, it could add
some clarity.

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator_unittest.mm
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator_unittest.mm:32:
[OCMockObject niceMockForProtocol:@protocol(TabCollectionConsumer)];
nit: What about using OCMProtocolMock(@protocol(TabCollectionConsumer)); now
that the rest of the file is using OCMock 3.0 syntax.

[commit-bot: I haz the power, 2017-05-18 17:15:34]

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

[commit-bot: I haz the power, 2017-05-18 17:15:35]

Dry run: This issue passed the CQ dry run.

[edchin, 2017-05-18 17:30:27]

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h:8: #import
"ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h"
On 2017/05/18 16:11:01, sczs wrote:
> Can we forward declare it? 

Done.

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_consumer.h:28: //
Initializes tab collection with |items|.
On 2017/05/18 16:11:01, sczs wrote:
> initialize sounds like this would return a TabCollection object, WDYT about
> populateItems or something like that? 

Done.

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h (right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_item.h:10: @interface
TabCollectionItem : NSObject
On 2017/05/18 16:11:02, sczs wrote:
> Can you add a comment for this Class

Done.

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator.mm
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator.mm:104: -
(TabCollectionItem*)itemFromWebState:(const web::WebState*)webState {
On 2017/05/18 16:11:02, sczs wrote:
> nit: WDYT about changing this to tabCollectionItemFromWebState, it could add
> some clarity.

Done.

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
File
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator_unittest.mm
(right):

https://codereview.chromium.org/2885983003/diff/80001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/tab_collection/tab_collection_mediator_unittest.mm:32:
[OCMockObject niceMockForProtocol:@protocol(TabCollectionConsumer)];
On 2017/05/18 16:11:02, sczs wrote:
> nit: What about using OCMProtocolMock(@protocol(TabCollectionConsumer)); now
> that the rest of the file is using OCMock 3.0 syntax.

Done.

[edchin, 2017-05-18 17:54:04]

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

[commit-bot: I haz the power, 2017-05-18 17:55:25]

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-05-18 23:09:41]

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

[commit-bot: I haz the power, 2017-05-18 23:09:42]

Dry run: This issue passed the CQ dry run.

[edchin, 2017-05-19 00:13:28]

The CQ bit was checked by edchin@chromium.org

[edchin, 2017-05-19 00:13:28]

The patchset sent to the CQ was uploaded after l-g-t-m from sczs@chromium.org
Link to the patchset: https://codereview.chromium.org/2885983003/#ps100001
(title: "Address comments")

[commit-bot: I haz the power, 2017-05-19 00:15:04]

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-05-19 00:24:40]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1495152808186200,
"parent_rev": "1c72320950b6ac44b11399af2c07691e354921b2", "commit_rev":
"66fd39bba716cb6aa352b9274a54333af05b5322"}

[commit-bot: I haz the power, 2017-05-19 00:24:50]

Description was changed from

==========
[ios] TabCollectionItem

In this CL, the collection view controller no longer uses an external 
data source protocol. The collection view controller now has its
own model, which is an array of TabCollectionItem. 

BUG=686770
==========

to

==========
[ios] TabCollectionItem

In this CL, the collection view controller no longer uses an external 
data source protocol. The collection view controller now has its
own model, which is an array of TabCollectionItem. 

BUG=686770

Review-Url: https://codereview.chromium.org/2885983003
Cr-Commit-Position: refs/heads/master@{#472991}
Committed:
https://chromium.googlesource.com/chromium/src/+/66fd39bba716cb6aa352b9274a54...
==========

[commit-bot: I haz the power, 2017-05-19 00:24:51]

Committed patchset #2 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/66fd39bba716cb6aa352b9274a54...