Suggestions UI - Add Data Source and Item

Suggestions UI - Add Data Source and Item

BUG=679369
Review-Url: https://codereview.chromium.org/2619963004
Cr-Commit-Position: refs/heads/master@{#443215}
Committed: https://chromium.googlesource.com/chromium/src/+/184a5d740d8a272e114007a7822d4b5640ace354

[gambard, 2017-01-10 13:00:57]

gambard@chromium.org changed reviewers:
+ lpromero@chromium.org, marq@chromium.org

[gambard, 2017-01-10 13:00:58]

PTAL.
Split 2/4 of https://codereview.chromium.org/2618343002/

[lpromero, 2017-01-11 09:47:05]

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.h (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:13: // handling the
items addition and removal. The items can be added before the
Is removal in one of the follow up CLs?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:14: //
CollectionViewController is set.
Is this really useful? It adds a lot of complexity in the impl.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:18: //
CollectionViewController will add the currents items in the DataSource to the
s/currents/current?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:23: // |section|. If
|section| > the current number of section, it will add a new
Avoid ">", make full text sentences.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:24: // section at
the end.
In the impl, passing a section more than "count" would crash. I'd be more
defensive in which section can be passed in.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:27:
toSection:(NSUInteger)section;
Add new line.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:25:
NSMutableArray<NSMutableArray<CollectionViewItem*>*>* _items;
No action needed: Hmm, you work around the fact that CollectionViewController's
collectionViewModel is readonly? I wonder if we should make it settable (and
remove the loadModel method).

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:33: _items =
[NSMutableArray arrayWithCapacity:1];
I usually don't try to give clues like that, especially in your case where this
array can get easily more objects. I just use [NSMutableArray array].

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:40: subtitle:@"The
subtitle"]];
Maybe remove this bogus data?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:88:
completion:^(BOOL){
Can't you just pass nil?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:92: [[_items
objectAtIndex:section] addObject:item];
Isn't this already done line 80? Plus, if section was more than _items.count,
there is no object at that index right?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:102:
completion:^(BOOL){
Idem, can you pass nil?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_item.h (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:11: // Item for a
suggestion item with a simple title and subtitle.
s/suggestion/suggestions

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:11: // Item for a
suggestion item with a simple title and subtitle.
What is simple?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:14: // Init a suggestion
item with a |title| and a |subtitle|. |type| is the type of
suggestions

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:26: @property(nonatomic,
strong) NSString* title;
Should be copy, here and below.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_item.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:26: _title = title;
You should [title copy] these. To be more idiomatic, I would make properties out
of these ivars. With the copy attribute.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:63: UIStackView*
labelsStack = [[UIStackView alloc]
I haven't been always happy with stack views in self sizing cells. If that
becomes an issue, prefer using constraints directly.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:82: 
Remove empty line.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:93: [_titleButton
setTitle:title forState:UIControlStateNormal];
Actually, do you need to store a local copy of the strings? Can't you just
provide a wrapper around the button and label one?

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:94: _title = title;
Should be [title copy].

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:99: _detailText =
detailText;
Idem, should be [detailText copy].

https://codereview.chromium.org/2619963004/diff/40001/ios/showcase/suggestion...
File ios/showcase/suggestions/sc_suggestions_coordinator.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/showcase/suggestion...
ios/showcase/suggestions/sc_suggestions_coordinator.mm:31:
_dataSource.collectionViewController = suggestion;
Since you set the collectionviewcontroller here, why not removing support for
allowing a data source without collectionviewcontroller? It will simplify
greatly.

[marq (ping after 24h), 2017-01-11 13:43:09]

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.h (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:15: @interface
SuggestionsDataSource : NSObject
The name 'DataSource" here is confusing, since that's usually the name of a
protocol that a collection view uses.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:20:
@property(nonatomic, strong) CollectionViewController* collectionViewController;
I don't think this object should be owning the view controller. That doesn't
seem like part of a "data source" object's job.

I also don't really think this pattern is right. I would expect that a data
source object would provide objects to a view controller, so I would expect it
to be a property of the view controller, not the other way around.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:17: ItemTypeText =
kItemTypeEnumZero,
Chromium style is to use all caps ("MACRO_STYLE") for enum values -- see
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md....

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_item.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:26: _title = title;
On 2017/01/11 09:47:05, lpromero wrote:
> You should [title copy] these. To be more idiomatic, I would make properties
out
> of these ivars. With the copy attribute.

+1 to properties, but you shouldn't use this class's properties in its init, so
you will still need to do _title = [title copy];

[lpromero, 2017-01-11 13:46:05]

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:17: ItemTypeText =
kItemTypeEnumZero,
On 2017/01/11 13:43:09, marq wrote:
> Chromium style is to use all caps ("MACRO_STYLE") for enum values -- see
>
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md....

This is outside the scope of this CL, as this is the style used pervasively for
ItemType.

[gambard, 2017-01-11 15:10:38]

Thanks, PTAL.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.h (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:13: // handling the
items addition and removal. The items can be added before the
On 2017/01/11 09:47:04, lpromero wrote:
> Is removal in one of the follow up CLs?

Not yet. I will remove it for now.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:14: //
CollectionViewController is set.
On 2017/01/11 09:47:04, lpromero wrote:
> Is this really useful? It adds a lot of complexity in the impl.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:15: @interface
SuggestionsDataSource : NSObject
On 2017/01/11 13:43:09, marq wrote:
> The name 'DataSource" here is confusing, since that's usually the name of a
> protocol that a collection view uses. 

I have changed the architecture.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:18: //
CollectionViewController will add the currents items in the DataSource to the
On 2017/01/11 09:47:04, lpromero wrote:
> s/currents/current?

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:23: // |section|. If
|section| > the current number of section, it will add a new
On 2017/01/11 09:47:04, lpromero wrote:
> Avoid ">", make full text sentences.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:24: // section at
the end.
On 2017/01/11 09:47:04, lpromero wrote:
> In the impl, passing a section more than "count" would crash. I'd be more
> defensive in which section can be passed in.

It should not. With the current implementation it works.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.h:27:
toSection:(NSUInteger)section;
On 2017/01/11 09:47:04, lpromero wrote:
> Add new line.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:17: ItemTypeText =
kItemTypeEnumZero,
On 2017/01/11 13:46:05, lpromero wrote:
> On 2017/01/11 13:43:09, marq wrote:
> > Chromium style is to use all caps ("MACRO_STYLE") for enum values -- see
> >
>
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md....
> 
> This is outside the scope of this CL, as this is the style used pervasively
for
> ItemType.

Acknowledged.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:25:
NSMutableArray<NSMutableArray<CollectionViewItem*>*>* _items;
On 2017/01/11 09:47:04, lpromero wrote:
> No action needed: Hmm, you work around the fact that
CollectionViewController's
> collectionViewModel is readonly? I wonder if we should make it settable (and
> remove the loadModel method).

This was to allow the addition of items before setting the
CollectionViewController. I removed it as I force a CollectionViewController to
be set in the init.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:33: _items =
[NSMutableArray arrayWithCapacity:1];
On 2017/01/11 09:47:04, lpromero wrote:
> I usually don't try to give clues like that, especially in your case where
this
> array can get easily more objects. I just use [NSMutableArray array].

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:40: subtitle:@"The
subtitle"]];
On 2017/01/11 09:47:04, lpromero wrote:
> Maybe remove this bogus data?

Why? This is still a demo. It shows what kind of items can be displayed.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:88:
completion:^(BOOL){
On 2017/01/11 09:47:04, lpromero wrote:
> Can't you just pass nil?

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:92: [[_items
objectAtIndex:section] addObject:item];
On 2017/01/11 09:47:04, lpromero wrote:
> Isn't this already done line 80? Plus, if section was more than _items.count,
> there is no object at that index right?

Acknowledged.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:102:
completion:^(BOOL){
On 2017/01/11 09:47:04, lpromero wrote:
> Idem, can you pass nil?

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_item.h (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:11: // Item for a
suggestion item with a simple title and subtitle.
On 2017/01/11 09:47:05, lpromero wrote:
> What is simple?

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:11: // Item for a
suggestion item with a simple title and subtitle.
On 2017/01/11 09:47:05, lpromero wrote:
> s/suggestion/suggestions

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:14: // Init a suggestion
item with a |title| and a |subtitle|. |type| is the type of
On 2017/01/11 09:47:05, lpromero wrote:
> suggestions

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.h:26: @property(nonatomic,
strong) NSString* title;
On 2017/01/11 09:47:05, lpromero wrote:
> Should be copy, here and below.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_item.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:26: _title = title;
On 2017/01/11 09:47:05, lpromero wrote:
> You should [title copy] these. To be more idiomatic, I would make properties
out
> of these ivars. With the copy attribute.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:26: _title = title;
On 2017/01/11 13:43:09, marq wrote:
> On 2017/01/11 09:47:05, lpromero wrote:
> > You should [title copy] these. To be more idiomatic, I would make properties
> out
> > of these ivars. With the copy attribute.
> 
> +1 to properties, but you shouldn't use this class's properties in its init,
so
> you will still need to do _title = [title copy];

Acknowledged.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:63: UIStackView*
labelsStack = [[UIStackView alloc]
On 2017/01/11 09:47:05, lpromero wrote:
> I haven't been always happy with stack views in self sizing cells. If that
> becomes an issue, prefer using constraints directly.

Acknowledged.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:82: 
On 2017/01/11 09:47:05, lpromero wrote:
> Remove empty line.

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:93: [_titleButton
setTitle:title forState:UIControlStateNormal];
On 2017/01/11 09:47:05, lpromero wrote:
> Actually, do you need to store a local copy of the strings? Can't you just
> provide a wrapper around the button and label one?

Done.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:94: _title = title;
On 2017/01/11 09:47:05, lpromero wrote:
> Should be [title copy].

Removed.

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_item.mm:99: _detailText =
detailText;
On 2017/01/11 09:47:05, lpromero wrote:
> Idem, should be [detailText copy].

Removed.

https://codereview.chromium.org/2619963004/diff/40001/ios/showcase/suggestion...
File ios/showcase/suggestions/sc_suggestions_coordinator.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/showcase/suggestion...
ios/showcase/suggestions/sc_suggestions_coordinator.mm:31:
_dataSource.collectionViewController = suggestion;
On 2017/01/11 09:47:05, lpromero wrote:
> Since you set the collectionviewcontroller here, why not removing support for
> allowing a data source without collectionviewcontroller? It will simplify
> greatly.

Done.

[lpromero, 2017-01-11 18:54:43]

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:92: [[_items
objectAtIndex:section] addObject:item];
On 2017/01/11 15:10:37, gambard wrote:
> On 2017/01/11 09:47:04, lpromero wrote:
> > Isn't this already done line 80? Plus, if section was more than
_items.count,
> > there is no object at that index right?
> 
> Acknowledged.

I don't understand how this doesn't crash if you pass a section that is 10 more
than the initial count for example.

[gambard, 2017-01-12 08:46:16]

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:92: [[_items
objectAtIndex:section] addObject:item];
On 2017/01/11 18:54:43, lpromero wrote:
> On 2017/01/11 15:10:37, gambard wrote:
> > On 2017/01/11 09:47:04, lpromero wrote:
> > > Isn't this already done line 80? Plus, if section was more than
> _items.count,
> > > there is no object at that index right?
> > 
> > Acknowledged.
> 
> I don't understand how this doesn't crash if you pass a section that is 10
more
> than the initial count for example.

If you look at line 59 of the patch 5, there is if ([model numberOfSections] <=
inputSection) which adds a new section and changes the section index to the one
of the new section.

[lpromero, 2017-01-12 11:43:35]

lgtm

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
File ios/chrome/browser/ui/suggestions/suggestions_data_source.mm (right):

https://codereview.chromium.org/2619963004/diff/40001/ios/chrome/browser/ui/s...
ios/chrome/browser/ui/suggestions/suggestions_data_source.mm:92: [[_items
objectAtIndex:section] addObject:item];
On 2017/01/12 08:46:16, gambard wrote:
> On 2017/01/11 18:54:43, lpromero wrote:
> > On 2017/01/11 15:10:37, gambard wrote:
> > > On 2017/01/11 09:47:04, lpromero wrote:
> > > > Isn't this already done line 80? Plus, if section was more than
> > _items.count,
> > > > there is no object at that index right?
> > > 
> > > Acknowledged.
> > 
> > I don't understand how this doesn't crash if you pass a section that is 10
> more
> > than the initial count for example.
> 
> If you look at line 59 of the patch 5, there is if ([model numberOfSections]
<=
> inputSection) which adds a new section and changes the section index to the
one
> of the new section.

Fair enough.

[gambard, 2017-01-12 11:58:12]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-01-12 11:58:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
lpromero@chromium.org
Link to the patchset: https://codereview.chromium.org/2619963004/#ps100001
(title: "Change to property")

[commit-bot: I haz the power, 2017-01-12 11:58: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-01-12 12:53:46]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484222292051140,
"parent_rev": "71fc94ca2e7688389d7d7de63ebf67cc3284a30f", "commit_rev":
"184a5d740d8a272e114007a7822d4b5640ace354"}

[commit-bot: I haz the power, 2017-01-12 12:54:17]

Description was changed from

==========
Suggestions UI - Add Data Source and Item

BUG=679369
==========

to

==========
Suggestions UI - Add Data Source and Item

BUG=679369

Review-Url: https://codereview.chromium.org/2619963004
Cr-Commit-Position: refs/heads/master@{#443215}
Committed:
https://chromium.googlesource.com/chromium/src/+/184a5d740d8a272e114007a7822d...
==========

[commit-bot: I haz the power, 2017-01-12 12:54:18]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/184a5d740d8a272e114007a7822d...

[Wez, 2017-02-07 19:44:46]

Hallo marq@chromium.org!
Due to a depot_tools patch which mistakenly removed the OWNERS check for
non-source files (see crbug.com/684270), the following files landed in this CL
and need a retrospective rubber-stamp review from you:
	ios/chrome/browser/ui/suggestions/BUILD.gn
Thanks,
Wez