Make the file chooser use PP_ArrayOutput

New file chooser interface that uses the new PP_ArrayOutput feature. This also changes PP_ArrayOutput to be pass-by-value.

This keeps backwards compat for the old interface. It fixes some bugs in the callback system that I found when working on the patch and adds some new machinery for doing array output in the proxy. It also re-enables the file chooser feature which was recently broken.

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

[brettw, 2012-03-26 16:12:30]

This fixes the file chooser and is am M19 blocker.

[viettrungluu, 2012-03-26 16:55:18]

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/dev/file_chooser_dev.cc
File ppapi/cpp/dev/file_chooser_dev.cc (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/dev/file_chooser_dev...
ppapi/cpp/dev/file_chooser_dev.cc:52: } else if
(has_interface<PPB_FileChooser_Dev_0_5>()) {
Nit: no |else| needed.

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/dev/file_chooser_dev...
ppapi/cpp/dev/file_chooser_dev.cc:53: // Data for out callback. The callback
handler will delete it.
"out"?

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/trusted/file_chooser...
File ppapi/cpp/trusted/file_chooser_trusted.cc (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/trusted/file_chooser...
ppapi/cpp/trusted/file_chooser_trusted.cc:64: } else if
(has_interface<PPB_FileChooserTrusted_0_5>()) {
No |else|, apparently.

http://codereview.chromium.org/9728001/diff/32/ppapi/cpp/trusted/file_chooser...
ppapi/cpp/trusted/file_chooser_trusted.cc:65: // Data for out callback. The
callback handler will delete it.
"out"?

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h
File ppapi/proxy/ppapi_messages.h (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h#n...
ppapi/proxy/ppapi_messages.h:681: PP_Bool /* save_as */,
Is there any particular advantage to this change?

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h#n...
ppapi/proxy/ppapi_messages.h:682: ppapi::proxy::SerializedVar /*
suggested_file_name */,
And this one?

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppb_file_chooser_p...
File ppapi/proxy/ppb_file_chooser_proxy.cc (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppb_file_chooser_p...
ppapi/proxy/ppb_file_chooser_proxy.cc:303:
scoped_refptr<RefCountedArrayOutputAdapter<PP_Resource> >
"

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppb_file_chooser_p...
File ppapi/proxy/ppb_file_chooser_proxy.h (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppb_file_chooser_p...
ppapi/proxy/ppb_file_chooser_proxy.h:66:
scoped_refptr<RefCountedArrayOutputAdapter<PP_Resource> >
no need to split this line

http://codereview.chromium.org/9728001/diff/32/ppapi/shared_impl/array_writer.cc
File ppapi/shared_impl/array_writer.cc (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/shared_impl/array_writer...
ppapi/shared_impl/array_writer.cc:73: std::copy(input.begin(), input.end(),
static_cast<PP_Resource*>(dest));
You need to include <algorithm>.

http://codereview.chromium.org/9728001/diff/32/ppapi/shared_impl/array_writer.h
File ppapi/shared_impl/array_writer.h (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/shared_impl/array_writer...
ppapi/shared_impl/array_writer.h:55: sizeof(PP_Resource));
sizeof(T)

http://codereview.chromium.org/9728001/diff/32/ppapi/shared_impl/array_writer...
ppapi/shared_impl/array_writer.h:66: memcpy(dest, &input[0], sizeof(T) *
input.size());
include <string.h>

[brettw, 2012-03-26 17:08:05]

New snap up PTAL

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h
File ppapi/proxy/ppapi_messages.h (right):

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h#n...
ppapi/proxy/ppapi_messages.h:681: PP_Bool /* save_as */,
On 2012/03/26 16:55:19, viettrungluu wrote:
> Is there any particular advantage to this change?

This saves conversion code. This way to only convert from PP_Bool-> bool once.

http://codereview.chromium.org/9728001/diff/32/ppapi/proxy/ppapi_messages.h#n...
ppapi/proxy/ppapi_messages.h:682: ppapi::proxy::SerializedVar /*
suggested_file_name */,
Ditto, otherwise we need to convert from a PP_Var to a string in the proxy, then
back to a PP_Var, then back to a string. The old code had a std::string in the
_APi class and did the string conversion in the thunk layer which is against our
rules (thunks should be literally just thunks to aid in future autogeneration).

[viettrungluu, 2012-03-26 19:51:26]

Oops, missed something the first time around. LGTM with that fixed.

http://codereview.chromium.org/9728001/diff/11008/ppapi/cpp/dev/file_chooser_...
File ppapi/cpp/dev/file_chooser_dev.cc (right):

http://codereview.chromium.org/9728001/diff/11008/ppapi/cpp/dev/file_chooser_...
ppapi/cpp/dev/file_chooser_dev.cc:37: } else if
(has_interface<PPB_FileChooser_Dev_0_6>()) {
Reverse the order of these |if|s.

[commit-bot: I haz the power, 2012-03-26 19:58:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brettw@chromium.org/9728001/9006

[commit-bot: I haz the power, 2012-03-26 19:59:10]

Presubmit check for 9728001-9006 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit Warnings **
Missing PPAPI header, no change or skipped generation?

***************
  ppapi/c/pp_array_output.idl
***************

Presubmit checks took 4.3s to calculate.