Add Unseen state to Reading List Entry

Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN

BUG=671185
Committed: https://crrev.com/bc9d2d7b3acde39d666adca91fdb897535455b5a
Cr-Commit-Position: refs/heads/master@{#436563}

[Olivier, 2016-12-05 17:34:01]

Description was changed from

==========
done

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

to

==========
Add Unseen state to Reading List Entry

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

[Olivier, 2016-12-05 18:23:46]

Description was changed from

==========
Add Unseen state to Reading List Entry

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

to

==========
Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is 
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN 

BUG=671185
==========

[Olivier, 2016-12-05 18:23:58]

olivierrobin@chromium.org changed reviewers:
+ jif@chromium.org, pavely@chromium.org

[pavely, 2016-12-05 19:10:31]

lgtm

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
File components/reading_list/ios/reading_list_entry.cc (right):

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
components/reading_list/ios/reading_list_entry.cc:222: State state = UNSEEN;
Absent status is the same as UNSEEN. Is it worth adding comment in proto file
that they are treated the same?

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
components/reading_list/ios/reading_list_entry.cc:338: if (state_== READ) {
When handling enum values, it is better to to use switch statement,makes it
easier to read (no chance of seeing unexpected condition in one of the "if"s in
the middle). It also helps you to not forget to update code when set of values
changes.

[jif-google, 2016-12-05 19:24:59]

jif@google.com changed reviewers:
+ jif@google.com

[jif-google, 2016-12-05 19:25:00]

lgtm

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
File components/reading_list/ios/reading_list_entry.h (right):

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
components/reading_list/ios/reading_list_entry.h:64: // Returns if ean entry has
ever been seen.
typo

[Olivier, 2016-12-06 09:59:03]

Patchset #3 (id:40001) has been deleted

[Olivier, 2016-12-06 09:59:29]

Thanks

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
File components/reading_list/ios/reading_list_entry.cc (right):

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
components/reading_list/ios/reading_list_entry.cc:222: State state = UNSEEN;
On 2016/12/05 19:10:31, pavely wrote:
> Absent status is the same as UNSEEN. Is it worth adding comment in proto file
> that they are treated the same?

Done.

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
File components/reading_list/ios/reading_list_entry.h (right):

https://codereview.chromium.org/2557443003/diff/20001/components/reading_list...
components/reading_list/ios/reading_list_entry.h:64: // Returns if ean entry has
ever been seen.
On 2016/12/05 19:24:59, jif-google wrote:
> typo

Done.

[Olivier, 2016-12-06 09:59:33]

The CQ bit was checked by olivierrobin@chromium.org

[Olivier, 2016-12-06 09:59:33]

The patchset sent to the CQ was uploaded after l-g-t-m from jif@google.com,
pavely@chromium.org
Link to the patchset: https://codereview.chromium.org/2557443003/#ps60001
(title: "feedback")

[commit-bot: I haz the power, 2016-12-06 09:59:49]

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-12-06 10:40:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481018373018470,
"parent_rev": "d17e4722935ffcaf0c488d505447187d9f9f8cdb", "commit_rev":
"bff4ae45bb4d9a83150cc7f1b242f77525bb11f4"}

[commit-bot: I haz the power, 2016-12-06 10:41:16]

Description was changed from

==========
Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is 
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN 

BUG=671185
==========

to

==========
Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is 
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN 

BUG=671185
==========

[commit-bot: I haz the power, 2016-12-06 10:41:17]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-12-06 10:43:41]

Description was changed from

==========
Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is 
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN 

BUG=671185
==========

to

==========
Add Unseen state to Reading List Entry

The new logic requires a flag in each entry, and this flag must be synced.
Add a state in ReadingListEntry.
When an entry is created, it is UNSEEN.
It cannot be set back to this state.

The logic is
IsRead() := state_ == READ
HasBeenSeen() := state_ != UNSEEN

BUG=671185

Committed: https://crrev.com/bc9d2d7b3acde39d666adca91fdb897535455b5a
Cr-Commit-Position: refs/heads/master@{#436563}
==========

[commit-bot: I haz the power, 2016-12-06 10:43:42]

Patchset 3 (id:??) landed as
https://crrev.com/bc9d2d7b3acde39d666adca91fdb897535455b5a
Cr-Commit-Position: refs/heads/master@{#436563}