Add methods to Reading List Model

Add methods to Reading List Model

This CL adds two methods to the reading list model.
One method to remove all entries for tests and a method to mark an entry as
unread.

BUG=656966
Committed: https://crrev.com/4487ea507dc46d6f7cee7dab314f939011c0c728
Cr-Commit-Position: refs/heads/master@{#426758}

[gambard, 2016-10-20 11:04:56]

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

[gambard, 2016-10-20 11:04:56]

PTAL.

[Olivier, 2016-10-20 11:25:39]

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model.h (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model.h:115: // Remove all reading
list entries. Used for tests only.
Removes
Do you need this in the public interface?

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model_impl.cc (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model_impl.cc:140: void
ReadingListModelImpl::MarkUnreadByURL(const GURL& url) {
new line

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model_impl.cc:148:
observer.ReadingListWillMoveEntry(this,
Comment on ReadingListWillMoveEntry is explicitly not handling this case.
So either change comment, rename method or use the remove and add notifications.

[gambard, 2016-10-20 11:51:35]

Thanks, PTAL.

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model.h (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model.h:115: // Remove all reading
list entries. Used for tests only.
On 2016/10/20 11:25:39, Olivier Robin wrote:
> Removes
> Do you need this in the public interface?

Done.
I need this for EG test. If you see a better place, please tell.

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model_impl.cc (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model_impl.cc:140: void
ReadingListModelImpl::MarkUnreadByURL(const GURL& url) {
On 2016/10/20 11:25:39, Olivier Robin wrote:
> new line

Done.

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model_impl.cc:148:
observer.ReadingListWillMoveEntry(this,
On 2016/10/20 11:25:39, Olivier Robin wrote:
> Comment on ReadingListWillMoveEntry is explicitly not handling this case.
> So either change comment, rename method or use the remove and add
notifications.

I changed the comment. I don't like using the remove/add notification.

[Olivier, 2016-10-20 13:38:03]

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model.h (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model.h:115: // Remove all reading
list entries. Used for tests only.
On 2016/10/20 11:51:35, gambard wrote:
> On 2016/10/20 11:25:39, Olivier Robin wrote:
> > Removes
> > Do you need this in the public interface?
> 
> Done.
> I need this for EG test. If you see a better place, please tell.

Unittests use directly ReadingListModelImpl, so you don't need the definition
here.
Alternatively, as there is nothing private in the method and you can add a
egtest helper that does this

while (model->unread_size()) {
 model_->RemoveEntryByUrl(GetUnreadEntryAtIndex(0).url());

[gambard, 2016-10-20 15:43:06]

Thanks, PTAL.

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
File ios/chrome/browser/reading_list/reading_list_model.h (right):

https://codereview.chromium.org/2434993002/diff/1/ios/chrome/browser/reading_...
ios/chrome/browser/reading_list/reading_list_model.h:115: // Remove all reading
list entries. Used for tests only.
On 2016/10/20 13:38:02, Olivier Robin wrote:
> On 2016/10/20 11:51:35, gambard wrote:
> > On 2016/10/20 11:25:39, Olivier Robin wrote:
> > > Removes
> > > Do you need this in the public interface?
> > 
> > Done.
> > I need this for EG test. If you see a better place, please tell.
> 
> Unittests use directly ReadingListModelImpl, so you don't need the definition
> here.
> Alternatively, as there is nothing private in the method and you can add a
> egtest helper that does this
> 
> while (model->unread_size()) {
>  model_->RemoveEntryByUrl(GetUnreadEntryAtIndex(0).url());
> 

Done.

[Olivier, 2016-10-21 08:59:55]

lgtm

[gambard, 2016-10-21 09:00:40]

The CQ bit was checked by gambard@chromium.org

[commit-bot: I haz the power, 2016-10-21 09:00:47]

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, 2016-10-21 09:29:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-21 13:28:25]

Description was changed from

==========
Add methods to Reading List Model

This CL adds two methods to the reading list model.
One method to remove all entries for tests and a method to mark an entry as
unread.

BUG=656966
==========

to

==========
Add methods to Reading List Model

This CL adds two methods to the reading list model.
One method to remove all entries for tests and a method to mark an entry as
unread.

BUG=656966

Committed: https://crrev.com/4487ea507dc46d6f7cee7dab314f939011c0c728
Cr-Commit-Position: refs/heads/master@{#426758}
==========

[commit-bot: I haz the power, 2016-10-21 13:28:26]

Patchset 4 (id:??) landed as
https://crrev.com/4487ea507dc46d6f7cee7dab314f939011c0c728
Cr-Commit-Position: refs/heads/master@{#426758}