Move guid generation from chrome/common/ to base/.

Move guid generation from chrome/common/ to base/.

It will be needed in webkit/dom_storage/.

BUG=NONE
TEST=Existing tests.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142142

[marja, 2012-06-05 14:56:57]

I'd need GUID generation functionality in webkit/dom_storage, so this CL moves
the GUID generation code from chrome/common to base/. (It used to be in base,
but it was moved to chrome/common by http://codereview.chromium.org/3800003 .)

Could you review the following parts:

dhollowa: all

jar:
base/

sky:
chrome/browser/ui/*
chrome/browser/search_engines/*
chrome/browser/history/*

pastarmovj:
chrome/browser/policy

[pastarmovj, 2012-06-05 14:59:56]

LGTM for /chrome/browser/policy.

[sky, 2012-06-05 16:03:49]

LGTM

[jar (doing other things), 2012-06-05 17:12:31]

Could you motivate why you need GUID generation elsewhere?   We end up having
privacy issues as we generate GUIDs, and I'm concerned to be sure you've thought
through such ramifications.

[marja, 2012-06-05 17:21:42]

We'd need persistent IDs which are unique across browsing sessions for storing
and restoring sessionStorage along with session restore. The GUIDs will be IDs
for the sessionStorage namespaces in the database where they are stored. When a
new browsing session is started, we'd like to be able to start generating new
unique IDs right away, before opening the database to see which IDs are already
present.

The sessionStorage data is associated with the GUID identifying the namespace,
and with an origin. If we happen to create the same GUID twice, the
"sessionStorage is unique for each tab" promise will be broken, but an unrelated
origin cannot read the data.

If we don't want to use the GUID generation code for this purpose, the dom
storage system could have a similar snippet for creating its own unique IDs
based on random data, but that would be duplicated code..

[jar (doing other things), 2012-06-05 19:23:14]

On Tue, Jun 5, 2012 at 10:21 AM, <marja@chromium.org> wrote:

> We'd need persistent IDs which are unique across browsing sessions for
> storing
> and restoring sessionStorage along with session restore. The GUIDs will be
> IDs
> for the sessionStorage namespaces in the database where they are stored.
> When a
> new browsing session is started, we'd like to be able to start generating
> new
> unique IDs right away, before opening the database to see which IDs are
> already
> present.
>

Is it plausible that you could use a counter, persisted in a file?  To put
it another way... you seem to need a unique ID, but probably not a globally
unique ID.  You'd probably have to be a bit careful.... but you could
always recover from a trashed file by looking at the max in your database
(which is traditionally well protected from corruption).

Am I missing something?  Yeah... reading from and updating a file would be
slower than calling rand(), ....but privacy folks don't seem to like
tagging their disk with GUIDs.  With the UMA metrics GUIDs, folks that
didn't opt-into UMA were bothered enough that they switched browser (to a
3rd party branch of chromium) just to avoid having this tag.  This was
despite the fact that we showed that the GUID would never be used without
their explicit permission.

Jim


>
> The sessionStorage data is associated with the GUID identifying the
> namespace,
> and with an origin. If we happen to create the same GUID twice, the
> "sessionStorage is unique for each tab" promise will be broken, but an
> unrelated
> origin cannot read the data.
>
> If we don't want to use the GUID generation code for this purpose, the dom
> storage system could have a similar snippet for creating its own unique IDs
> based on random data, but that would be duplicated code..
>
>
http://codereview.chromium.**org/10540003/<http://codereview.chromium.org/105...
>

[michaeln, 2012-06-05 21:16:42]

Privacy, good question.

> Is it plausible that you could use a counter, persisted in a file?  To put
> it another way... you seem to need a unique ID, but probably not a globally
> unique ID.  You'd probably have to be a bit careful.... but you could
> always recover from a trashed file by looking at the max in your database
> (which is traditionally well protected from corruption).

We started with that as a plan and backed off after complications. A session
'id' is coined very early on. To generate the 'next' near startup would require
reading from the db near startup. We don't want to incur more diskio at that
time. Also the ids are coined on the UI thread where 'tab' data structures are
created. Reading a persistent value from disk (if not already done ealier which
we dont like) at that time would involve injecting async steps into that
creation process. Just too much complexity.

> Am I missing something?  Yeah... reading from and updating a file would be
> slower than calling rand(), ....but privacy folks don't seem to like
> tagging their disk with GUIDs.  With the UMA metrics GUIDs, folks that
> didn't opt-into UMA were bothered enough that they switched browser (to a
> 3rd party branch of chromium) just to avoid having this tag.  This was
> despite the fact that we showed that the GUID would never be used without
> their explicit permission.

These id values get stored in two places.
- lower level SessionStorage database
- higher level (chrome) SessionRestore repository of some kind

For the SessionStorage usage, these values never actually make it into renderer
processes at all, and they never leave the local host (not counting the profile
directory being stored on a remote file system). I'm not sure if SessionRestore
data is subject to 'chrome sync' or not? SessionStorage is not. If
SessionRestore data does not leave the local host, is there any privacy issue
with this?

[jar (doing other things), 2012-06-05 21:48:40]

On Tue, Jun 5, 2012 at 2:16 PM, <michaeln@chromium.org> wrote:

> Privacy, good question.
>
>
>  Is it plausible that you could use a counter, persisted in a file?  To put
>> it another way... you seem to need a unique ID, but probably not a
>> globally
>> unique ID.  You'd probably have to be a bit careful.... but you could
>> always recover from a trashed file by looking at the max in your database
>> (which is traditionally well protected from corruption).
>>
>
> We started with that as a plan and backed off after complications. A
> session
> 'id' is coined very early on. To generate the 'next' near startup would
> require
> reading from the db near startup. We don't want to incur more diskio at
> that
> time. Also the ids are coined on the UI thread where 'tab' data structures
> are
> created. Reading a persistent value from disk (if not already done ealier
> which
> we dont like) at that time would involve injecting async steps into that
> creation process. Just too much complexity.


Since you're generating (and keeping) the id in memory, it sounds like you
could be lazy about getting it from disk.  You could use a bogus-value
during startup, and then replace it asap when you got the read/update.  In
the extreme, it could be sitting at a central place, and then just be
updated (to a counter based value).   I was suggesting *not* using the db,
but rather using a tiny file (reducing load/decipher time and complexity),
but if you went with lazy setting, then the delay time would be
insignificant (even if you went with the db access).


>
>
>  Am I missing something?  Yeah... reading from and updating a file would be
>> slower than calling rand(), ....but privacy folks don't seem to like
>> tagging their disk with GUIDs.  With the UMA metrics GUIDs, folks that
>> didn't opt-into UMA were bothered enough that they switched browser (to a
>> 3rd party branch of chromium) just to avoid having this tag.  This was
>> despite the fact that we showed that the GUID would never be used without
>> their explicit permission.
>>
>
> These id values get stored in two places.
> - lower level SessionStorage database
> - higher level (chrome) SessionRestore repository of some kind
>
> For the SessionStorage usage, these values never actually make it into
> renderer
> processes at all, and they never leave the local host (not counting the
> profile
> directory being stored on a remote file system). I'm not sure if
> SessionRestore
> data is subject to 'chrome sync' or not? SessionStorage is not. If
> SessionRestore data does not leave the local host, is there any privacy
> issue
> with this?
>

The UMA GUID was stored only in the browser process, and then only
persisted on disk in one file.  It was never used for anything else, never
revealed to a renderer, and in fact, there was no API for anyone to get it
(during Chrome's run) for use other than in an opted-in UMA upload.  It was
considered unacceptable from a privacy perspective to have this tag on the
disk.

I don't fully grok how you're using and perhaps disposing of these GUIDs
(do they last forever in a DB?), but perhaps the disposition would be
sufficient.  I'm not a privacy evangelist.... and I'm just replaying my
recollection of this progression.


>
>
>
>
https://chromiumcodereview.**appspot.com/10540003/<https://chromiumcodereview...
>

[jochen (gone - plz use gerrit), 2012-06-06 08:21:20]

I don't really see a privacy issue here. The UMA id was potentially send over
the network (when the user opted in to UMA) which made it a potential privacy
issue.

Here we're just talking about database keys. Also, the keys are not really long
living, but just as long as the corresponding tab exists (or is restorable).

[jar (doing other things), 2012-06-06 18:25:46]

As I also mentioned, the lifetime may be short enough that this is a non-problem
(note that the UMA ID was only sent under SSL protection, and only sent if the
user opted-in, but still, it was considered a privacy issue).

I still don't understand why you can't be lazy in your use of the ID.  It only
has to be unique relative to the database, and hence its "true value" only needs
to be corrected when the database is available.  Why can't you be lazy, give it
a temporary value that is corrected when the DB comes available?


On 2012/06/06 08:21:20, jochen wrote:
> I don't really see a privacy issue here. The UMA id was potentially send over
> the network (when the user opted in to UMA) which made it a potential privacy
> issue.
> 
> Here we're just talking about database keys. Also, the keys are not really
long
> living, but just as long as the corresponding tab exists (or is restorable).

[michaeln, 2012-06-06 19:45:17]

> I still don't understand why you can't be lazy in your use of the ID.  It only
> has to be unique relative to the database, and hence its "true value" only
needs
> to be corrected when the database is available.  Why can't you be lazy, give
it
> a temporary value that is corrected when the DB comes available?

The value gets written to two different places (there are two databases),
sessionrestore journal of some kind and to the sessionstoragedb, correcting the
value after its been written to sessionrestore journal (of some kind) would be
interesting... i suppose we could log a 'corrected' value sometime later but how
could you restore if the corrected value had not yet been logged when the system
crashed?

Unless there is a real privacy issue (which i dont really think there is), a
guid seems like a greatly simplifying thing.

[dhollowa, 2012-06-07 15:59:57]

Autofill LGTM.

[jar (doing other things), 2012-06-07 16:44:05]

http://codereview.chromium.org/10540003/diff/8003/base/guid.cc
File base/guid.cc (right):

http://codereview.chromium.org/10540003/diff/8003/base/guid.cc#newcode10
base/guid.cc:10: namespace guid {
I'm wondering if this should be in base, or base::guid.

http://codereview.chromium.org/10540003/diff/8003/base/guid.h
File base/guid.h (right):

http://codereview.chromium.org/10540003/diff/8003/base/guid.h#newcode15
base/guid.h:15: namespace guid {
If we're going to migrate to base... we should go ahead and put this is the
base:; namespace.  Looking at the names, I don't think you need a guid::
namespace (the names all have the word guid in them... and I don't see potential
for a collision).

[marja, 2012-06-08 08:58:45]

Thanks for having a look; I moved this into the base:: namespace.

http://codereview.chromium.org/10540003/diff/8003/base/guid.h
File base/guid.h (right):

http://codereview.chromium.org/10540003/diff/8003/base/guid.h#newcode15
base/guid.h:15: namespace guid {
On 2012/06/07 16:44:05, jar wrote:
> If we're going to migrate to base... we should go ahead and put this is the
> base:; namespace.  Looking at the names, I don't think you need a guid::
> namespace (the names all have the word guid in them... and I don't see
potential
> for a collision).

Done.

[jar (doing other things), 2012-06-11 17:03:32]

base changes LGTM

[commit-bot: I haz the power, 2012-06-12 16:14:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/10540003/6008

[commit-bot: I haz the power, 2012-06-12 17:36:39]

Try job failure for 10540003-6008 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-14 11:23:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/10540003/10006

[commit-bot: I haz the power, 2012-06-14 12:34:54]

Try job failure for 10540003-10006 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-14 13:19:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/10540003/8011

[commit-bot: I haz the power, 2012-06-14 14:22:09]

Change committed as 142142