[NTP Snippets] Introduce ContentSuggestion::ID

[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4bbc5492ff92a6e7e1c71a14cc9c36296a67eee5
Cr-Commit-Position: refs/heads/master@{#421555}

[Marc Treib, 2016-09-27 14:51:17]

Description was changed from

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this instead of the previous "unique ID" strings.

BUG=649048
==========

to

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this instead of the previous "unique ID" strings.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Marc Treib, 2016-09-27 15:44:13]

Description was changed from

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this instead of the previous "unique ID" strings.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have
been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Marc Treib, 2016-09-28 09:32:33]

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

[Marc Treib, 2016-09-28 09:32:34]

This is a big one, but most of the changes are fairly trivial. PTAL!

[Bernhard Bauer, 2016-09-28 09:45:33]

Looks mostly good, but TBH I'm not a huge fan of the expression
"within-category-ID" (I can't even find a way to write it outside of code that
doesn't require all these hyphens). Maybe |id_within_category|?

https://codereview.chromium.org/2377663002/diff/40001/components/ntp_snippets...
File components/ntp_snippets/ntp_snippets_service.cc (right):

https://codereview.chromium.org/2377663002/diff/40001/components/ntp_snippets...
components/ntp_snippets/ntp_snippets_service.cc:329:
DCHECK(categories_.find(suggestion_id.category()) != categories_.end());
Nit: Using base::Contains() for this (from base/stl_util.h) might look a bit
nicer.

[Marc Treib, 2016-09-28 11:14:09]

Thanks! I wasn't a fan of "within-category-ID" either. id_within_category is at
least a bit better, done!

https://codereview.chromium.org/2377663002/diff/40001/components/ntp_snippets...
File components/ntp_snippets/ntp_snippets_service.cc (right):

https://codereview.chromium.org/2377663002/diff/40001/components/ntp_snippets...
components/ntp_snippets/ntp_snippets_service.cc:329:
DCHECK(categories_.find(suggestion_id.category()) != categories_.end());
On 2016/09/28 09:45:33, Bernhard Bauer wrote:
> Nit: Using base::Contains() for this (from base/stl_util.h) might look a bit
> nicer.

Done.

[Marc Treib, 2016-09-28 11:14:15]

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

[commit-bot: I haz the power, 2016-09-28 11:14:28]

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, 2016-09-28 12:02:15]

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

[commit-bot: I haz the power, 2016-09-28 12:02:16]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2016-09-28 14:02:01]

LGTM!

[Marc Treib, 2016-09-28 14:22:40]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2016-09-28 14:22:51]

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-09-28 14:27:02]

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

[commit-bot: I haz the power, 2016-09-28 14:27:06]

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

[Marc Treib, 2016-09-28 16:22:24]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2016-09-28 16:22:25]

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

[commit-bot: I haz the power, 2016-09-28 16:22: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-09-28 17:27:10]

Description was changed from

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have
been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have
been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-28 17:27:11]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-28 17:31:34]

Description was changed from

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have
been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[NTP Snippets] Introduce ContentSuggestion::ID

Use this new type of ID instead of the previous "unique ID" strings, which have
been causing no end of confusion.

BUG=649048
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4bbc5492ff92a6e7e1c71a14cc9c36296a67eee5
Cr-Commit-Position: refs/heads/master@{#421555}
==========

[commit-bot: I haz the power, 2016-09-28 17:31:37]

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