List our features in ntp_snippets::kAllFeatures

List our features in ntp_snippets::kAllFeatures

Move the notifications feature into the component for consistency, even
though nothing in the component actually uses it.

Having this list lets us answer questions list "what active experiment
IDs correspond to our features?" Active experiments are important for
debugging, but they're only loosely tied to features.

BUG=713038
Review-Url: https://codereview.chromium.org/2831553003
Cr-Commit-Position: refs/heads/master@{#466640}
Committed: https://chromium.googlesource.com/chromium/src/+/429f312b0855e94bd2facaa27842513fc2c362dd

[sfiera, 2017-04-19 09:54:52]

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

[commit-bot: I haz the power, 2017-04-19 09:55:16]

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

[sfiera, 2017-04-19 09:55:23]

sfiera@chromium.org changed reviewers:
+ dgn@chromium.org, jkrcal@chromium.org

[sfiera, 2017-04-19 09:55:24]

Small prelude to https://crrev.com/2826013002

[commit-bot: I haz the power, 2017-04-19 11:10:17]

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

[commit-bot: I haz the power, 2017-04-19 11:10:19]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2017-04-19 11:23:40]

lgtm

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
File components/ntp_snippets/features.cc (right):

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
components/ntp_snippets/features.cc:17: const base::Feature*(kAllFeatures[]) =
{&kArticleSuggestionsFeature,
I am a bit worried this can easily get out of sync but I see no clearly better
solution :|

[sfiera, 2017-04-19 11:41:10]

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
File components/ntp_snippets/features.cc (right):

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
components/ntp_snippets/features.cc:17: const base::Feature*(kAllFeatures[]) =
{&kArticleSuggestionsFeature,
On 2017/04/19 11:23:40, jkrcal wrote:
> I am a bit worried this can easily get out of sync but I see no clearly better
> solution :|

Ack :|

[dgn, 2017-04-19 12:53:26]

Which features do you expect to have listed here? There are other missing, at
least the one I added recently: chrome::android::kContentSuggestionsSettings

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
File components/ntp_snippets/features.h (right):

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
components/ntp_snippets/features.h:22: extern const
base::Feature*(kAllFeatures[]);
nit: add a big comment here or at the beginning of the file to remind that new
features have to be added there?

[sfiera, 2017-04-19 13:04:54]

On 2017/04/19 12:53:26, dgn (in PST timezone) wrote:
> Which features do you expect to have listed here? There are other missing, at
> least the one I added recently: chrome::android::kContentSuggestionsSettings

At least at present, for the use case I have in mind (listing experiments), that
one probably doesn't matter. It probably won't have experiment IDs associated
with it. If we use the list for anything else, though, it might be confusing if
we have content suggestions features that aren't in it.

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
File components/ntp_snippets/features.h (right):

https://codereview.chromium.org/2831553003/diff/1/components/ntp_snippets/fea...
components/ntp_snippets/features.h:22: extern const
base::Feature*(kAllFeatures[]);
On 2017/04/19 12:53:26, dgn (in PST timezone) wrote:
> nit: add a big comment here or at the beginning of the file to remind that new
> features have to be added there?

I lengthened it here, but I'm not sure it's flashy enough (or if there is such a
thing).

[sfiera, 2017-04-20 17:36:43]

Ping? I'm happy to move that feature as well, if you think we should. This list
currently contains all features in ntp files and directories, and if we're not
sure what other features we own, that's a good argument for centralizing.

[dgn, 2017-04-21 20:42:40]

sorry for the delay, lgtm

[sfiera, 2017-04-24 11:58:56]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2017-04-24 11:58:56]

The patchset sent to the CQ was uploaded after l-g-t-m from jkrcal@chromium.org
Link to the patchset: https://codereview.chromium.org/2831553003/#ps20001
(title: "Lengthen comment")

[sfiera, 2017-04-24 11:59:03]

The CQ bit was unchecked by sfiera@chromium.org

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

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sfiera, 2017-04-24 11:59:40]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2017-04-24 11:59:41]

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

[commit-bot: I haz the power, 2017-04-24 12:00:00]

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-24 14:44:37]

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

[commit-bot: I haz the power, 2017-04-24 14:44:38]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sfiera, 2017-04-24 14:47:34]

The CQ bit was checked by sfiera@chromium.org

[commit-bot: I haz the power, 2017-04-24 14:48:00]

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-24 16:18:00]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493045254762600,
"parent_rev": "e1efe87a94f1aaf93e461b52ed395b3e3e75ed68", "commit_rev":
"429f312b0855e94bd2facaa27842513fc2c362dd"}

[commit-bot: I haz the power, 2017-04-24 16:18:11]

Description was changed from

==========
List our features in ntp_snippets::kAllFeatures

Move the notifications feature into the component for consistency, even
though nothing in the component actually uses it.

Having this list lets us answer questions list "what active experiment
IDs correspond to our features?" Active experiments are important for
debugging, but they're only loosely tied to features.

BUG=713038
==========

to

==========
List our features in ntp_snippets::kAllFeatures

Move the notifications feature into the component for consistency, even
though nothing in the component actually uses it.

Having this list lets us answer questions list "what active experiment
IDs correspond to our features?" Active experiments are important for
debugging, but they're only loosely tied to features.

BUG=713038

Review-Url: https://codereview.chromium.org/2831553003
Cr-Commit-Position: refs/heads/master@{#466640}
Committed:
https://chromium.googlesource.com/chromium/src/+/429f312b0855e94bd2facaa27842...
==========

[commit-bot: I haz the power, 2017-04-24 16:18:12]

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