Update provider on Reading List model update

Update provider on Reading List model update

When the Reading List model is updated, the provider notifies the service.

BUG=702241
Review-Url: https://codereview.chromium.org/2794853004
Cr-Commit-Position: refs/heads/master@{#462013}
Committed: https://chromium.googlesource.com/chromium/src/+/fea421c12c5499a4f6b04291a97705662cf41d1c

[gambard, 2017-04-03 10:50:21]

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

[gambard, 2017-04-03 10:50:22]

PTAL.

[Olivier, 2017-04-03 11:18:07]

https://codereview.chromium.org/2794853004/diff/1/components/ntp_snippets/rea...
File components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
(right):

https://codereview.chromium.org/2794853004/diff/1/components/ntp_snippets/rea...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:119:
DCHECK(model == reading_list_model_);
if (model->IsPerformingBatchUpdates())
     return;

or add it in FetchReadingListInternal

[gambard, 2017-04-03 11:25:06]

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

[gambard, 2017-04-03 11:25:07]

Thanks, PTAL.
treib@: the provider might notify the service OnNewSuggestions with the same
suggestions. I did not see anything in the API preventing this. WDYT?

https://codereview.chromium.org/2794853004/diff/1/components/ntp_snippets/rea...
File components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
(right):

https://codereview.chromium.org/2794853004/diff/1/components/ntp_snippets/rea...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:119:
DCHECK(model == reading_list_model_);
On 2017/04/03 11:18:07, Olivier Robin wrote:
> if (model->IsPerformingBatchUpdates())
>      return;
> 
> or add it in FetchReadingListInternal

Done.

[Olivier, 2017-04-03 13:13:53]

lgtm

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
File components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
(right):

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:148:
if (reading_list_model_->IsPerformingBatchUpdates())
super nit: group with the if above.

[gambard, 2017-04-04 16:15:08]

treib@: ping!

[Marc Treib, 2017-04-04 16:57:49]

Sorry, I missed this one. LGTM!

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
File components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
(right):

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:148:
if (reading_list_model_->IsPerformingBatchUpdates())
nit: Braces please, also above

[gambard, 2017-04-05 08:08:46]

The CQ bit was checked by gambard@chromium.org

[gambard, 2017-04-05 08:08:47]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org,
olivierrobin@chromium.org
Link to the patchset: https://codereview.chromium.org/2794853004/#ps40001
(title: "Address comments")

[gambard, 2017-04-05 08:08:55]

Thanks!

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
File components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc
(right):

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:148:
if (reading_list_model_->IsPerformingBatchUpdates())
On 2017/04/04 16:57:49, Marc Treib wrote:
> nit: Braces please, also above

Done.

https://codereview.chromium.org/2794853004/diff/20001/components/ntp_snippets...
components/ntp_snippets/reading_list/reading_list_suggestions_provider.cc:148:
if (reading_list_model_->IsPerformingBatchUpdates())
On 2017/04/03 13:13:53, Olivier Robin wrote:
> super nit: group with the if above.

Done.

[commit-bot: I haz the power, 2017-04-05 08:08:59]

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-05 08:57:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491379726681290,
"parent_rev": "68e4a897f7551cf7c603cb9ad1fbf534da856c91", "commit_rev":
"fea421c12c5499a4f6b04291a97705662cf41d1c"}

[commit-bot: I haz the power, 2017-04-05 08:58:09]

Description was changed from

==========
Update provider on Reading List model update

When the Reading List model is updated, the provider notifies the service.

BUG=702241
==========

to

==========
Update provider on Reading List model update

When the Reading List model is updated, the provider notifies the service.

BUG=702241

Review-Url: https://codereview.chromium.org/2794853004
Cr-Commit-Position: refs/heads/master@{#462013}
Committed:
https://chromium.googlesource.com/chromium/src/+/fea421c12c5499a4f6b04291a977...
==========

[commit-bot: I haz the power, 2017-04-05 08:58:10]

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