[NTP Snippets] NTPSnippet cleanup: Make ctor take all IDs at once

[NTP Snippets] NTPSnippet cleanup: Make ctor take all IDs at once

BUG=655099
Committed: https://crrev.com/2d688b9e4edab7bb544fa5929817faee83d1df67
Cr-Commit-Position: refs/heads/master@{#434635}

[Marc Treib, 2016-11-25 14:22:51]

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

[Marc Treib, 2016-11-25 14:22:51]

And one more small cleanup :)

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippet.cc (left):

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippet.cc:105: std::unique_ptr<NTPSnippet>
snippet(new NTPSnippet(id, kArticlesRemoteId));
Sorry for the large diff - I had to move stuff around, we now have to parse the
sources first, to collect all IDs.

[tschumann, 2016-11-25 15:55:26]

lgtm

with nits

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippet.cc (right):

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippet.cc:101: std::string id;
nit: rename 'id' to 'primary_id'?

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippet.h (right):

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippet.h:34: // the CreateFrom* methods
below instead.
Here's a nice discussion around this topic:
https://yaqs.googleplex.com/eng/q/5099710530453504 

I'd suggest to have a private helper function MakeUnique(...) which calls new
and wraps it into a unique_ptr.

[Marc Treib, 2016-11-25 16:57:29]

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

[Marc Treib, 2016-11-25 16:57:36]

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippet.cc (right):

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippet.cc:101: std::string id;
On 2016/11/25 15:55:26, tschumann wrote:
> nit: rename 'id' to 'primary_id'?

Done.

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
File components/ntp_snippets/remote/ntp_snippet.h (right):

https://codereview.chromium.org/2526313002/diff/20001/components/ntp_snippets...
components/ntp_snippets/remote/ntp_snippet.h:34: // the CreateFrom* methods
below instead.
On 2016/11/25 15:55:26, tschumann wrote:
> Here's a nice discussion around this topic:
> https://yaqs.googleplex.com/eng/q/5099710530453504 
> 
> I'd suggest to have a private helper function MakeUnique(...) which calls new
> and wraps it into a unique_ptr.

Done.

[commit-bot: I haz the power, 2016-11-25 16:57:39]

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

[Marc Treib, 2016-11-25 16:58:04]

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

[Marc Treib, 2016-11-25 16:58:04]

Once more, +jkrcal for approval. PTAL!

[commit-bot: I haz the power, 2016-11-25 17:53:13]

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

[commit-bot: I haz the power, 2016-11-25 17:53:14]

Dry run: This issue passed the CQ dry run.

[jkrcal, 2016-11-28 10:47:03]

On 2016/11/25 16:58:04, Marc Treib wrote:
> Once more, +jkrcal for approval. PTAL!

lgtm

[Marc Treib, 2016-11-28 10:51:58]

The CQ bit was checked by treib@chromium.org

[Marc Treib, 2016-11-28 10:51:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
tschumann@chromium.org
Link to the patchset: https://codereview.chromium.org/2526313002/#ps40001
(title: "review")

[commit-bot: I haz the power, 2016-11-28 10:52:10]

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-11-28 11:43:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480330318638700,
"parent_rev": "7cda7ae3af0bba70d9337095af2445a7cef8d594", "commit_rev":
"43d9c7ad7874c8685a04898b23dd02d634dc4566"}

[commit-bot: I haz the power, 2016-11-28 11:43:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-28 11:46:18]

Description was changed from

==========
[NTP Snippets] NTPSnippet cleanup: Make ctor take all IDs at once

BUG=655099
==========

to

==========
[NTP Snippets] NTPSnippet cleanup: Make ctor take all IDs at once

BUG=655099

Committed: https://crrev.com/2d688b9e4edab7bb544fa5929817faee83d1df67
Cr-Commit-Position: refs/heads/master@{#434635}
==========

[commit-bot: I haz the power, 2016-11-28 11:46:19]

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