Add a size parameter to SharedMemoryHandle.

Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2875453002
Cr-Commit-Position: refs/heads/master@{#472865}
Committed: https://chromium.googlesource.com/chromium/src/+/9d6afd71a4937e9f400609f839c2b39b22c4c497

[erikchen, 2017-05-09 22:59:02]

Description was changed from

==========
Add size to SharedMemoryHandle.

BUG=
==========

to

==========
Add size to SharedMemoryHandle.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[erikchen, 2017-05-09 23:19:03]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 23:20:27]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-09 23:36:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 23:36:30]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[erikchen, 2017-05-10 17:44:29]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 17:46:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 17:59:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 17:59:45]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[erikchen, 2017-05-10 19:44:27]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 19:45:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 20:05:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 20:05:45]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[erikchen, 2017-05-10 20:15:39]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-10 20:16:48]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 21:01:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 21:01:39]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[erikchen, 2017-05-11 01:19:37]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 01:21:02]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-11 02:05:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 02:05:13]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[erikchen, 2017-05-11 21:43:53]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 21:45:08]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[erikchen, 2017-05-11 21:46:11]

Description was changed from

==========
Add size to SharedMemoryHandle.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Add size to SharedMemoryHandle.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[commit-bot: I haz the power, 2017-05-11 21:51:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 21:51:12]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  win10_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x6...)

[erikchen, 2017-05-11 22:09:12]

Description was changed from

==========
Add size to SharedMemoryHandle.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Add a size paraemter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[erikchen, 2017-05-11 22:09:23]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 22:10:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[erikchen, 2017-05-11 22:48:55]

Description was changed from

==========
Add a size paraemter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[erikchen, 2017-05-11 22:50:28]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-11 22:51:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 00:09:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 00:09:58]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[erikchen, 2017-05-12 01:08:19]

Description was changed from

==========
Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[erikchen, 2017-05-12 01:55:00]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 01:55:34]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 03:43:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 03:44:00]

Dry run: This issue passed the CQ dry run.

[erikchen, 2017-05-12 05:30:09]

erikchen@chromium.org changed reviewers:
+ rockot@chromium.org, thakis@chromium.org

[erikchen, 2017-05-12 05:30:10]

thakis: Please review.
rockot: Please review *ipc* and *mojo*

[Nico, 2017-05-12 14:49:14]

As mentioned before, this "handle" class is imo getting way too heavy. Are there
no other ways?

[erikchen, 2017-05-12 16:54:04]

On 2017/05/12 14:49:14, Nico wrote:
> As mentioned before, this "handle" class is imo getting way too heavy. Are
there
> no other ways?

The alternative would be to introduce another primitive type to Mojo [basically
shared buffer w/o size], which also doesn't seem great. See the changes to
"ui/gfx/mojo/buffer_types_struct_traits.cc" for an example of what this CL
allows - we now serialize a SharedMemoryHandle as a shared buffer, which in turn
will pick up the GUID changes that rockot@ will eventually make.

Basically, we need SharedMemoryHandle and mojo SharedBuffer to be
interchangeable. 

https://cs.chromium.org/chromium/src/mojo/edk/embedder/platform_shared_buffer...

I considered also adding a "read_only" field, but decided that that isn't
necessary [at least for now].

Note that all of these changes *could* be justified even in the absence of mojo.
It doesn't really make sense to have a SharedMemoryHandle without a size, since
callers that want to map it pretty much always need to know its size anyways.
Right now we just happen to pass size as a second parameter along with SHM
handle in pretty much every IPC message.

[Ken Rockot(use gerrit already), 2017-05-12 16:59:50]

I think maybe the issue here is that the word "handle" is a bit of a
misnomer at this point, and it's setting the wrong expectations for what
the type should represent. Keeping track of a shared buffer reference in a
way that can be used consistently and reliably across process boundaries
requires more than just a system object handle, in practice.

Imagine we renamed it to MappableSharedBufferReference or something, but
then imagine that such a rename would create more churn than is worthwhile.

On Fri, May 12, 2017 at 9:54 AM, <erikchen@chromium.org> wrote:

> On 2017/05/12 14:49:14, Nico wrote:
> > As mentioned before, this "handle" class is imo getting way too heavy.
> Are
> there
> > no other ways?
>
> The alternative would be to introduce another primitive type to Mojo
> [basically
> shared buffer w/o size], which also doesn't seem great. See the changes to
> "ui/gfx/mojo/buffer_types_struct_traits.cc" for an example of what this CL
> allows - we now serialize a SharedMemoryHandle as a shared buffer, which
> in turn
> will pick up the GUID changes that rockot@ will eventually make.
>
> Basically, we need SharedMemoryHandle and mojo SharedBuffer to be
> interchangeable.
>
> https://cs.chromium.org/chromium/src/mojo/edk/embedder/platform_shared_
> buffer.h?type=cs&l=120
>
> I considered also adding a "read_only" field, but decided that that isn't
> necessary [at least for now].
>
> Note that all of these changes *could* be justified even in the absence of
> mojo.
> It doesn't really make sense to have a SharedMemoryHandle without a size,
> since
> callers that want to map it pretty much always need to know its size
> anyways.
> Right now we just happen to pass size as a second parameter along with SHM
> handle in pretty much every IPC message.
>
> https://codereview.chromium.org/2875453002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2017-05-15 22:08:47]

On 2017/05/12 16:59:50, Ken Rockot wrote:
> I think maybe the issue here is that the word "handle" is a bit of a
> misnomer at this point, and it's setting the wrong expectations for what
> the type should represent. Keeping track of a shared buffer reference in a
> way that can be used consistently and reliably across process boundaries
> requires more than just a system object handle, in practice.
> 
> Imagine we renamed it to MappableSharedBufferReference or something, but
> then imagine that such a rename would create more churn than is worthwhile.

Renaming a class to be named like what it is seems worthwhile to me.

Is there any longer-term vision for this class? Do you expect to add more and
more things to it over time?

[Nico, 2017-05-15 22:13:36]

I only looked at base/

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_android.cc (right):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_android.cc:46:
SharedMemoryHandle(base::FileDescriptor(dup(shm_.GetHandle()), false),
Maybe SharedMemoryHandle should have a ReadonlyDup() method by now?

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_handle.h (right):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:75: // output variable |size|.
What?

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:97: // UnguessableToken.
Document size and what happens if the passed-in size doesn't match the "real"
size of the thing backed by the fd (also other ctors)

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:189: size_t size_ = 0;
Either use in-class initialization for all fields or for none of them.

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_mac.cc (left):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_mac.cc:247: shm_.GetGUID());
Why is this changing?

[erikchen, 2017-05-15 22:57:19]

thakis: PTAL
rockot: Please review everything not in base/

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_android.cc (right):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_android.cc:46:
SharedMemoryHandle(base::FileDescriptor(dup(shm_.GetHandle()), false),
On 2017/05/15 22:13:36, Nico wrote:
> Maybe SharedMemoryHandle should have a ReadonlyDup() method by now?

Actually, the GetReadOnlyHandle method is on SharedMemory, and that's because
the implementation on POSIX requires us to reopen the file, which we no longer
keep around, and thus is done during creation of a shared memory segment.

All of this being said, this code doesn't even drop the protection status, so
Duplicate() is sufficient.

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_handle.h (right):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:75: // output variable |size|.
On 2017/05/15 22:13:36, Nico wrote:
> What?

Updated comment.

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:97: // UnguessableToken.
On 2017/05/15 22:13:36, Nico wrote:
> Document size and what happens if the passed-in size doesn't match the "real"
> size of the thing backed by the fd (also other ctors)

Done.

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_handle.h:189: size_t size_ = 0;
On 2017/05/15 22:13:36, Nico wrote:
> Either use in-class initialization for all fields or for none of them.

all of them, it is.

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_mac.cc (left):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_mac.cc:247: shm_.GetGUID());
On 2017/05/15 22:13:36, Nico wrote:
> Why is this changing?

The old code weas basically the same, expect it changes the
ownership_passes_to_ipc_ member to false. POSIX fd is used in exactly one place,
which doesn't pass to IPC.

And this now patches the implementation on POSIX.

[Nico, 2017-05-15 23:11:56]

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
File base/memory/shared_memory_android.cc (right):

https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
base/memory/shared_memory_android.cc:46:
SharedMemoryHandle(base::FileDescriptor(dup(shm_.GetHandle()), false),
On 2017/05/15 22:57:18, erikchen wrote:
> On 2017/05/15 22:13:36, Nico wrote:
> > Maybe SharedMemoryHandle should have a ReadonlyDup() method by now?
> 
> Actually, the GetReadOnlyHandle method is on SharedMemory, and that's because
> the implementation on POSIX requires us to reopen the file, which we no longer
> keep around, and thus is done during creation of a shared memory segment.

I mean, this here and a few other places in this CL iirc call the SMH ctor with
a bunch of arguments that are all accessors on smh. The law of demeter suggests
that this should instead be some well-named method on SMH.

[erikchen, 2017-05-15 23:58:29]

On 2017/05/15 23:11:56, Nico wrote:
>
https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
> File base/memory/shared_memory_android.cc (right):
> 
>
https://codereview.chromium.org/2875453002/diff/180001/base/memory/shared_mem...
> base/memory/shared_memory_android.cc:46:
> SharedMemoryHandle(base::FileDescriptor(dup(shm_.GetHandle()), false),
> On 2017/05/15 22:57:18, erikchen wrote:
> > On 2017/05/15 22:13:36, Nico wrote:
> > > Maybe SharedMemoryHandle should have a ReadonlyDup() method by now?
> > 
> > Actually, the GetReadOnlyHandle method is on SharedMemory, and that's
because
> > the implementation on POSIX requires us to reopen the file, which we no
longer
> > keep around, and thus is done during creation of a shared memory segment.
> 
> I mean, this here and a few other places in this CL iirc call the SMH ctor
with
> a bunch of arguments that are all accessors on smh. The law of demeter
suggests
> that this should instead be some well-named method on SMH.

I went through all call sites touched in this CL. Most of the time, the
arguments are not coming from accessors on shm.

[erikchen, 2017-05-15 23:58:34]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 23:59:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2017-05-16 00:11:26]

non-base lgtm

[commit-bot: I haz the power, 2017-05-16 01:54:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-16 01:54:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Nico, 2017-05-16 16:11:45]

base/ lgtm

[erikchen, 2017-05-18 01:23:54]

The CQ bit was checked by erikchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-18 01:25:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[erikchen, 2017-05-18 01:27:21]

erikchen@chromium.org changed reviewers:
+ bbudge@chromium.org, reveman@chromium.org, wfh@chromium.org

[erikchen, 2017-05-18 01:27:22]

wfh: Please review ui/gfx/mojo/ and sandbox/win/
bbudge: Please review ppapi/proxy/nacl_message_scanner.cc
reveman: Please review components/exo/OWNERS.

[Will Harris, 2017-05-18 01:56:21]

lgtm for ui/gfx/mojo/ and sandbox/win/

[reveman, 2017-05-18 03:57:58]

components/exo lgtm

[commit-bot: I haz the power, 2017-05-18 05:34:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-18 05:34:03]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[bbudge, 2017-05-18 11:56:38]

ppapi/proxy/nacl_message_scanner.cc lgtm

[erikchen, 2017-05-18 16:15:19]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2017-05-18 16:16:13]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-18 17:49:09]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1495124119569210,
"parent_rev": "13c37ee0b4babbdec65c0ea3c68c9d771999689c", "commit_rev":
"9d6afd71a4937e9f400609f839c2b39b22c4c497"}

[commit-bot: I haz the power, 2017-05-18 17:49:25]

Description was changed from

==========
Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Add a size parameter to SharedMemoryHandle.

This is needed to construct a Mojo shared buffer from a SharedMemoryHandle,
which in turn is needed to pass GUIDs through Mojo. Previously, classes like
gfx::GpuMemoryBufferHandle were being serialized as a file descriptor or Windows
HANDLE, rather than a SharedMemoryHandle because they did not have access to a
size field.

BUG=713763
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2875453002
Cr-Commit-Position: refs/heads/master@{#472865}
Committed:
https://chromium.googlesource.com/chromium/src/+/9d6afd71a4937e9f400609f839c2...
==========

[commit-bot: I haz the power, 2017-05-18 17:49:27]

Committed patchset #14 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/9d6afd71a4937e9f400609f839c2...