Bug 48571 - [chromium] UUID generation does not work in Linux that has sandbox turned on
Summary: [chromium] UUID generation does not work in Linux that has sandbox turned on
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Jian Li
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-28 15:54 PDT by Jian Li
Modified: 2010-10-29 11:29 PDT (History)
3 users (show)

See Also:

Attachments
Proposed Patch (2.93 KB, patch)
2010-10-28 17:00 PDT, Jian Li 		jianli: commit-queue-
Details | Formatted Diff | Diff
Proposed Patch (2.77 KB, patch)
2010-10-28 17:26 PDT, Jian Li 		dimich: review-
jianli: commit-queue-
Details | Formatted Diff | Diff
Proposed Patch (2.84 KB, patch)
2010-10-28 17:58 PDT, Jian Li 		dimich: review+
jianli: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jian Li 2010-10-28 15:54:18 PDT 
UUID generation does not work in Linux that has sandbox turned on.
Comment 1 Jian Li 2010-10-28 17:00:11 PDT 
Created attachment 72262 [details]
Proposed Patch
Comment 2 Jian Li 2010-10-28 17:26:55 PDT 
Created attachment 72268 [details]
Proposed Patch
Comment 3 Dmitry Titov 2010-10-28 17:53:10 PDT 
Comment on attachment 72268 [details]
Proposed Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=72268&action=review

A few nits.
r- because  it implements it in the case where it should probably stay unimplemented...

> WebCore/platform/UUID.cpp:99
>  #else

Is there a specific benefit in enabling it for everything else? Why not limit to LINUX + CHROMIUM? If the bug is about Chromium Linux implementation, then it should be narrowly scoped, otherwise there should be a note why specifically the previously unimplemented functionality should be implemented.

> WebCore/platform/UUID.cpp:101
> +    for (size_t i = 0; i < sizeof(randomData) / sizeof(unsigned); ++i)

.. / sizeof(randomData[0]) ?

> WebCore/platform/UUID.cpp:113
> +    builder.append(String::format("%03x", randomData[2] >> 16));

seems it has to be (randomData[2]  >> 16 ) & 0x00000fff here...
Comment 4 Jian Li 2010-10-28 17:58:45 PDT 
Created attachment 72276 [details]
Proposed Patch

All fixed.
Comment 5 Dmitry Titov 2010-10-28 18:03:36 PDT 
Comment on attachment 72276 [details]
Proposed Patch

r=me
Comment 6 Jian Li 2010-10-28 18:18:21 PDT 
Committed as http://trac.webkit.org/changeset/70830
Comment 7 Adam Barth 2010-10-28 23:41:19 PDT 
Why don't we just use this code on all platforms?  I don't see the value in calling the platform-specific libraries.
Comment 8 Jian Li 2010-10-29 11:29:40 PDT 
(In reply to comment #7)
> Why don't we just use this code on all platforms?  I don't see the value in calling the platform-specific libraries.

I will watch this for a while. If we do not see any problem, we can start to extend to all other platforms.