Convert Chrome PPAPI proxy from bool to PP_Bool.

Convert Chrome PPAPI proxy from bool to PP_Bool.

TEST=none
BUG=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65381

[dmichael(do not use this one), 2010-11-08 15:31:19]

LGTM, just a few comments.

http://codereview.chromium.org/4659001/diff/1/2
File ppapi/proxy/interface_proxy.h (right):

http://codereview.chromium.org/4659001/diff/1/2#newcode79
ppapi/proxy/interface_proxy.h:79: }
Is there a reason you can't or shouldn't use ppapi/cpp/common.h instead of
defining them separately?  This means that there are both:
pp::BoolToPPBool
and
pp::proxy::BoolToPPBool
defined, and they do exactly the same thing.  It seems slightly odd.  Maybe the
real issue there is whether interface_proxy.h should really put things in the pp
namespace?  If this is a full-fledged member of ppapi, I think it can use the
common.h definitions.  If it's not, maybe we should consider taking it out of
namespace pp (in a future CL).  Thoughts?

http://codereview.chromium.org/4659001/diff/1/4
File ppapi/proxy/ppb_core_proxy.cc (right):

http://codereview.chromium.org/4659001/diff/1/4#newcode68
ppapi/proxy/ppb_core_proxy.cc:68: : PP_FALSE;
Why not use your convenience functions?

http://codereview.chromium.org/4659001/diff/1/18
File ppapi/proxy/serialized_var.cc (right):

http://codereview.chromium.org/4659001/diff/1/18#newcode126
ppapi/proxy/serialized_var.cc:126: m->WriteBool(var_.value.as_bool);
This part compiles okay on Windows?  I might have expected the usual
warning/error.

http://codereview.chromium.org/4659001/diff/1/18#newcode177
ppapi/proxy/serialized_var.cc:177: success = m->ReadBool(iter, &bool_value);
Maybe this would be premature optimization, but you could just serialize the
PP_Bool with a cast and WriteInt.  That's what happens to the bool value anyway,
so the current code converts enum->bool->int for serialization, and
int->bool->enum for deserialization.

[brettw, 2010-11-08 15:51:02]

http://codereview.chromium.org/4659001/diff/1/2
File ppapi/proxy/interface_proxy.h (right):

http://codereview.chromium.org/4659001/diff/1/2#newcode79
ppapi/proxy/interface_proxy.h:79: }
The proxy layer is actually totally separate from both ppapi/cpp and shouldn't
have any dependencies. ppapi/cpp is for plugins to statically link for
convenience. This proxy code will be compiled and distributed with Chrome
(although it's likewide a PPAPI plugin with no dependencies on the actual
implementation).

http://codereview.chromium.org/4659001/diff/1/18
File ppapi/proxy/serialized_var.cc (right):

http://codereview.chromium.org/4659001/diff/1/18#newcode177
ppapi/proxy/serialized_var.cc:177: success = m->ReadBool(iter, &bool_value);
The plugin might be malicious or have a bug and write a non-PP_TRUE/FALSE value
like "2". I prefer to get a correct bool value out at the renderer side to avoid
bugs. This current code will "fix" the problem when serializing, and if it's
invalid when deserializing, will report an error. Probably eventually the
deserializing error will cause us to kill the plugins since it's sending us
incorrect messages (we do this with renderers).

[brettw, 2010-11-08 15:59:14]

http://codereview.chromium.org/4659001/diff/1/18
File ppapi/proxy/serialized_var.cc (right):

http://codereview.chromium.org/4659001/diff/1/18#newcode126
ppapi/proxy/serialized_var.cc:126: m->WriteBool(var_.value.as_bool);
This doesn't build on Windows (yet), but thanks for pointing this out.

[dmichael(do not use this one), 2010-11-08 16:11:51]

Feel free to ping me when you have it building on Windows if you want me to look
again.

You might consider using this as your bug:
http://code.google.com/p/chromium/issues/detail?id=59791

http://codereview.chromium.org/4659001/diff/1/2
File ppapi/proxy/interface_proxy.h (right):

http://codereview.chromium.org/4659001/diff/1/2#newcode79
ppapi/proxy/interface_proxy.h:79: }
That makes sense.  That being the case, I'm not sure if any of the proxy stuff
really belongs in namespace pp, but that's an issue for another CL (if at all).

http://codereview.chromium.org/4659001/diff/1/18
File ppapi/proxy/serialized_var.cc (right):

http://codereview.chromium.org/4659001/diff/1/18#newcode177
ppapi/proxy/serialized_var.cc:177: success = m->ReadBool(iter, &bool_value);
Good point.  May warrant a comment, then, to fend off future thoughts of
inappropriate optimization?

[brettw, 2010-11-08 16:28:55]

On Mon, Nov 8, 2010 at 8:11 AM,  <dmichael@google.com> wrote:
> Feel free to ping me when you have it building on Windows if you want me to
> look
> again.

The whole proxy doesn't actually compile for anybody but me right now,
I'm still in the process of landing all the files. Making it all
compile on Windows and Mac will be separate patches.

I'll write a comment on the bool serialization, good idea.

Brett

[dmichael(do not use this one), 2010-11-08 16:45:25]

Then still LGTM (in case you were waiting)