Add proxies for some of the PDF & Flash functionality. There are still a few...

Add proxies for some of the PDF & Flash functionality. There are still a few
unimplemented parts in the Flash proxy, and I only implemented the necessary
part of the PDF one.

TEST=none
BUG=none

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

[brettw, 2010-11-11 18:14:00]

There are also some dependencies on the font patch (did them in separate trees)
before I can finish all the Pdf font seriaization stuff. I'm going to land both
of the patches and then refactor a bit to make it work.

[viettrungluu, 2010-11-11 19:27:03]

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc
File ppapi/proxy/dispatcher.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc#newco...
ppapi/proxy/dispatcher.cc:220: if (interface_name == PPB_PRIVATE_INTERFACE)
I assume this will eventually be changed to PPB_PDF_INTERFACE?

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits.cc
File ppapi/proxy/ppapi_param_traits.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits....
ppapi/proxy/ppapi_param_traits.cc:63: if (type != PP_FILETYPE_REGULAR ||
&&

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits....
ppapi/proxy/ppapi_param_traits.cc:68: if (system_type !=
PP_FILESYSTEMTYPE_EXTERNAL ||
&&

[brettw, 2010-11-11 20:12:09]

New snap up.

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc
File ppapi/proxy/dispatcher.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc#newco...
ppapi/proxy/dispatcher.cc:220: if (interface_name == PPB_PRIVATE_INTERFACE)
On 2010/11/11 19:27:03, viettrungluu wrote:
> I assume this will eventually be changed to PPB_PDF_INTERFACE?

Yes, I want to rename them, but I don't want to confuse this patch with it. I
just wanted the new stuff to have the better name.

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits.cc
File ppapi/proxy/ppapi_param_traits.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits....
ppapi/proxy/ppapi_param_traits.cc:63: if (type != PP_FILETYPE_REGULAR ||
Thanks for catching this.

[viettrungluu, 2010-11-11 20:27:51]

LGTM otherwise. I'm sure I missed plenty.

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_buffer_proxy.cc
File ppapi/proxy/ppb_buffer_proxy.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_buffer_proxy.cc...
ppapi/proxy/ppb_buffer_proxy.cc:7: #include <string.h>  // For memcpy
Silly nit: Apparently, comments are supposed to be terminated by punctuation?

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_flash_proxy.cc
File ppapi/proxy/ppb_flash_proxy.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_flash_proxy.cc#...
ppapi/proxy/ppb_flash_proxy.cc:65: return false; // FIXME
TODO?

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_flash_proxy.cc#...
ppapi/proxy/ppb_flash_proxy.cc:86: 
extraneous blank line

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_pdf_proxy.cc
File ppapi/proxy/ppb_pdf_proxy.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_pdf_proxy.cc#ne...
ppapi/proxy/ppb_pdf_proxy.cc:7: #include <string.h>  // For memcpy
"

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_pdf_proxy.cc#ne...
ppapi/proxy/ppb_pdf_proxy.cc:105: NULL,  // &GetLocalizedString,
Are these TODOs?

[viettrungluu, 2010-11-11 20:44:05]

LGTM.

On 2010/11/11 20:12:09, brettw wrote:
> New snap up.
> 
> http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc
> File ppapi/proxy/dispatcher.cc (right):
> 
>
http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/dispatcher.cc#newco...
> ppapi/proxy/dispatcher.cc:220: if (interface_name == PPB_PRIVATE_INTERFACE)
> On 2010/11/11 19:27:03, viettrungluu wrote:
> > I assume this will eventually be changed to PPB_PDF_INTERFACE?
> 
> Yes, I want to rename them, but I don't want to confuse this patch with it. I
> just wanted the new stuff to have the better name.
> 
>
http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits.cc
> File ppapi/proxy/ppapi_param_traits.cc (right):
> 
>
http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppapi_param_traits....
> ppapi/proxy/ppapi_param_traits.cc:63: if (type != PP_FILETYPE_REGULAR ||
> Thanks for catching this.

[brettw, 2010-11-11 20:49:19]

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_pdf_proxy.cc
File ppapi/proxy/ppb_pdf_proxy.cc (right):

http://codereview.chromium.org/4752008/diff/1/ppapi/proxy/ppb_pdf_proxy.cc#ne...
ppapi/proxy/ppb_pdf_proxy.cc:105: NULL,  // &GetLocalizedString,
On 2010/11/11 20:27:51, viettrungluu wrote:
> Are these TODOs?

Not unless we want to run PDF out of process.

[viettrungluu, 2010-11-11 21:09:56]

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (right):

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.cc#ne...
ipc/ipc_message_utils.cc:244: 
What about Log?

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.h#new...
ipc/ipc_message_utils.h:245: struct ParamTraits<unsigned short> {
It's a little odd to have param traits for |unsigned short| but not for |short|,
though admittedly uint16s probably see more use than int16s.

[brettw, 2010-11-11 21:35:46]

New snap up. Turns out the new IPC thing was only half done because I got
distracted in the middle, and deserialization didn't work right.

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

http://codereview.chromium.org/4752008/diff/37001/ipc/ipc_message_utils.h#new...
ipc/ipc_message_utils.h:245: struct ParamTraits<unsigned short> {
Yeah, I think we should just add stuff we actually need.

[viettrungluu, 2010-11-11 22:51:17]

LGTM.