Remove unused IPC::Message priority.

Remove unused IPC::Message priority.

Removes the PriorityValue enum and field from IPC::Message. This doesn't
appear to be used anywhere.

Changes the data message ctor to take a size_t data_len parameter.  This
works around an ambiguity problem with the main ctor, which has a similar
signature and would require lots of futzing with our test code to fix. To
make this work, the matching Pickle constructor is also changed to take a
size_t data_len parameter.

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

[bbudge, 2013-10-23 00:36:59]

ben@chromium.org: Please review changes in cloud_print

mseaborn@chromium.org: Please review changes in components/nacl

[Mark Seaborn, 2013-10-23 00:43:52]

On 2013/10/23 00:36:59, bbudge1 wrote:
> mseaborn@chromium.org: Please review changes in components/nacl

LGTM

[jam, 2013-10-23 00:44:31]

lgtm

When updating the IPC serialization, you would need to update the checked in
binaries or otherwise the perf tests will fail. See 

http://src.chromium.org/viewvc/chrome/trunk/deps/reference_builds/README?revi...

[bbudge, 2013-10-23 01:13:14]

On 2013/10/23 00:44:31, jam wrote:
> lgtm
> 
> When updating the IPC serialization, you would need to update the checked in
> binaries or otherwise the perf tests will fail. See 
> 
>
http://src.chromium.org/viewvc/chrome/trunk/deps/reference_builds/README?revi...

It seems like I would have to commit the IPC change first before updating the
reference builds, which means the perf tests would fail in the interim. Am I
understanding correctly?

[Ben Goodger (Google), 2013-10-23 06:47:33]

cloud_print lgtm

[bbudge, 2013-10-23 22:14:26]

John, I reworked this significantly. You may want to re-review, at least
ipc/ipc_message.*.

The problem I ran into is that removing the priority param from the constructor
led to many ambiguous calls, all in our test code. Basically, calling with
parameters like (0, 1) would match the changed ctor and the following one. I
tried fixing this by modifying the call sites but this would just bite people in
the future.

I could have modified the second ctor to take a size_t for the data size but
that is funky, since it passes through to the Pickle ctor, and I don't want to
change that.

This is the cleanest way around the problem. It also gives me the ability to use
the 2 newly freed up bits for my own purposes in the PPAPI proxy.

PTAL

[jam, 2013-10-24 00:06:58]

the flags is something that's meant to be used internally by the IPC system, and
not externally by users. That's what's the payload is for.

[bbudge, 2013-10-24 00:20:37]

On 2013/10/24 00:06:58, jam wrote:
> the flags is something that's meant to be used internally by the IPC system,
and
> not externally by users. That's what's the payload is for.

This is intended to simplify message interception for the NaCl Pepper proxy. We
are trying to audit some messages that are going back and forth between the
Browser and the NaCl Plugin. Some of the messages are Pepper resource reply
messages, so I can't use the message id to figure out which messages to unpack.
This patch is one way I can detect messages which should be unpacked.

There are other approaches I can take if you think this is inappropriate. They
have some undesirable implications (e.g. sending additional audit messages that
can be easily intercepted by id).

[bbudge, 2013-10-24 00:22:54]

On 2013/10/24 00:20:37, bbudge1 wrote:
> On 2013/10/24 00:06:58, jam wrote:
> > the flags is something that's meant to be used internally by the IPC system,
> and
> > not externally by users. That's what's the payload is for.
> 
> This is intended to simplify message interception for the NaCl Pepper proxy.
We
> are trying to audit some messages that are going back and forth between the
> Browser and the NaCl Plugin. Some of the messages are Pepper resource reply
> messages, so I can't use the message id to figure out which messages to
unpack.
> This patch is one way I can detect messages which should be unpacked.
> 
> There are other approaches I can take if you think this is inappropriate. They
> have some undesirable implications (e.g. sending additional audit messages
that
> can be easily intercepted by id).

One additional piece of background information: These messages are being
intercepted in the trusted part of the NaCl process where the native OS channel
traffic is translated to NaCl (POSIX) format and handles are intercepted and
wrapped as NaCl descriptors.

[bbudge, 2013-10-24 10:40:03]

Only removes unused PriorityValue enum and field.

I don't think this changes serialization now.

[jam, 2013-10-25 00:26:23]

lgtm

i'm not an owner in base btw

https://codereview.chromium.org/35643005/diff/860001/base/pickle.cc
File base/pickle.cc (right):

https://codereview.chromium.org/35643005/diff/860001/base/pickle.cc#newcode178
base/pickle.cc:178: if (data_len >= static_cast<int>(sizeof(Header)))
nit: you can get rid of a bunch of static_casts here

https://codereview.chromium.org/35643005/diff/860001/ipc/ipc_message.h
File ipc/ipc_message.h (right):

https://codereview.chromium.org/35643005/diff/860001/ipc/ipc_message.h#newcode41
ipc/ipc_message.h:41: SYNC_BIT          = 0x04,
while it's a positive that we get rid of unused priority bits, it's a bit odd
reading this and seeing unused bits.

hmm, perhaps just have

UNUSED = 0x03;
?

[bbudge, 2013-10-25 13:40:06]

+darin for base/pickle*

[darin (slow to review), 2013-10-25 19:43:09]

base/ changes LGTM

[commit-bot: I haz the power, 2013-10-28 13:21:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/35643005/1010001

[commit-bot: I haz the power, 2013-10-28 13:59:40]

Retried try job too often on ios_dbg_simulator for step(s) content_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-10-28 14:02:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bbudge@chromium.org/35643005/1010001

[commit-bot: I haz the power, 2013-10-28 15:30:08]

Change committed as 231330