Remove PP_Module from parameters for PPB_Var.VarFromUtf8.

Remove PP_Module from parameters for PPB_Var.VarFromUtf8.

This change has tendrils stretching throughout the code, but mostly this lets us delete a bunch of stuff.

This also does a slight refactor to put the PPB_Var implementations in 1 place, since they were already practically identical, and I didn't want to do the backwards-compat code in 2 places.

BUG=106596
TEST=N/A
TBR=darin,tony

darin,tony TBR for rubber-stamp of webkit/glue/webkit_glue.gypi

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

[dmichael (off chromium), 2011-12-07 04:40:47]

Most of the files were pretty mechanical, so it's not as bad as it looks.

bbudge: please review ppapi/native_client (sorry)
brettw: please review the rest (sorry)

[bbudge, 2011-12-07 15:53:28]

ppapi/native_client LGTM

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/tests/pp...
File ppapi/native_client/tests/ppapi_test_lib/test_interface.cc (left):

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/tests/pp...
ppapi/native_client/tests/ppapi_test_lib/test_interface.cc:33: PP_Var post_var =
PPBVar()->VarFromUtf8(pp_instance(),
This was passing the instance instead of the module?

[bbudge, 2011-12-07 16:00:34]

Missed a couple of nits. Still LGTM.

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
File ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc (right):

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc:123:
DeserializeTo(path_bytes.get(), length, 1 /* argc */, &path)) {
indentation + 2?

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc:124: return
path;
indentation - 2?

[brettw, 2011-12-07 17:40:53]

lgtm

http://codereview.chromium.org/8826011/diff/10001/ppapi/cpp/var.cc
File ppapi/cpp/var.cc (right):

http://codereview.chromium.org/8826011/diff/10001/ppapi/cpp/var.cc#newcode109
ppapi/cpp/var.cc:109: get_interface<PPB_Var>()->AddRef(var_);
There are a few other places in this file where we use get_interface. Doing
backwards-compat for the strings only helps if we do it for everything. You can
probably just use 1.0 for the rest of them since I think we're committing to
support it.

[dmichael (off chromium), 2011-12-07 18:07:38]

FWIW, I just manually tested a small example plugin
(ppapi/examples/scripting/post_message.html), and the plugin built with this
latest patchseemed to work with an older Chrome. The local build of Chrome with
this patch also appears to work with legacy NaCl apps.

Thanks for the fast reviews!

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
File ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc (right):

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc:123:
DeserializeTo(path_bytes.get(), length, 1 /* argc */, &path)) {
On 2011/12/07 16:00:34, bbudge1 wrote:
> indentation + 2?
Done.

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_file_ref.cc:124: return
path;
On 2011/12/07 16:00:34, bbudge1 wrote:
> indentation - 2?
Done.

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/tests/pp...
File ppapi/native_client/tests/ppapi_test_lib/test_interface.cc (left):

http://codereview.chromium.org/8826011/diff/2001/ppapi/native_client/tests/pp...
ppapi/native_client/tests/ppapi_test_lib/test_interface.cc:33: PP_Var post_var =
PPBVar()->VarFromUtf8(pp_instance(),
On 2011/12/07 15:53:28, bbudge1 wrote:
> This was passing the instance instead of the module?
Yep! Fortunately(?) we didn't really use the module for anything, so it was a
harmless bug.

http://codereview.chromium.org/8826011/diff/10001/ppapi/cpp/var.cc
File ppapi/cpp/var.cc (right):

http://codereview.chromium.org/8826011/diff/10001/ppapi/cpp/var.cc#newcode109
ppapi/cpp/var.cc:109: get_interface<PPB_Var>()->AddRef(var_);
On 2011/12/07 17:40:54, brettw wrote:
> There are a few other places in this file where we use get_interface. Doing
> backwards-compat for the strings only helps if we do it for everything. You
can
> probably just use 1.0 for the rest of them since I think we're committing to
> support it.
Done, nice catch.

[dmichael (off chromium), 2011-12-07 18:12:34]

Added darin and tony TBR for webkit/glue/webkit_glue.gypi change.

[commit-bot: I haz the power, 2011-12-07 18:30:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmichael@chromium.org/8826011/16002

[tony, 2011-12-07 19:08:41]

On 2011/12/07 18:12:34, dmichael wrote:
> Added darin and tony TBR for webkit/glue/webkit_glue.gypi change.

webkit/glue LGTM.

[commit-bot: I haz the power, 2011-12-07 20:45:59]

Can't apply patch for file ppapi/ppapi_shared.gypi.
While running patch -p1 --forward --force;
patching file ppapi/ppapi_shared.gypi
Hunk #1 FAILED at 62.
1 out of 1 hunk FAILED -- saving rejects to file ppapi/ppapi_shared.gypi.rej

[Matt Ball, 2011-12-08 00:25:38]

FYI: Looks like the NaCl SDK may need to be tweak just a bit to handle this
change:


hello_world_c/hello_world_c.c: In function 'CStrToVar':
hello_world_c/hello_world_c.c:73: warning: passing argument 1 of
'ppb_var_interface->VarFromUtf8' makes pointer from integer without a cast
hello_world_c/hello_world_c.c:73: note: expected 'const char *' but argument is
of type 'PP_Module'
hello_world_c/hello_world_c.c:73: warning: passing argument 2 of
'ppb_var_interface->VarFromUtf8' makes integer from pointer without a cast
hello_world_c/hello_world_c.c:73: note: expected 'uint32_t' but argument is of
type 'const char *'
hello_world_c/hello_world_c.c:73: error: too many arguments to function
'ppb_var_interface->VarFromUtf8'

[dmichael (off chromium), 2011-12-08 03:34:18]

Sorry about that, I forgot about the SDK not being behind a DEPS roll
anymore.

CL to fix it here:
http://codereview.chromium.org/8883008/

On Wed, Dec 7, 2011 at 5:25 PM, <mball@google.com> wrote:

> FYI: Looks like the NaCl SDK may need to be tweak just a bit to handle this
> change:
>
>
> hello_world_c/hello_world_c.c: In function 'CStrToVar':
> hello_world_c/hello_world_c.c:**73: warning: passing argument 1 of
> 'ppb_var_interface->**VarFromUtf8' makes pointer from integer without a
> cast
> hello_world_c/hello_world_c.c:**73: note: expected 'const char *' but
> argument is
> of type 'PP_Module'
> hello_world_c/hello_world_c.c:**73: warning: passing argument 2 of
> 'ppb_var_interface->**VarFromUtf8' makes integer from pointer without a
> cast
> hello_world_c/hello_world_c.c:**73: note: expected 'uint32_t' but
> argument is of
> type 'const char *'
> hello_world_c/hello_world_c.c:**73: error: too many arguments to function
> 'ppb_var_interface->**VarFromUtf8'
>
>
>
http://codereview.chromium.**org/8826011/<http://codereview.chromium.org/8826...
>