[Offline pages] Refactor createClientIdForTabSharing and ClientId/UUID creation

[Offline pages] Refactor createClientIdForTabSharing and ClientId/UUID creation

BUG=635644
Committed: https://crrev.com/664f86feeb732c4d3bfd104a206343fccef37188
Cr-Commit-Position: refs/heads/master@{#414749}

[chili, 2016-08-25 19:01:00]

chili@chromium.org changed reviewers:
+ dimich@chromium.org, fgorski@chromium.org

[Dmitry Titov, 2016-08-25 19:53:52]

lgtm with a nit to fix before landing:

https://codereview.chromium.org/2273373003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/ClientId.java
(right):

https://codereview.chromium.org/2273373003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/ClientId.java:59:
public static ClientId createClientIdForNamespace(String namespace) {
Naming suggestion: to reflect the fact that the ClientID will contain a GUID, it
can be named something like createGuidClientIdForNamespace.
We have other ClientIDs around that for example contain URLs in place of guids
(as in AGSA API for example)

[chili, 2016-08-25 22:43:09]

Thanks!

https://codereview.chromium.org/2273373003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/ClientId.java
(right):

https://codereview.chromium.org/2273373003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/ClientId.java:59:
public static ClientId createClientIdForNamespace(String namespace) {
On 2016/08/25 19:53:52, Dmitry Titov wrote:
> Naming suggestion: to reflect the fact that the ClientID will contain a GUID,
it
> can be named something like createGuidClientIdForNamespace.
> We have other ClientIDs around that for example contain URLs in place of guids
> (as in AGSA API for example)

Done.

[fgorski, 2016-08-25 22:50:48]

lgtm with comment.

https://codereview.chromium.org/2273373003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
(right):

https://codereview.chromium.org/2273373003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java:416:
public void savePageLaterForDownload(final String url, final String namespace) {
did you consider renamsing this to savePageLaterForNamespace

[chili, 2016-08-26 17:11:55]

The CQ bit was checked by chili@chromium.org

[chili, 2016-08-26 17:11:55]

The patchset sent to the CQ was uploaded after l-g-t-m from dimich@chromium.org
Link to the patchset: https://codereview.chromium.org/2273373003/#ps20001
(title: "Include Guid in name to prevent possible confusion")

[chili, 2016-08-26 17:12:00]

https://codereview.chromium.org/2273373003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
(right):

https://codereview.chromium.org/2273373003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java:416:
public void savePageLaterForDownload(final String url, final String namespace) {
On 2016/08/25 22:50:48, fgorski wrote:
> did you consider renamsing this to savePageLaterForNamespace

Acknowledged.  Will do in a different patch.

[commit-bot: I haz the power, 2016-08-26 17:12:30]

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-08-26 18:23:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-26 18:28:04]

Description was changed from

==========
[Offline pages] Refactor createClientIdForTabSharing and ClientId/UUID creation

BUG=635644
==========

to

==========
[Offline pages] Refactor createClientIdForTabSharing and ClientId/UUID creation

BUG=635644

Committed: https://crrev.com/664f86feeb732c4d3bfd104a206343fccef37188
Cr-Commit-Position: refs/heads/master@{#414749}
==========

[commit-bot: I haz the power, 2016-08-26 18:28:05]

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