Extract a DataSource interface for items shown in a status card.

Extract a DataSource interface for items shown in a status card.

This gets rid of some of the awkwardness in the class hierarchy for
SignInPromoItem (which implements ItemGroup but also indirectly inherits from
NewTabPageItem).

BUG=616090
Committed: https://crrev.com/281c15dfc08bd0a1b1d3972c5b27784ce5e25020
Cr-Commit-Position: refs/heads/master@{#424770}

[Bernhard Bauer, 2016-10-11 15:03:20]

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

[commit-bot: I haz the power, 2016-10-11 15:03:29]

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

[Bernhard Bauer, 2016-10-11 15:26:42]

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

[commit-bot: I haz the power, 2016-10-11 15:26:59]

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

[Bernhard Bauer, 2016-10-11 15:46:22]

bauerb@chromium.org changed reviewers:
+ dgn@chromium.org, mvanouwerkerk@chromium.org

[Bernhard Bauer, 2016-10-11 15:46:23]

Please review. Thanks!

[commit-bot: I haz the power, 2016-10-11 15:59:32]

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

[commit-bot: I haz the power, 2016-10-11 15:59:32]

Dry run: This issue passed the CQ dry run.

[Michael van Ouwerkerk, 2016-10-11 16:01:16]

lgtm

[dgn, 2016-10-11 16:43:44]

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
(right):

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java:28:
public class SigninPromoItem implements ItemGroup,
StatusCardViewHolder.DataSource {
It's a bit strange that it's called SigninPromoITEM now that it's not a NtpItem
anymore. 

Also, our view holders logically hold a NtpItem, while the StatusCardViewHolder
now holds a DataSource... How about having another interface extending NtpItem
to replace DataSource?

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java
(right):

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java:49:
* @return Resource ID for the action label string, or 0 if the card does not
have a label.
s/a label/an action

Or something more explicit saying that no action button will be shown if we
return 0?

[Bernhard Bauer, 2016-10-11 16:50:24]

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
(right):

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java:28:
public class SigninPromoItem implements ItemGroup,
StatusCardViewHolder.DataSource {
On 2016/10/11 16:43:43, dgn wrote:
> It's a bit strange that it's called SigninPromoITEM now that it's not a
NtpItem
> anymore. 
> 
> Also, our view holders logically hold a NtpItem, while the
StatusCardViewHolder
> now holds a DataSource... How about having another interface extending NtpItem
> to replace DataSource?

Weelll... in the longer term I actually want to get rid of NTPItem. The other
ViewHolders are then going to hold on to the data item (SnippetArticle)
directly, if they have any at all.

Should I just rename this to SigninPromo?

[dgn, 2016-10-11 17:00:40]

ltgm after the comment update.

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
(right):

https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java:28:
public class SigninPromoItem implements ItemGroup,
StatusCardViewHolder.DataSource {
On 2016/10/11 16:50:24, Bernhard Bauer wrote:
> On 2016/10/11 16:43:43, dgn wrote:
> > It's a bit strange that it's called SigninPromoITEM now that it's not a
> NtpItem
> > anymore. 
> > 
> > Also, our view holders logically hold a NtpItem, while the
> StatusCardViewHolder
> > now holds a DataSource... How about having another interface extending
NtpItem
> > to replace DataSource?
> 
> Weelll... in the longer term I actually want to get rid of NTPItem. The other
> ViewHolders are then going to hold on to the data item (SnippetArticle)
> directly, if they have any at all.
> 
> Should I just rename this to SigninPromo?

lgtm for Sign[Ii]nPromo. (the capitalisation there is inconsistent across the
code, I noticed after landing my initial CL :/)

Okay if we are going to remove the NtpItems.

[Bernhard Bauer, 2016-10-12 09:54:43]

On 2016/10/11 17:00:40, dgn wrote:
> ltgm after the comment update.
> 
>
https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java
> (right):
> 
>
https://codereview.chromium.org/2407283002/diff/20001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SigninPromoItem.java:28:
> public class SigninPromoItem implements ItemGroup,
> StatusCardViewHolder.DataSource {
> On 2016/10/11 16:50:24, Bernhard Bauer wrote:
> > On 2016/10/11 16:43:43, dgn wrote:
> > > It's a bit strange that it's called SigninPromoITEM now that it's not a
> > NtpItem
> > > anymore. 
> > > 
> > > Also, our view holders logically hold a NtpItem, while the
> > StatusCardViewHolder
> > > now holds a DataSource... How about having another interface extending
> NtpItem
> > > to replace DataSource?
> > 
> > Weelll... in the longer term I actually want to get rid of NTPItem. The
other
> > ViewHolders are then going to hold on to the data item (SnippetArticle)
> > directly, if they have any at all.
> > 
> > Should I just rename this to SigninPromo?
> 
> lgtm for Sign[Ii]nPromo. (the capitalisation there is inconsistent across the
> code, I noticed after landing my initial CL :/)
> 
> Okay if we are going to remove the NtpItems.

OK, renamed to SignInPromo (I think SignIn is more common, and at the moment we
can't land a capitalization-only rename, because that might confuse systems with
a case-insensitive filesystem, so I'd rather take this opportunity to fix the
capitalization).

[Bernhard Bauer, 2016-10-12 09:54:56]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-10-12 09:54:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407283002/#ps40001
(title: "rename")

[commit-bot: I haz the power, 2016-10-12 09:55:22]

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-12 10:15:44]

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

[commit-bot: I haz the power, 2016-10-12 10:15:45]

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

[Bernhard Bauer, 2016-10-12 12:23:39]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-10-12 12:23:39]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407283002/#ps60001
(title: "fix")

[commit-bot: I haz the power, 2016-10-12 12:23:56]

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-12 12:25:43]

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

[commit-bot: I haz the power, 2016-10-12 12:25:44]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Bernhard Bauer, 2016-10-12 13:16:23]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-10-12 13:16:23]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407283002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-10-12 13:16:29]

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-12 13:26:08]

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

[commit-bot: I haz the power, 2016-10-12 13:26:09]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Bernhard Bauer, 2016-10-12 14:03:03]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-10-12 14:03:03]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407283002/#ps90007
(title: "argh")

[commit-bot: I haz the power, 2016-10-12 14:03:13]

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-12 14:27:11]

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

[commit-bot: I haz the power, 2016-10-12 14:27:12]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Bernhard Bauer, 2016-10-12 15:28:23]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2016-10-12 15:28:23]

The patchset sent to the CQ was uploaded after l-g-t-m from dgn@chromium.org,
mvanouwerkerk@chromium.org
Link to the patchset: https://codereview.chromium.org/2407283002/#ps110001
(title: "blergh")

[commit-bot: I haz the power, 2016-10-12 15:28:42]

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-12 15:37:43]

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

[commit-bot: I haz the power, 2016-10-12 15:37:45]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Bernhard Bauer, 2016-10-12 15:53:14]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2016-10-12 15:53:36]

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-12 16:04:11]

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

[commit-bot: I haz the power, 2016-10-12 16:04:13]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Bernhard Bauer, 2016-10-12 17:09:27]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2016-10-12 17:09:44]

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-12 17:20:12]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2016-10-12 17:49:25]

Description was changed from

==========
Extract a DataSource interface for items shown in a status card.

This gets rid of some of the awkwardness in the class hierarchy for
SignInPromoItem (which implements ItemGroup but also indirectly inherits from
NewTabPageItem).

BUG=616090
==========

to

==========
Extract a DataSource interface for items shown in a status card.

This gets rid of some of the awkwardness in the class hierarchy for
SignInPromoItem (which implements ItemGroup but also indirectly inherits from
NewTabPageItem).

BUG=616090

Committed: https://crrev.com/281c15dfc08bd0a1b1d3972c5b27784ce5e25020
Cr-Commit-Position: refs/heads/master@{#424770}
==========

[commit-bot: I haz the power, 2016-10-12 17:49:27]

Patchset 7 (id:??) landed as
https://crrev.com/281c15dfc08bd0a1b1d3972c5b27784ce5e25020
Cr-Commit-Position: refs/heads/master@{#424770}