Split secure command channel and untrusted application channel

Secure command channel and untrusted application channel have been split as two separated socket addresses, this allows secure command channel address to be passed to parent, etc.

Subsequently, the secure command service was refactored to allow accepting multiple connections and uses custom thread factory to track the number of command channels. When all command channels are closed, the application exits.

Secure command service was split into separate source files for clarity as they are independent from rest of the sel_ldr source code.

This SelUniveral-based test infrastructure is only temporary; once the current SelLdrLauncher will be replaced by the new interface, the test for this interface could be simplified to avoid the use of SelUniversal at all.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=3019
TESTS=small_tests

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=9853

[bsy, 2012-09-07 17:15:52]

please rebase & rerun try jobs

https://chromiumcodereview.appspot.com/10914138/diff/1/src/trusted/nonnacl_ut...
File src/trusted/nonnacl_util/sel_ldr_launcher.h (right):

https://chromiumcodereview.appspot.com/10914138/diff/1/src/trusted/nonnacl_ut...
src/trusted/nonnacl_util/sel_ldr_launcher.h:88: // Sets up the (|secure|)
command channel |command|.
is this a stale comment? |secure|?  no such parameter.  and the name is
SetupUntrusteCommand, so why |secure|?

[Petr Hosek, 2012-09-11 18:16:08]

This change is based on load_irt so we need to wait till that one gets through
to run the tests.

https://chromiumcodereview.appspot.com/10914138/diff/1/src/trusted/nonnacl_ut...
File src/trusted/nonnacl_util/sel_ldr_launcher.h (right):

https://chromiumcodereview.appspot.com/10914138/diff/1/src/trusted/nonnacl_ut...
src/trusted/nonnacl_util/sel_ldr_launcher.h:88: // Sets up the (|secure|)
command channel |command|.
On 2012/09/07 17:15:53, bsy wrote:
> is this a stale comment? |secure|?  no such parameter.  and the name is
> SetupUntrusteCommand, so why |secure|?

Fixed.

[bsy, 2012-09-24 22:47:18]

if bots are happy, lgtm

https://codereview.chromium.org/10914138/diff/15001/src/trusted/manifest_name...
File src/trusted/manifest_name_service_proxy/manifest_proxy.c (right):

https://codereview.chromium.org/10914138/diff/15001/src/trusted/manifest_name...
src/trusted/manifest_name_service_proxy/manifest_proxy.c:357: NaClLog(LOG_FATAL,
fix indent

[Petr Hosek, 2012-09-25 18:05:50]

command_setup SRPC call added to secure command channel.

[Petr Hosek, 2012-09-25 18:06:10]

https://codereview.chromium.org/10914138/diff/15001/src/trusted/manifest_name...
File src/trusted/manifest_name_service_proxy/manifest_proxy.c (right):

https://codereview.chromium.org/10914138/diff/15001/src/trusted/manifest_name...
src/trusted/manifest_name_service_proxy/manifest_proxy.c:357: NaClLog(LOG_FATAL,
On 2012/09/24 22:47:18, bsy wrote:
> fix indent

Fixed.

[bsy, 2012-09-25 18:37:10]

https://codereview.chromium.org/10914138/diff/20007/src/trusted/service_runti...
File src/trusted/service_runtime/sel_ldr.c (right):

https://codereview.chromium.org/10914138/diff/20007/src/trusted/service_runti...
src/trusted/service_runtime/sel_ldr.c:1409: struct NaClDesc                
*pair[2];
plz make the pair be part of the NaClApp object as discussed, so that we only
create one pair and spin off one thread for all additional command channels.

[Petr Hosek, 2012-09-25 23:09:11]

https://codereview.chromium.org/10914138/diff/20007/src/trusted/service_runti...
File src/trusted/service_runtime/sel_ldr.c (right):

https://codereview.chromium.org/10914138/diff/20007/src/trusted/service_runti...
src/trusted/service_runtime/sel_ldr.c:1409: struct NaClDesc                
*pair[2];
On 2012/09/25 18:37:10, bsy wrote:
> plz make the pair be part of the NaClApp object as discussed, so that we only
> create one pair and spin off one thread for all additional command channels.

Separated secure command channel and untrusted app channel as discussed.

[bsy, 2012-09-26 00:03:00]

a couple of minor bugs.  i think it's okay to have the secure service thread do
the exit, which mirrors the existing way of exiting immediately when the command
channel closes.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
File src/trusted/service_runtime/nacl_secure_service.c (right):

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:51: NaClLog(4, "NaClCondVar
failed\n");
NaClMutexDtor(&ref->mu); here, or add a label failure_mutex_dtor: before
failure_simple_ctor and put there to have the reversed cleanup exit pattern.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:250: NaClExit(0);
Who is waiting for the thread_count to reach zero?  If we exit here, then that
code might never awaken.  Either remove the condvar broadcast, or don't exit
here -- and make the code that's waiting for the thread_count to reach zero
responsible for exiting.

Since this is mostly for surf-away, I think it's okay to just exit here, but
will double check to see where in this CL do we wait for the thread_count to go
to zero.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
File src/trusted/service_runtime/sel_ldr.c (right):

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:806: if
(NACL_FI_ERROR_COND("NaClCreateServiceSocket__boundsock",
micro-nit: change the fault injection control name to
NaClCreateServiceSocket__secure_boundsock or something like that, so that in
testing we can cause these calls to fail independently.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:818:
NaClDescRef(nap->secure_service_port);
this is not necessary, since descriptors are created with a refcount of 1.  if
we every tried to have a NaClAppDtor do unrefs, we'd leak.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:822:
NaClDescRef(nap->secure_service_address);
not needed.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:833: NaClSetDesc(nap,
NACL_SERVICE_PORT_DESCRIPTOR, pair[0]);
this transferred ownership of the initial reference to the desc array... and
that's why below we have to take another reference for the pointer inside the
nap.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:1289: void
NaClBlockIfCommandChannelExists(struct NaClApp *nap) {
this waits for all command threads to exit, then returns.  we'd have a race
where sel_main and sel_main_chrome's invocation of
NaClBlockIfCommandChannelExists will return and exit, versus the NaClExit in the
code that does the condvar broadcast and NaClExit's.

[Petr Hosek, 2012-09-26 00:45:13]

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
File src/trusted/service_runtime/nacl_secure_service.c (right):

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:51: NaClLog(4, "NaClCondVar
failed\n");
On 2012/09/26 00:03:00, bsy wrote:
> NaClMutexDtor(&ref->mu); here, or add a label failure_mutex_dtor: before
> failure_simple_ctor and put there to have the reversed cleanup exit pattern.

Fixed by providing abort path which does the cleanup.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
File src/trusted/service_runtime/sel_ldr.c (right):

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:806: if
(NACL_FI_ERROR_COND("NaClCreateServiceSocket__boundsock",
On 2012/09/26 00:03:00, bsy wrote:
> micro-nit: change the fault injection control name to
> NaClCreateServiceSocket__secure_boundsock or something like that, so that in
> testing we can cause these calls to fail independently.

Fixed.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:818:
NaClDescRef(nap->secure_service_port);
On 2012/09/26 00:03:00, bsy wrote:
> this is not necessary, since descriptors are created with a refcount of 1.  if
> we every tried to have a NaClAppDtor do unrefs, we'd leak.

Removed.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:822:
NaClDescRef(nap->secure_service_address);
On 2012/09/26 00:03:00, bsy wrote:
> not needed.

Removed.

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:833: NaClSetDesc(nap,
NACL_SERVICE_PORT_DESCRIPTOR, pair[0]);
On 2012/09/26 00:03:00, bsy wrote:
> this transferred ownership of the initial reference to the desc array... and
> that's why below we have to take another reference for the pointer inside the
> nap.

Aha, now it's cleaner :)

http://codereview.chromium.org/10914138/diff/20009/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:1289: void
NaClBlockIfCommandChannelExists(struct NaClApp *nap) {
On 2012/09/26 00:03:00, bsy wrote:
> this waits for all command threads to exit, then returns.  we'd have a race
> where sel_main and sel_main_chrome's invocation of
> NaClBlockIfCommandChannelExists will return and exit, versus the NaClExit in
the
> code that does the condvar broadcast and NaClExit's.

Shall we just stick with the original version?

[bsy, 2012-09-26 03:35:33]

looking good.  please revert to the previous block-forever behavior and simplify
the secure service by removing the cv etc.

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
File src/trusted/service_runtime/nacl_secure_service.c (right):

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:67:
NaClCondVarDtor(&self->cv);
you cannot safely dtor what has not been successfully ctor'd.

instead, this should look like:

  if (!NaClMutexCtor(&self->mu)) {
    NaClLog(4, "NaClSecureServiceCtor: NaClMutexCtor failed\n");
    goto done;
  }
  if (!NaClCondVarCtor(&self->cv)) {
    NaClLog(4, "NaClSecureServiceCtor: NaClCondVarCtor failed\n");
    goto failure_dtor_mutex;
  }
  self->nap = nap;
  self->thread_cont = 0;

  NACL_VTBL(NaClRefCount, self) =
      (struct NaClRefCountVtbl *) &kNaClSecureServiceVtbl;
  retval = 1;
  goto done;

failure_dtor_mutex:
  NaClMutexDtor(&self->mu);
done:
  return retval;
}

but... since we are going to have the one NaClExit be when the thread count goes
to zero and not have a NaClSecureServiceWaitForServiceThreadsToExit method, this
simplifies to:


  if (!NaClMutexCtor(&self->mu)) {
    NaClLog(4, "NaClSecureServiceCtor: NaClMutexCtor failed\n");
    goto done;
  }
  self->nap = nap;
  self->thread_cont = 0;

  NACL_VTBL(NaClRefCount, self) =
      (struct NaClRefCountVtbl *) &kNaClSecureServiceVtbl;
  retval = 1;

done:
  return retval;
}

(and it's fine to also just get rid of retval and return directly rather than
try to have a single return.)

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:80: self->thread_count = 0;
not needed, post-condition implies this.

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:223: void
NaClSecureServiceWaitForServiceThreadsToExit(
remove method

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.c:274:
NaClSecureServiceWaitForServiceThreadsToExit,
remove method

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
File src/trusted/service_runtime/nacl_secure_service.h (right):

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.h:28: struct NaClCondVar        
 cv;
remove cv

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.h:47: void                      
   (*WaitForServiceThreadsToExit)(
remove this method

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/nacl_secure_service.h:50: void                      
   (*ThreadCountIncr)(
not clear if these two really need to be virtual functions rather than
non-virtual, but it's okay as is.

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
File src/trusted/service_runtime/sel_ldr.c (right):

http://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runtim...
src/trusted/service_runtime/sel_ldr.c:1291: (*NACL_VTBL(NaClSecureService,
nap->secure_service)->
revert to wait forever

[bsy, 2012-09-26 03:50:54]

one more bug

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:50: goto failure_simple_ctor;
  goto failure_no_mutex;

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:68: NaClMutexDtor(&self->mu);
failure_no_mutex:
  (*NACL_VTBL(NaClRefCount, self)->Dtor)((struct NaClRefCount *) self);
to run NaClSimpleService dtor and chain through to the rest.

[Petr Hosek, 2012-09-26 04:22:21]

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:50: goto failure_simple_ctor;
On 2012/09/26 03:50:55, bsy wrote:
>   goto failure_no_mutex;

Changed.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:67:
NaClCondVarDtor(&self->cv);
On 2012/09/26 03:35:33, bsy wrote:
> you cannot safely dtor what has not been successfully ctor'd.
> 
> instead, this should look like:
> 
>   if (!NaClMutexCtor(&self->mu)) {
>     NaClLog(4, "NaClSecureServiceCtor: NaClMutexCtor failed\n");
>     goto done;
>   }
>   if (!NaClCondVarCtor(&self->cv)) {
>     NaClLog(4, "NaClSecureServiceCtor: NaClCondVarCtor failed\n");
>     goto failure_dtor_mutex;
>   }
>   self->nap = nap;
>   self->thread_cont = 0;
> 
>   NACL_VTBL(NaClRefCount, self) =
>       (struct NaClRefCountVtbl *) &kNaClSecureServiceVtbl;
>   retval = 1;
>   goto done;
> 
> failure_dtor_mutex:
>   NaClMutexDtor(&self->mu);
> done:
>   return retval;
> }
> 
> but... since we are going to have the one NaClExit be when the thread count
goes
> to zero and not have a NaClSecureServiceWaitForServiceThreadsToExit method,
this
> simplifies to:
> 
> 
>   if (!NaClMutexCtor(&self->mu)) {
>     NaClLog(4, "NaClSecureServiceCtor: NaClMutexCtor failed\n");
>     goto done;
>   }
>   self->nap = nap;
>   self->thread_cont = 0;
> 
>   NACL_VTBL(NaClRefCount, self) =
>       (struct NaClRefCountVtbl *) &kNaClSecureServiceVtbl;
>   retval = 1;
> 
> done:
>   return retval;
> }
> 
> (and it's fine to also just get rid of retval and return directly rather than
> try to have a single return.)

Reworked.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:68: NaClMutexDtor(&self->mu);
On 2012/09/26 03:50:55, bsy wrote:
> failure_no_mutex:
>   (*NACL_VTBL(NaClRefCount, self)->Dtor)((struct NaClRefCount *) self);
> to run NaClSimpleService dtor and chain through to the rest.

Yep, already added ;)

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:80: self->thread_count = 0;
On 2012/09/26 03:35:33, bsy wrote:
> not needed, post-condition implies this.

Removed, as well as condvar.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:223: void
NaClSecureServiceWaitForServiceThreadsToExit(
On 2012/09/26 03:35:33, bsy wrote:
> remove method

Gone.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:274:
NaClSecureServiceWaitForServiceThreadsToExit,
On 2012/09/26 03:35:33, bsy wrote:
> remove method

Gone.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.h (right):

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.h:28: struct NaClCondVar        
 cv;
On 2012/09/26 03:35:33, bsy wrote:
> remove cv

Removed.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.h:47: void                      
   (*WaitForServiceThreadsToExit)(
On 2012/09/26 03:35:33, bsy wrote:
> remove this method

Removed.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.h:50: void                      
   (*ThreadCountIncr)(
On 2012/09/26 03:35:33, bsy wrote:
> not clear if these two really need to be virtual functions rather than
> non-virtual, but it's okay as is.

Turned into non-virtual, dropped NaClSecureServiceVtbl and used
NaClSimpleServiceVtbl instead.

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
File src/trusted/service_runtime/sel_ldr.c (right):

https://codereview.chromium.org/10914138/diff/27019/src/trusted/service_runti...
src/trusted/service_runtime/sel_ldr.c:1291: (*NACL_VTBL(NaClSecureService,
nap->secure_service)->
On 2012/09/26 03:35:33, bsy wrote:
> revert to wait forever

Reverted.

[bsy, 2012-09-26 15:45:10]

one nit.  please check veracity of comment & fix.

https://codereview.chromium.org/10914138/diff/19024/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/19024/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:156: * Takes ownership of rev
reference.  Caller should Ref() the argument
|rev| is no longer a formal parameter name.  plz fix comment.

[Petr Hosek, 2012-09-26 17:45:39]

https://codereview.chromium.org/10914138/diff/19024/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/19024/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:156: * Takes ownership of rev
reference.  Caller should Ref() the argument
On 2012/09/26 15:45:10, bsy wrote:
> |rev| is no longer a formal parameter name.  plz fix comment.

Sorry, copy & paste error, fixed.

[bsy, 2012-09-26 18:01:44]

a couple of quick style nits.  o/w lgtm.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:162: int                      
                rv = 1;
indent.  either line up, or one space after type specifier.

also, prefer initialization of rv to a default value to be failure.  it doesn't
matter, since this assignment is currently optimized out since all paths will
overwrite rv again....

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:261:
UNREFERENCED_PARAMETER(tif);
this is wrong, since tif is used in the hand_ptr->handler function call.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:369: struct
NaClSecureReverseClient *self,
indent.  either line up or use single-space after type-declaractor style.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
File src/trusted/service_runtime/sel_ldr.c (right):

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/sel_ldr.c:872: (uintptr_t) inherited_desc);
just for consistency, add return after LOG_FATAL, or delete returns below that
occurs after similar LOG_FATAL statements.

[Petr Hosek, 2012-09-26 18:49:34]

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
File src/trusted/service_runtime/nacl_secure_service.c (right):

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:162: int                      
                rv = 1;
On 2012/09/26 18:01:44, bsy wrote:
> indent.  either line up, or one space after type specifier.
> 
> also, prefer initialization of rv to a default value to be failure.  it
doesn't
> matter, since this assignment is currently optimized out since all paths will
> overwrite rv again....

Fixed.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:261:
UNREFERENCED_PARAMETER(tif);
On 2012/09/26 18:01:44, bsy wrote:
> this is wrong, since tif is used in the hand_ptr->handler function call.

Removed.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/nacl_secure_service.c:369: struct
NaClSecureReverseClient *self,
On 2012/09/26 18:01:44, bsy wrote:
> indent.  either line up or use single-space after type-declaractor style.

Fixed.

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
File src/trusted/service_runtime/sel_ldr.c (right):

https://codereview.chromium.org/10914138/diff/21016/src/trusted/service_runti...
src/trusted/service_runtime/sel_ldr.c:872: (uintptr_t) inherited_desc);
On 2012/09/26 18:01:44, bsy wrote:
> just for consistency, add return after LOG_FATAL, or delete returns below that
> occurs after similar LOG_FATAL statements.

Added.

[Mark Seaborn, 2012-09-28 16:37:45]

This change seems to break run_pnacl_bad_browser_test when I roll it into
Chromium (see the *_naclmore bot results on
http://codereview.chromium.org/10989068/).  Will you have a fix soon, or should
we revert this for now?

Mark

[phosek, 2012-09-28 16:47:45]

We already have a fix, see https://codereview.chromium.org/10979061/, it's
just going in.

Petr


On Fri, Sep 28, 2012 at 9:37 AM, <mseaborn@chromium.org> wrote:

> This change seems to break run_pnacl_bad_browser_test when I roll it into
> Chromium (see the *_naclmore bot results on
>
http://codereview.chromium.**org/10989068/<http://codereview.chromium.org/109...).
>  Will you have a fix soon, or should
> we revert this for now?
>
> Mark
>
>
>
https://chromiumcodereview.**appspot.com/10914138/<https://chromiumcodereview...
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.