Enable some parts of base that were previously disabled in base_untrusted

Enable parts of base_untrusted that are used by remoting client

1. base64, rand_util, threading, time and threads were previosly disabled in
base_untrusted. Remoting code depends on these parts of base, so they
need to be enabled. 
2. Implemented NaCl version of rand_util.
3. Enabled glibc version of base_untrusted

BUG=134216
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=148641

[bradn, 2012-07-24 23:02:29]

http://codereview.chromium.org/10795083/diff/4003/base/rand_util_nacl.cc
File base/rand_util_nacl.cc (right):

http://codereview.chromium.org/10795083/diff/4003/base/rand_util_nacl.cc#newc...
base/rand_util_nacl.cc:20:
pp::Module::Get()->GetPluginInterface(PPB_CRYPTO_DEV_INTERFACE));
Two questions:
1. I had the impression this interface was only available on the main thread. Is
this safe to make general?
2. I believe the IRT itself may have a random interface that works on all
threads, would that be better to use? NACL_IRT_RANDOM_v0_1

[Sergey Ulanov, 2012-07-24 23:33:23]

rewrote rand_util_nacl.cc using nacl_irt_random interface.

On 2012/07/24 23:02:29, bradn wrote:
> http://codereview.chromium.org/10795083/diff/4003/base/rand_util_nacl.cc
> File base/rand_util_nacl.cc (right):
> 
>
http://codereview.chromium.org/10795083/diff/4003/base/rand_util_nacl.cc#newc...
> base/rand_util_nacl.cc:20:
> pp::Module::Get()->GetPluginInterface(PPB_CRYPTO_DEV_INTERFACE));
> Two questions:
> 1. I had the impression this interface was only available on the main thread.
Is
> this safe to make general?
> 2. I believe the IRT itself may have a random interface that works on all
> threads, would that be better to use? NACL_IRT_RANDOM_v0_1

[bradn, 2012-07-24 23:36:39]

LGTM, but I believe you'll need an OWNERS review too.

[Sergey Ulanov, 2012-07-24 23:40:06]

+willchan for OWNERS approval

[willchan no longer on Chromium, 2012-07-25 17:52:55]

I've never seen the base_untrusted stuff before. Can you explain it briefly for
me? What's up with the files you are moving from base.gyp to base.gypi?

I had some binary bloat nits, but I did not review the logic in detail. I'm
trusting Brad on this to do the NaCl logic review. Brad, if that's a mistake,
please say so :) If not, then once my nits and questions have been addressed,
I'll rubberstamp your approval.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc
File base/rand_util_nacl.cc (right):

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:19: LOG(FATAL) << "Failed to initialize RNG.";
Can you split this out into DLOG(ERROR) with the message and then a
CHECK(false)? I don't think we need this string linked into an official build.
Binary bloat by a thousand cuts. Or if you don't need the message, just kill it.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:29: CHECK(error != 0) << "Failed to get random bytes.";
Ditto here on the CHECKs with messages. Ditch the messages unless you really
need them.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:40: base::LazyInstance<NaclRandom> g_nacl_random =
LAZY_INSTANCE_INITIALIZER;
Should this be leaky? I don't know anything about NACL, but it seems like it'd
be bad if another thread were running that was calling RandUint64(), while the
main thread's AtExitManager was deleting NaclRandom.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:47: uint64 result;
I think you may need basictypes.h for this type.

[Sergey Ulanov, 2012-07-25 19:43:23]

base.gypi is included by base.gyp and base_untrusted.gyp, so it contains list of
files that are shared by the base and base_untrusted targets.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc
File base/rand_util_nacl.cc (right):

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:19: LOG(FATAL) << "Failed to initialize RNG.";
On 2012/07/25 17:52:55, willchan wrote:
> Can you split this out into DLOG(ERROR) with the message and then a
> CHECK(false)? I don't think we need this string linked into an official build.
> Binary bloat by a thousand cuts. Or if you don't need the message, just kill
it.

Replaced with CHECK()

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:29: CHECK(error != 0) << "Failed to get random bytes.";
On 2012/07/25 17:52:55, willchan wrote:
> Ditto here on the CHECKs with messages. Ditch the messages unless you really
> need them.

Done.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:40: base::LazyInstance<NaclRandom> g_nacl_random =
LAZY_INSTANCE_INITIALIZER;
On 2012/07/25 17:52:55, willchan wrote:
> Should this be leaky? I don't know anything about NACL, but it seems like it'd
> be bad if another thread were running that was calling RandUint64(), while the
> main thread's AtExitManager was deleting NaclRandom.

Done.

http://codereview.chromium.org/10795083/diff/14001/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:47: uint64 result;
On 2012/07/25 17:52:55, willchan wrote:
> I think you may need basictypes.h for this type.

Done.

[Sergey Ulanov, 2012-07-25 20:09:40]

Also updated base64 to disable warnings that pop up when compiling for 64-bit as
part of base_static_win64 target.

[willchan no longer on Chromium, 2012-07-25 20:18:43]

I noticed the native_client header include. We're generally pretty
sensitive to including headers from other directories. This one seems
reasonable though. But I don't think we should have all of +native_client
in DEPS. Can you narrow that down to the specific subdirectory?

[willchan no longer on Chromium, 2012-07-25 20:19:27]

http://codereview.chromium.org/10795083/diff/16004/base/rand_util_nacl.cc
File base/rand_util_nacl.cc (right):

http://codereview.chromium.org/10795083/diff/16004/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:18: &random_, sizeof(random_));
indentation is off now :P

[Sergey Ulanov, 2012-07-25 20:34:42]

On 2012/07/25 20:18:43, willchan wrote:
> I noticed the native_client header include. We're generally pretty
> sensitive to including headers from other directories. This one seems
> reasonable though. But I don't think we should have all of +native_client
> in DEPS. Can you narrow that down to the specific subdirectory?

Done

[Sergey Ulanov, 2012-07-25 20:34:48]

http://codereview.chromium.org/10795083/diff/16004/base/rand_util_nacl.cc
File base/rand_util_nacl.cc (right):

http://codereview.chromium.org/10795083/diff/16004/base/rand_util_nacl.cc#new...
base/rand_util_nacl.cc:18: &random_, sizeof(random_));
On 2012/07/25 20:19:27, willchan wrote:
> indentation is off now :P

Done.

[bradn, 2012-07-25 20:41:30]

http://codereview.chromium.org/10795083/diff/15012/base/DEPS
File base/DEPS (right):

http://codereview.chromium.org/10795083/diff/15012/base/DEPS#newcode3
base/DEPS:3: "+native_client/src/untrusted/irt",
Actually perhaps narrow this down to that one header?

[Sergey Ulanov, 2012-07-25 20:43:55]

http://codereview.chromium.org/10795083/diff/15012/base/DEPS
File base/DEPS (right):

http://codereview.chromium.org/10795083/diff/15012/base/DEPS#newcode3
base/DEPS:3: "+native_client/src/untrusted/irt",
On 2012/07/25 20:41:30, bradn wrote:
> Actually perhaps narrow this down to that one header?

Done. Didn't know it is possible.

[bradn, 2012-07-25 20:52:25]

lgtm

[willchan no longer on Chromium, 2012-07-25 21:13:18]

lgtm

[commit-bot: I haz the power, 2012-07-25 21:15:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10795083/21016

[commit-bot: I haz the power, 2012-07-25 22:22:09]

Try job failure for 10795083-21016 (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-07-26 07:19:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10795083/28002

[commit-bot: I haz the power, 2012-07-26 16:11:44]

Try job failure for 10795083-28002 (retry) on linux_rel for steps
"interactive_ui_tests, browser_tests".
It's a second try, previously, steps "interactive_ui_tests, browser_tests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-26 20:05:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10795083/28002

[commit-bot: I haz the power, 2012-07-26 20:56:06]

Try job failure for 10795083-28002 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-07-26 21:58:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10795083/18007