Cleanup: Remove CategoryStatus::SIGNED_OUT

Cleanup: Remove CategoryStatus::SIGNED_OUT

BUG=699979
Review-Url: https://codereview.chromium.org/2740783004
Cr-Commit-Position: refs/heads/master@{#456073}
Committed: https://chromium.googlesource.com/chromium/src/+/ce0e63223261f496b2721a83c42aae8b163da960

[Marc Treib, 2017-03-09 16:52:23]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 16:52:57]

Dry run: 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-09 18:09:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 18:09:04]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Marc Treib, 2017-03-10 13:02:20]

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

[Marc Treib, 2017-03-10 13:02:21]

PTAL, in particular at the Java test changes.

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
(right):

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:288:
assertItemsFor(section(6));
This fails, because the all-dismissed-item is still there. Is that a bug? Or is
it intended that we can't get out of the all-dismissed state without explicit
user action? (If so, then we can't really test this anymore, because there isn't
a status anymore that'll remove suggestions, but leave the section around.)

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:318:
// When the category gets disabled, the section should go away and not load any
suggestions.
Moved due to the same reason as above.

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:740:
verify(dataObserver, times(2)).onItemRangeChanged(2, 1, null); // Spacer refresh
Why does this happen twice? Bug, expected, or unexpected but "oh well"?

[dgn, 2017-03-10 13:51:30]

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
(right):

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:288:
assertItemsFor(section(6));
On 2017/03/10 13:02:21, Marc Treib wrote:
> This fails, because the all-dismissed-item is still there. Is that a bug? Or
is
> it intended that we can't get out of the all-dismissed state without explicit
> user action? (If so, then we can't really test this anymore, because there
isn't
> a status anymore that'll remove suggestions, but leave the section around.)

The closest thing to "reset section" through the suggestionsSource observer is
#onFullRefreshRequired (instead of setStatusForCategory). That is also the
method we use to clear the suggestions when the history is cleared so it would
be a good replacement to have a test around that.

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:318:
// When the category gets disabled, the section should go away and not load any
suggestions.
On 2017/03/10 13:02:21, Marc Treib wrote:
> Moved due to the same reason as above.

Does it work keeping the same order as before but with NOT_PROVIDED ? it does
not remove the section so it might be good enough.

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:740:
verify(dataObserver, times(2)).onItemRangeChanged(2, 1, null); // Spacer refresh
On 2017/03/10 13:02:21, Marc Treib wrote:
> Why does this happen twice? Bug, expected, or unexpected but "oh well"?

it's refreshed once when we remove the section, then another time when the
allDismissedItem is added.

[Marc Treib, 2017-03-10 14:37:11]

The CQ bit was checked by treib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 14:37:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marc Treib, 2017-03-10 14:37:20]

PTAL again!

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
File
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
(right):

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:288:
assertItemsFor(section(6));
On 2017/03/10 13:51:30, dgn wrote:
> On 2017/03/10 13:02:21, Marc Treib wrote:
> > This fails, because the all-dismissed-item is still there. Is that a bug? Or
> is
> > it intended that we can't get out of the all-dismissed state without
explicit
> > user action? (If so, then we can't really test this anymore, because there
> isn't
> > a status anymore that'll remove suggestions, but leave the section around.)
> 
> The closest thing to "reset section" through the suggestionsSource observer is
> #onFullRefreshRequired (instead of setStatusForCategory). That is also the
> method we use to clear the suggestions when the history is cleared so it would
> be a good replacement to have a test around that.

Done. (I think - it passes now, but please double-check that it actually makes
sense :) )

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:318:
// When the category gets disabled, the section should go away and not load any
suggestions.
On 2017/03/10 13:51:30, dgn wrote:
> On 2017/03/10 13:02:21, Marc Treib wrote:
> > Moved due to the same reason as above.
> 
> Does it work keeping the same order as before but with NOT_PROVIDED ? it does
> not remove the section so it might be good enough.

Yup, that works! (with slightly different expectations, since NOT_PROVIDED
doesn't result in a status card, but I guess the main thing we're testing is
still there)

https://codereview.chromium.org/2740783004/diff/20001/chrome/android/junit/sr...
chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java:740:
verify(dataObserver, times(2)).onItemRangeChanged(2, 1, null); // Spacer refresh
On 2017/03/10 13:51:30, dgn wrote:
> On 2017/03/10 13:02:21, Marc Treib wrote:
> > Why does this happen twice? Bug, expected, or unexpected but "oh well"?
> 
> it's refreshed once when we remove the section, then another time when the
> allDismissedItem is added.

Thanks! Added a comment.

[dgn, 2017-03-10 14:56:59]

Thanks! lgtm

[commit-bot: I haz the power, 2017-03-10 15:53:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-10 15:53:19]

Dry run: This issue passed the CQ dry run.

[Marc Treib, 2017-03-10 16:05:14]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2017-03-10 16:05:31]

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-10 16:11:56]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489161914836160,
"parent_rev": "f351604c9dd3faeedb53ac99e71c7594414bd19a", "commit_rev":
"ce0e63223261f496b2721a83c42aae8b163da960"}

[commit-bot: I haz the power, 2017-03-10 16:12:44]

Description was changed from

==========
Cleanup: Remove CategoryStatus::SIGNED_OUT

BUG=699979
==========

to

==========
Cleanup: Remove CategoryStatus::SIGNED_OUT

BUG=699979

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

[commit-bot: I haz the power, 2017-03-10 16:12:45]

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