Fetch Suggestions by category

Fetch Suggestions by category

This CL changes the behavior of the ContentSuggestions fetching.
Instead of throwing away all the suggestions and reloading the new ones, the
data sink only loads new suggestions if they are in an empty category.

BUG=699124
Review-Url: https://codereview.chromium.org/2751713002
Cr-Commit-Position: refs/heads/master@{#456725}
Committed: https://chromium.googlesource.com/chromium/src/+/8401d78afcd1cf80cbe547f597b3bad9edf83694

[gambard, 2017-03-14 15:07:54]

gambard@chromium.org changed reviewers:
+ stkhapugin@chromium.org

[gambard, 2017-03-14 15:07:55]

PTAL.

[stkhapugin, 2017-03-14 15:25:12]

lgtm with comment about mutable array

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/cont...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
(right):

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/cont...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:256:
toArray:(NSMutableArray<ContentSuggestion*>*)contentArray {
I think you can replace the |toArray| with a return value of NSArray, which has
an additional benefit of not passing around mutable arrays (which is bad
practice because concurrency etc)

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
File ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
(right):

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h:11:
@class ContentSuggestionsSectionInformation;
move to line 9 to group @class

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h:26:
- (NSArray<ContentSuggestion*>*)suggestionsForSection:
Consider nullability identifiers in this section :)

[gambard, 2017-03-14 16:05:30]

Thanks!

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/cont...
File ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
(right):

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/cont...
ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm:256:
toArray:(NSMutableArray<ContentSuggestion*>*)contentArray {
On 2017/03/14 15:25:12, stkhapugin wrote:
> I think you can replace the |toArray| with a return value of NSArray, which
has
> an additional benefit of not passing around mutable arrays (which is bad
> practice because concurrency etc)

I am calling this method multiple times with the same array (see line 167). So I
am keeping it this way.

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
File ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
(right):

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h:11:
@class ContentSuggestionsSectionInformation;
On 2017/03/14 15:25:12, stkhapugin wrote:
> move to line 9 to group @class

Done.

https://codereview.chromium.org/2751713002/diff/20001/ios/chrome/browser/ui/c...
ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h:26:
- (NSArray<ContentSuggestion*>*)suggestionsForSection:
On 2017/03/14 15:25:12, stkhapugin wrote:
> Consider nullability identifiers in this section :) 

Done :)

[gambard, 2017-03-14 16:05:33]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-03-14 16:05:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
stkhapugin@chromium.org
Link to the patchset: https://codereview.chromium.org/2751713002/#ps40001
(title: "git cl web")

[commit-bot: I haz the power, 2017-03-14 16:06:24]

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-03-14 16:27:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489507533935320,
"parent_rev": "5b069f0bfe05879dc2dfbc32d9b4b171c622d5c3", "commit_rev":
"8401d78afcd1cf80cbe547f597b3bad9edf83694"}

[commit-bot: I haz the power, 2017-03-14 16:27:47]

Description was changed from

==========
Fetch Suggestions by category

This CL changes the behavior of the ContentSuggestions fetching.
Instead of throwing away all the suggestions and reloading the new ones, the
data sink only loads new suggestions if they are in an empty category.

BUG=699124
==========

to

==========
Fetch Suggestions by category

This CL changes the behavior of the ContentSuggestions fetching.
Instead of throwing away all the suggestions and reloading the new ones, the
data sink only loads new suggestions if they are in an empty category.

BUG=699124

Review-Url: https://codereview.chromium.org/2751713002
Cr-Commit-Position: refs/heads/master@{#456725}
Committed:
https://chromium.googlesource.com/chromium/src/+/8401d78afcd1cf80cbe547f597b3...
==========

[commit-bot: I haz the power, 2017-03-14 16:27:48]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8401d78afcd1cf80cbe547f597b3...