Make the new Mojo EDK work on XP.

Make the new Mojo EDK work on XP.

Main points to note:
-child processes started by Mojo inherit all inheritable handles. This isn't good security wise, but is needed because UpdateProcThreadAttribute doesn't exist on XP. I added a CHECK to ensure we don't use this code in production.
-when sending a pipe, if there's a pending read then we block the current thread to call CancelIo on the IO thread. This is because CancelIOEx, which cancels process-wide, doesn't exist. In practice, this is not a major concern because we only start reading from a pipe once the mojo primitive is read/written to, and once that happens we almost never send the pipe.

BUG=478251
Committed: https://crrev.com/12d28add8388f5ea5760783a2817eca3a9a722a1
Cr-Commit-Position: refs/heads/master@{#359632}

[jam, 2015-11-13 08:03:27]

Description was changed from

==========
Make the new Mojo EDK work on XP.

Main points to note:
-child processes started by Mojo inherit all inheritable handles. This is not
ideal security wise, but is needed because UpdateProcThreadAttribute doesn't
exist on XP.
-when sending a pipe, if there's a pending read then we block the current thread
to call CancelIo on the IO thread. This is because CancelIOEx, which cancels
process-wide, doesn't exist. In practice, this is not a major concern because we
only start reading from a pipe once the mojo primitive is read/written to, and
once that happens we almost never send the pipe.

BUG=478251
==========

to

==========
Make the new Mojo EDK work on XP.

Main points to note:
-child processes started by Mojo inherit all inheritable handles. This is not
ideal security wise, but is needed because UpdateProcThreadAttribute doesn't
exist on XP.
-when sending a pipe, if there's a pending read then we block the current thread
to call CancelIo on the IO thread. This is because CancelIOEx, which cancels
process-wide, doesn't exist. In practice, this is not a major concern because we
only start reading from a pipe once the mojo primitive is read/written to, and
once that happens we almost never send the pipe.

BUG=478251
==========

[jam, 2015-11-13 08:03:28]

jam@chromium.org changed reviewers:
+ yzshen@chromium.org

[jam, 2015-11-13 16:32:59]

Description was changed from

==========
Make the new Mojo EDK work on XP.

Main points to note:
-child processes started by Mojo inherit all inheritable handles. This is not
ideal security wise, but is needed because UpdateProcThreadAttribute doesn't
exist on XP.
-when sending a pipe, if there's a pending read then we block the current thread
to call CancelIo on the IO thread. This is because CancelIOEx, which cancels
process-wide, doesn't exist. In practice, this is not a major concern because we
only start reading from a pipe once the mojo primitive is read/written to, and
once that happens we almost never send the pipe.

BUG=478251
==========

to

==========
Make the new Mojo EDK work on XP.

Main points to note:
-child processes started by Mojo inherit all inheritable handles. This isn't
good security wise, but is needed because UpdateProcThreadAttribute doesn't
exist on XP. I added a CHECK to ensure we don't use this code in production.
-when sending a pipe, if there's a pending read then we block the current thread
to call CancelIo on the IO thread. This is because CancelIOEx, which cancels
process-wide, doesn't exist. In practice, this is not a major concern because we
only start reading from a pipe once the mojo primitive is read/written to, and
once that happens we almost never send the pipe.

BUG=478251
==========

[yzshen1, 2015-11-13 17:00:30]

LGTM

To make sure I understand correctly: We will use the new EDK in production code,
even on XP. However, it is okay because currently we don't use
PlatformChannelPair to pass the initial OS pipe in chrome. Is that correct?

[jam, 2015-11-13 17:08:51]

On 2015/11/13 17:00:30, yzshen1 wrote:
> LGTM
> 
> To make sure I understand correctly: We will use the new EDK in production
code,
> even on XP. However, it is okay because currently we don't use
> PlatformChannelPair to pass the initial OS pipe in chrome. Is that correct?

yep

[jam, 2015-11-13 18:10:55]

On 2015/11/13 17:08:51, jam wrote:
> On 2015/11/13 17:00:30, yzshen1 wrote:
> > LGTM
> > 
> > To make sure I understand correctly: We will use the new EDK in production
> code,
> > even on XP. However, it is okay because currently we don't use
> > PlatformChannelPair to pass the initial OS pipe in chrome. Is that correct?
> 
> yep

ptal, I added a few small changes to ensure mojo_system_unittests2.exe passed on
XP

[yzshen1, 2015-11-13 18:27:07]

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
File mojo/edk/system/core_unittest.cc (right):

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
mojo/edk/system/core_unittest.cc:721: #if defined(OS_WIN)
Please use #if #endif around complete statements:
#if ...
  if (xxx) {
    ...
  }
#else
  ...
#endif

[yzshen1, 2015-11-13 18:31:06]

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
File mojo/edk/system/core_unittest.cc (right):

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
mojo/edk/system/core_unittest.cc:721: #if defined(OS_WIN)
Maybe:

bool write_message = true;
#if defined(OS_WIN)
  write_message = xxxx;
#endif
if (write_message) {
  ...core()->WriteMessage...
}

[jam, 2015-11-13 18:35:46]

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
File mojo/edk/system/core_unittest.cc (right):

https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
mojo/edk/system/core_unittest.cc:721: #if defined(OS_WIN)
On 2015/11/13 18:31:06, yzshen1 wrote:
> Maybe:
> 
> bool write_message = true;
> #if defined(OS_WIN)
>   write_message = xxxx;
> #endif
> if (write_message) {
>   ...core()->WriteMessage...
> }

yeah i considered the alternatives, but thought i'd do the simplest thing since
this is test code that'll be deleted in a few months :)

[yzshen1, 2015-11-13 18:54:29]

On 2015/11/13 18:35:46, jam wrote:
>
https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
> File mojo/edk/system/core_unittest.cc (right):
> 
>
https://codereview.chromium.org/1441853003/diff/40001/mojo/edk/system/core_un...
> mojo/edk/system/core_unittest.cc:721: #if defined(OS_WIN)
> On 2015/11/13 18:31:06, yzshen1 wrote:
> > Maybe:
> > 
> > bool write_message = true;
> > #if defined(OS_WIN)
> >   write_message = xxxx;
> > #endif
> > if (write_message) {
> >   ...core()->WriteMessage...
> > }
> 
> yeah i considered the alternatives, but thought i'd do the simplest thing
since
> this is test code that'll be deleted in a few months :)

LGTM

I feel that the alternative above is slightly simpler in that it only has one
set of #if/#endif. But please consider this as an optional comment.

[jam, 2015-11-13 20:39:12]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-11-13 20:39:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1441853003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1441853003/40001

[commit-bot: I haz the power, 2015-11-13 20:58:37]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-11-13 21:00:05]

Patchset 3 (id:??) landed as
https://crrev.com/12d28add8388f5ea5760783a2817eca3a9a722a1
Cr-Commit-Position: refs/heads/master@{#359632}