Use pipe as ID separator and add inverse functions for MakeUniqueID

Use pipe as ID separator and add inverse functions for MakeUniqueID

A ContentSuggestionsProvider needs to be able to split up the
suggestion_id it retrieves in DiscardSuggestion and FetchSuggestionImage
into the category and the within_category_id that it provided to
MakeUniqueID when creating the ContentSuggestion.
Add two static helper functions to ContentSuggestionsProvider that
do the splitting.

Use the pipe "|" character instead of a colon to separate the category
from the within_category_id to prevent mistakes when implementing a
ContentSuggestionsProvider: Accidentally splitting a URL (which might
already be the within_category_id) instead of the unique_id fails
silently because URLs contain a colon. Using a different separator
prevents this.

BUG=619560
Committed: https://crrev.com/0cde3173b62fee4162a93fe497f8df2597c5e49e
Cr-Commit-Position: refs/heads/master@{#406268}

[Philipp Keck, 2016-07-19 12:20:28]

pke@google.com changed reviewers:
+ treib@chromium.org

[Philipp Keck, 2016-07-19 12:20:28]

PTAL

[Marc Treib, 2016-07-19 12:31:11]

LGTM with two nits, thanks!

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
File components/ntp_snippets/content_suggestions_provider.cc (right):

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
components/ntp_snippets/content_suggestions_provider.cc:15: const char
kSeparator[] = "|";
Can you make this an actual char, rather than a string?

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
components/ntp_snippets/content_suggestions_provider.cc:33:
ContentSuggestionsCategory ContentSuggestionsProvider::GetCategoryFromUniqueID(
Add
// static
above, also for the other method below.

[Philipp Keck, 2016-07-19 13:02:38]

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
File components/ntp_snippets/content_suggestions_provider.cc (right):

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
components/ntp_snippets/content_suggestions_provider.cc:15: const char
kSeparator[] = "|";
On 2016/07/19 12:31:11, Marc Treib wrote:
> Can you make this an actual char, rather than a string?

Done.

https://codereview.chromium.org/2155243003/diff/1/components/ntp_snippets/con...
components/ntp_snippets/content_suggestions_provider.cc:33:
ContentSuggestionsCategory ContentSuggestionsProvider::GetCategoryFromUniqueID(
On 2016/07/19 12:31:11, Marc Treib wrote:
> Add
> // static
> above, also for the other method below.

Done.

[Philipp Keck, 2016-07-19 13:02:48]

The CQ bit was checked by pke@google.com

[Philipp Keck, 2016-07-19 13:02:48]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org
Link to the patchset: https://codereview.chromium.org/2155243003/#ps20001
(title: "Marc's comments")

[commit-bot: I haz the power, 2016-07-19 13:03:16]

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-07-19 13:14:25]

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

[commit-bot: I haz the power, 2016-07-19 13:14:26]

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...)

[Philipp Keck, 2016-07-19 13:21:59]

PTAL again

[Marc Treib, 2016-07-19 13:40:56]

On 2016/07/19 13:21:59, Philipp Keck wrote:
> PTAL again

Still LGTM :)

[Philipp Keck, 2016-07-19 13:41:18]

The CQ bit was checked by pke@google.com

[commit-bot: I haz the power, 2016-07-19 13:41: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-07-19 14:24:12]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-19 14:24:20]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-19 14:25:25]

Description was changed from

==========
Use pipe as ID separator and add inverse functions for MakeUniqueID

A ContentSuggestionsProvider needs to be able to split up the
suggestion_id it retrieves in DiscardSuggestion and FetchSuggestionImage
into the category and the within_category_id that it provided to
MakeUniqueID when creating the ContentSuggestion.
Add two static helper functions to ContentSuggestionsProvider that
do the splitting.

Use the pipe "|" character instead of a colon to separate the category
from the within_category_id to prevent mistakes when implementing a
ContentSuggestionsProvider: Accidentally splitting a URL (which might
already be the within_category_id) instead of the unique_id fails
silently because URLs contain a colon. Using a different separator
prevents this.

BUG=619560
==========

to

==========
Use pipe as ID separator and add inverse functions for MakeUniqueID

A ContentSuggestionsProvider needs to be able to split up the
suggestion_id it retrieves in DiscardSuggestion and FetchSuggestionImage
into the category and the within_category_id that it provided to
MakeUniqueID when creating the ContentSuggestion.
Add two static helper functions to ContentSuggestionsProvider that
do the splitting.

Use the pipe "|" character instead of a colon to separate the category
from the within_category_id to prevent mistakes when implementing a
ContentSuggestionsProvider: Accidentally splitting a URL (which might
already be the within_category_id) instead of the unique_id fails
silently because URLs contain a colon. Using a different separator
prevents this.

BUG=619560

Committed: https://crrev.com/0cde3173b62fee4162a93fe497f8df2597c5e49e
Cr-Commit-Position: refs/heads/master@{#406268}
==========

[commit-bot: I haz the power, 2016-07-19 14:25:26]

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