add find hookup

add find hookup
BUG=none
TEST=compiles

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

[wjia(left Chromium), 2010-07-10 03:26:46]

This patch depends on patch http://codereview.chromium.org/2972004/show.

There are some issues undecided:
1. query PPP_FIND_INTERFACE on demand?
2. keep find related functions in pepper_plugin_instance or move them to
pepper_find?

After this patch, I will finish cpp wrapper for find.

[wjia(left Chromium), 2010-07-10 03:27:36]

his patch depends on patch http://codereview.chromium.org/2972004/show.

There are some issues undecided:
1. query PPP_FIND_INTERFACE on demand?
2. keep find related functions in pepper_plugin_instance or move them to
pepper_find?

After this patch, I will finish cpp wrapper for find.

[brettw, 2010-07-12 05:06:32]

http://codereview.chromium.org/2896009/diff/1/2
File webkit/glue/plugins/pepper_find.cc (right):

http://codereview.chromium.org/2896009/diff/1/2#newcode14
webkit/glue/plugins/pepper_find.cc:14: void
NumberOfFindResultsChanged(PP_Instance instance_id,
I think this code can just go in the instance class. I'm not sure it's worth an
extra file just for these simple pass-throughs.

http://codereview.chromium.org/2896009/diff/1/3
File webkit/glue/plugins/pepper_find.h (right):

http://codereview.chromium.org/2896009/diff/1/3#newcode16
webkit/glue/plugins/pepper_find.h:16: explicit Find() { }
Note: style guide says no space in the {}

http://codereview.chromium.org/2896009/diff/1/4
File webkit/glue/plugins/pepper_plugin_instance.cc (right):

http://codereview.chromium.org/2896009/diff/1/4#newcode177
webkit/glue/plugins/pepper_plugin_instance.cc:177: // Make it on-demand?
Yes, I'd make it on demand. Plus, since some plugins might not implement this
interface, you will have to keep three states: haven't asked yet, plugin doesn't
support the interface and no functions should be called, and plugin supports the
interface and I have the pointer.

[jam, 2010-07-12 05:52:11]

http://codereview.chromium.org/2896009/diff/1/4
File webkit/glue/plugins/pepper_plugin_instance.cc (right):

http://codereview.chromium.org/2896009/diff/1/4#newcode356
webkit/glue/plugins/pepper_plugin_instance.cc:356: bool
PluginInstance::SupportsFind() {
this function doesn't exist anymore, since StartFind now returns a bool value. 
Please sync to get r52040

[wjia(left Chromium), 2010-07-12 18:32:12]

Now find related functions are in pepper_plugin_instance. Hopefully this file
won't be swollen that much.

[brettw, 2010-07-12 19:49:44]

LGTM, just two suggestions to simplify things a bit.

http://codereview.chromium.org/2896009/diff/9001/10002
File webkit/glue/plugins/pepper_plugin_instance.h (right):

http://codereview.chromium.org/2896009/diff/9001/10002#newcode51
webkit/glue/plugins/pepper_plugin_instance.h:51: static const PPB_Find*
GetInterface();
I think we can just have this be in the Instance class so you don't need this
Find class at all.

http://codereview.chromium.org/2896009/diff/9001/10002#newcode154
webkit/glue/plugins/pepper_plugin_instance.h:154: const PPP_Find*
plugin_find_interface_;
You can just have this be a local in the anonymous namespace in the .cc file.