Implement an input event resource.

Add interfaces for requesting and receiving input event resources.

This converts the input event from a C struct to a resource to give us more
ability to change over time. This patch includes a proxy and a C++ wrapper for
this resource.

You now have to register for classes of input events. No events are sent by
default. This also allows us to specify whether the events support bubbling
or not, which allows us to better-optimize IPC.

TEST=none
BUG=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91711

[darin (slow to review), 2011-07-01 17:57:37]

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:280: * the input event, rather than sending the input
event asynchrohously. This
sp error: asynchrohously

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:294: uint32_t event_classes);
nit: indentation

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:313: uint32_t event_classes);
do we need this function?  can't plugins just call Request*InputEvents passing 0
to clear input events, or do the Request*InputEvents functions always indicate a
union operation?

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:373: int32_t (*GetMouseClickCount)(PP_Resource
mouse_event);
this is used to report double clicks and triple clicks

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:440: PP_Var (*GetCharacterText)(PP_Resource
character_event);
i'm a little worried that this interface is going to grow quite large and messy
once we add things like touch, ime, and game controller input in the future.  it
feels like we might be better served by creating separate interfaces for each
input event class.  i know that means more interface juggling and casting, but
it feels more scalable.

[brettw, 2011-07-01 17:59:36]

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:440: PP_Var (*GetCharacterText)(PP_Resource
character_event);
What do you imagine the C++ API looking like?

[darin (slow to review), 2011-07-01 18:06:24]

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:440: PP_Var (*GetCharacterText)(PP_Resource
character_event);
On 2011/07/01 17:59:36, brettw wrote:
> What do you imagine the C++ API looking like?

It could look like this:

switch (event.GetEventType()) {
  case PP_INPUTEVENT_TYPE_MOUSEDOWN:
    const MouseEvent mouse_event(event);
    // read properties from mouse_event
    break:

Since InputEvent is really just a container around a PP_Resource, it
is cheap to copy it.  We could make MouseEvent extend from InputEvent,
and have no other member variables.  The constructor could assert that
the given InputEvent is a mouse class event causing a debug build to
trip over the error.  In release builds, the only protection would be
at the PPB_MouseEvent interface, where it would assert that the given
event type is a mouse class event.

[dmichael (off chromium), 2011-07-01 20:02:36]

On 2011/07/01 18:06:24, darin wrote:
> http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
> File ppapi/c/ppb_input_event.h (right):
> 
>
http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
> ppapi/c/ppb_input_event.h:440: PP_Var (*GetCharacterText)(PP_Resource
> character_event);
> On 2011/07/01 17:59:36, brettw wrote:
> > What do you imagine the C++ API looking like?
> 
> It could look like this:
> 
> switch (event.GetEventType()) {
>   case PP_INPUTEVENT_TYPE_MOUSEDOWN:
>     const MouseEvent mouse_event(event);
>     // read properties from mouse_event
>     break:
Actually...  if we did it this way, the C++ wrapper in many cases could just
hold the data directly (and extract it at the time HandleInputEvent is called),
so that copies can be done without locking.  But I think that the C++ level can
be independent of whether we have multiple PPB interfaces.  I think for now just
1 seems fine; we can change it up later if necessary.
> 
> Since InputEvent is really just a container around a PP_Resource, it
> is cheap to copy it.  We could make MouseEvent extend from InputEvent,
> and have no other member variables.  The constructor could assert that
> the given InputEvent is a mouse class event causing a debug build to
> trip over the error.  In release builds, the only protection would be
> at the PPB_MouseEvent interface, where it would assert that the given
> event type is a mouse class event.

[dmichael (off chromium), 2011-07-01 20:04:19]

I like the new interfaces.  The no-filtering use case is really compelling, I
think.

I smattered some comments around about MouseEvent's location; it looks like
int/PP_Point is the right thing, but you can ignore them.  I realize now that
you meant to change it and it now matches what WebKit gives you.

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:221: * for this class of input.
Just for my own edification... Why this limitation?

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:294: uint32_t event_classes);
nit: indentation

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:368: PP_Point (*GetMousePosition)(PP_Resource
mouse_event);
I think you meant PP_FloatPoint?

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:440: PP_Var (*GetCharacterText)(PP_Resource
character_event);
The return here could reasonably char[5] like the old one, right?  That may be
somewhat more efficient than having to do VarToUTF8, which will require a map
lookup and so probably some locking in the multithreaded case.

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppp_input_event.h
File ppapi/c/ppp_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppp_input_event.h#ne...
ppapi/c/ppp_input_event.h:32: * handling.
Maybe point out that if it's not a filtering event, the return has no effect?

Maybe a reminder to Release the resource when you're done handling the event?

http://codereview.chromium.org/7285010/diff/8001/ppapi/cpp/point.h
File ppapi/cpp/point.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/cpp/point.h#newcode37
ppapi/cpp/point.h:37: /// @param[in] point A pointer to a PP_Point.
This should just say 'A PP_Point' I think.

http://codereview.chromium.org/7285010/diff/8001/ppapi/cpp/point.h#newcode146
ppapi/cpp/point.h:146: /// A constructor accepting two values for x and y and
converting / them to a
nit: extra '/' after converting, and again at 149 and 152

http://codereview.chromium.org/7285010/diff/8001/ppapi/cpp/point.h#newcode161
ppapi/cpp/point.h:161: /// @param[in] point A pointer to a PP_Point.
Copy/paste error duplication :-) I would just say 'A PP_Point'

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
File ppapi/proxy/ppb_instance_proxy.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
ppapi/proxy/ppb_instance_proxy.cc:206: // the flags so we can notify it if
anything was invalid, withougt requiring
withougt->without

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
ppapi/proxy/ppb_instance_proxy.cc:218: // the flags so we can notify it if
anything was invalid, withougt requiring
withougt->without

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
ppapi/proxy/ppb_instance_proxy.cc:226: INTERFACE_ID_PPB_INSTANCE, instance,
event_classes));
Because this message is asynchronous & the proxy doesn't do any filtering
(unless I missed it), this has the side effect that an o.o.p. plugin might get
input events after clearing them.

Kind of a minor point, but if you don't fix it in this CL (with maybe some
proxy, plugin-side filtering? Or the easier way of just making it sync?), it
would be good to file a bug.

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppp_input_event_...
File ppapi/proxy/ppp_input_event_proxy.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppp_input_event_...
ppapi/proxy/ppp_input_event_proxy.cc:111:
PluginResourceTracker::GetInstance()->ReleaseResource(event_resource);
Okay, scratch my earlier comment about reminder to decrement the reference
count.  Is this typical for passing resources?  Do they all do this?  It seems
like it should be documented that upon exit from the function, the resource ref
count is decremented automatically, so if a plugin wants to retain an input
event (for instance to use in another thread), it must AddRefResource it.

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppp_input_event_...
File ppapi/proxy/ppp_input_event_proxy.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppp_input_event_...
ppapi/proxy/ppp_input_event_proxy.h:28: return reinterpret_cast<const
PPP_InputEvent*>(target_interface());
static_cast

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
File ppapi/shared_impl/input_event_impl.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
ppapi/shared_impl/input_event_impl.cc:15: mouse_position(PP_MakePoint(0, 0)),
PP_MakeFloatPoint?

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
ppapi/shared_impl/input_event_impl.cc:50: PP_Point
InputEventImpl::GetMousePosition() {
I think you want PP_FloatPoint here

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
File ppapi/shared_impl/input_event_impl.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
ppapi/shared_impl/input_event_impl.h:43: // This simple class implements the
PPB_InputEvent_API in terms of the
looks like you didn't finish your thought.

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/input_even...
ppapi/shared_impl/input_event_impl.h:46: InputEventImpl(const InputEventData&
data);
explicit?

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/instance_i...
File ppapi/shared_impl/instance_impl.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/instance_i...
ppapi/shared_impl/instance_impl.cc:19: ~int32_t(PP_INPUTEVENT_CLASS_MOUSE |
PP_INPUTEVENT_CLASS_KEYBOARD |
uint32_t, static_cast instead of C-style cast

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/event_c...
File webkit/plugins/ppapi/event_conversion.cc (right):

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/event_c...
webkit/plugins/ppapi/event_conversion.cc:76: reinterpret_cast<const
WebKeyboardEvent&>(event);
static_cast

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1562: event_classes &
~uint32_t(PP_INPUTEVENT_CLASS_KEYBOARD);
static_cast

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1573: filtered_input_event_mask_
&= event_classes;
Shouldn't that be |=?

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1575: return
ValidateRequestInputEvents(true, event_classes);
Maybe it doesn't matter, but it looks from all of the above like
input_event_mask_ and filtered_input_event_mask_ can have invalid bits set by
plugins.  It might be worth masking them out to be safe in case it's ever
assumed anywhere that those bits are always 0?

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
File webkit/plugins/ppapi/ppapi_plugin_instance.h (right):

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.h:403: // corresponding intefaces, so
that we can ask only once.
intefaces->interfaces

[dmichael (off chromium), 2011-07-01 20:05:38]

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:368: PP_Point (*GetMousePosition)(PP_Resource
mouse_event);
On 2011/07/01 20:04:19, dmichael1 wrote:
> I think you meant PP_FloatPoint?
Retracted.  I think you just need to update the comment to take out the sentence
about floating point units.

[brettw, 2011-07-01 20:23:45]

I decided Darin was right about the interfaces so I separated it out.
Fortunately with the new API/thunk system, the changes are minimal and don't
require extra proxying.

I also implemented Darin's suggestion for the C++ wrapper. I originally didn't
like this, but I don't have a better idea, and I think it's more clear than
having everything in one. I changed all the constructors for the C++ wrappers to
typecheck the resource, so you can check is_null() to see if you've created it
with the wrong type.

[darin (slow to review), 2011-07-01 20:51:15]

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:290: * the input event, rather than sending the input
event asynchrohously. This
nit: asynchrohously

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:304: uint32_t event_classes);
nit: indentation

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:335: PP_InputEvent_Type (*GetEventType)(PP_Resource
event);
nit: maybe just call these GetType, GetTimeStamp, and GetModifiers?  since these
are on the event interface, it is OK to leave out the "Event" word?

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:453: PP_Bool (*GetWheelScrollByPage)(PP_Resource
wheel_event);
nit: GetScrollByPage?  (the "Wheel" prefix isn't important now)

http://codereview.chromium.org/7285010/diff/8007/ppapi/cpp/input_event.h
File ppapi/cpp/input_event.h (right):

http://codereview.chromium.org/7285010/diff/8007/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:69: 
nit: whitespace

http://codereview.chromium.org/7285010/diff/8007/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:150: 
me being annoying (ignore if you like): should we be trying to have one
interface per file?  do we want people to add touch, ime, game controller events
in this file?

[brettw, 2011-07-01 21:14:52]

New snap up.

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:368: PP_Point (*GetMousePosition)(PP_Resource
mouse_event);
Yeah. We get ints and all current plugins expect ints and do annoying casting to
get the ints back from the floats.

Now that we can rev the interface, if we support high resolution devices, we can
add a separate high res position getter.

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppp_input_event.h
File ppapi/c/ppp_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/c/ppp_input_event.h#ne...
ppapi/c/ppp_input_event.h:32: * handling.
You don't have to release the resource (since this is a function call, the
caller hangs onto one for you). I tried to explain this in the new comment, but
it's difficult.

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
File ppapi/proxy/ppb_instance_proxy.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/proxy/ppb_instance_pro...
ppapi/proxy/ppb_instance_proxy.cc:226: INTERFACE_ID_PPB_INSTANCE, instance,
event_classes));
Making this message sync doesn't help because there could already be a bunch of
messages "in flight" at the time of the call that are queued up. I will mention
that this can happen.

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/instance_i...
File ppapi/shared_impl/instance_impl.cc (right):

http://codereview.chromium.org/7285010/diff/8001/ppapi/shared_impl/instance_i...
ppapi/shared_impl/instance_impl.cc:19: ~int32_t(PP_INPUTEVENT_CLASS_MOUSE |
PP_INPUTEVENT_CLASS_KEYBOARD |
I changed it since it isn't normal for us to do this, although for the record
this isn't technically a C-style cast.

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/event_c...
File webkit/plugins/ppapi/event_conversion.cc (right):

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/event_c...
webkit/plugins/ppapi/event_conversion.cc:76: reinterpret_cast<const
WebKeyboardEvent&>(event);
On 2011/07/01 20:04:19, dmichael1 wrote:
> static_cast

Done.

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

http://codereview.chromium.org/7285010/diff/8001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1575: return
ValidateRequestInputEvents(true, event_classes);
I thought about this but it's kind of a pain to implement masking off the bits
we don't know about, and then this is another place to forget to update when we
add more.

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/8007/ppapi/c/ppb_input_event.h#ne...
ppapi/c/ppb_input_event.h:453: PP_Bool (*GetWheelScrollByPage)(PP_Resource
wheel_event);
I also added this getter for the C++ wrapper (I forgot it before).

http://codereview.chromium.org/7285010/diff/8007/ppapi/cpp/input_event.h
File ppapi/cpp/input_event.h (right):

http://codereview.chromium.org/7285010/diff/8007/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:150: 
I thought about it and was on the fence, but decided it would be annoying to
have to include all those in your source file. I can change it if people feel
strongly.

[dmichael (off chromium), 2011-07-01 21:35:18]

LGTM

[brettw, 2011-07-01 22:08:44]

I forgot to add ppb_input_event_impl.* to the CL, so I just added that. It's
trivial if you want to take a look.

[kochi, 2011-07-04 13:39:56]

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h#n...
ppapi/c/ppb_input_event.h:231: * for this class of input.
I'm still not clear about why KEYBOARD class is special-cased here.
This comment is useful for plugin developers, but for interface developers
like me, e.g. if I want to add PP_INPUTEVENT_CLASS_IME, should I also
special-case that users must not call this with PP_INPUTEVENT_CLASS_IME?

[brettw, 2011-07-04 16:21:23]

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h#n...
ppapi/c/ppb_input_event.h:231: * for this class of input.
The reason I special-cased keyboard is that taking keyboard events and not
saying you don't handle some will mess up the page because some commands will
get lost.

It's likely we'll change around how this works in a subsequent pass of this
interface. I know Erik wanted to discuss a way not to have this restriction.

[dmichael (off chromium), 2011-07-05 15:37:22]

Your try runs look unhappy; I think you have extra semicolons in
ppapi_messages.h

It would be good to roll in a brief explanation of click count, since Darin
knows what it is?

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h#n...
ppapi/c/ppb_input_event.h:231: * for this class of input.
On 2011/07/04 16:21:23, brettw wrote:
> The reason I special-cased keyboard is that taking keyboard events and not
> saying you don't handle some will mess up the page because some commands will
> get lost.
> 
> It's likely we'll change around how this works in a subsequent pass of this
> interface. I know Erik wanted to discuss a way not to have this restriction.
Thanks for explaining; that makes sense.  So the thinking is maybe that we'll
possibly add a finer-grained ability to filter down the road, like 'handle this
set of keys' or something?

http://codereview.chromium.org/7285010/diff/15020/ppapi/cpp/input_event.h
File ppapi/cpp/input_event.h (right):

http://codereview.chromium.org/7285010/diff/15020/ppapi/cpp/input_event.h#new...
ppapi/cpp/input_event.h:40: InputEvent(PP_Resource input_event_resource);
explicit

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

http://codereview.chromium.org/7285010/diff/15020/ppapi/proxy/ppapi_messages....
ppapi/proxy/ppapi_messages.h:664: uint32_t /* event_classes */);
You're getting compile errors here...  probably just need to drop the semicolons

http://codereview.chromium.org/7285010/diff/15020/webkit/plugins/ppapi/ppb_in...
File webkit/plugins/ppapi/ppb_input_event_impl.h (right):

http://codereview.chromium.org/7285010/diff/15020/webkit/plugins/ppapi/ppb_in...
webkit/plugins/ppapi/ppb_input_event_impl.h:29: virtual PP_Var
StringToPPVar(const std::string& str) OVERRIDE;
Linter is pointing out you should include <string>

[brettw, 2011-07-06 16:31:43]

New snap up.

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h
File ppapi/c/ppb_input_event.h (right):

http://codereview.chromium.org/7285010/diff/15020/ppapi/c/ppb_input_event.h#n...
ppapi/c/ppb_input_event.h:231: * for this class of input.
Yeah, something like that.

http://codereview.chromium.org/7285010/diff/15020/webkit/plugins/ppapi/ppb_in...
File webkit/plugins/ppapi/ppb_input_event_impl.h (right):

http://codereview.chromium.org/7285010/diff/15020/webkit/plugins/ppapi/ppb_in...
webkit/plugins/ppapi/ppb_input_event_impl.h:29: virtual PP_Var
StringToPPVar(const std::string& str) OVERRIDE;
Seems wrong since this is an override of a base class that does include <string>

[dmichael (off chromium), 2011-07-06 17:41:00]

LGTM

Suggestion for a future CL: Maybe we should add a function on PPB_Testing to
inject input events so that we can easily test the rest of the machinery.

[darin (slow to review), 2011-07-06 18:05:41]

+1

The WebKit layout test system has this, and it is really helpful.

-Darin

On Wed, Jul 6, 2011 at 10:41 AM, <dmichael@chromium.org> wrote:

> LGTM
>
> Suggestion for a future CL: Maybe we should add a function on PPB_Testing
> to
> inject input events so that we can easily test the rest of the machinery.
>
>
>
http://codereview.chromium.**org/7285010/<http://codereview.chromium.org/7285...
>