Add GetUsbKeyCode dev interface for Pepper key events

Add GetUsbKeyCode _Dev interface for Pepper key events

Remove CreatePPEvent since it isn't used anywhere.

BUG=none
TEST=none

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

[Wez, 2012-02-07 23:48:13]

http://codereview.chromium.org/9353013/diff/2001/ppapi/api/ppb_input_event.idl
File ppapi/api/ppb_input_event.idl (right):

http://codereview.chromium.org/9353013/diff/2001/ppapi/api/ppb_input_event.id...
ppapi/api/ppb_input_event.idl:234: * filtered mode so you can pass on the
events.
nit: I'm not convinced that this is true for low-level input.

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/event_c...
File webkit/plugins/ppapi/event_conversion.h (right):

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/event_c...
webkit/plugins/ppapi/event_conversion.h:31: // to be returned for key events
(instead of Windows virtual keycodes).
We should see if we can find a better way to do this.

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

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1905: event_classes |=
PP_INPUTEVENT_CLASS_KEYBOARD;
Can we not just require that the caller supply both flags, or define
PP_INPUTEVENT_CLASS_LOWLEVEL_KEYBOARD_Dev as a combination of the two?

[garykac, 2012-02-09 14:26:03]

I'll be out this morning, so I wanted to send this out for a quick sanity check.

The vast majority of this CL is fallout from the various thunks and rpcs.

I still need to test properly and verify that I've fixed up all the calls to
CreateKeyboardInputEvent, but I'd like to get confirmation that this approach is
good while I dig into that.

Thanks!

http://codereview.chromium.org/9353013/diff/2001/ppapi/api/ppb_input_event.idl
File ppapi/api/ppb_input_event.idl (right):

http://codereview.chromium.org/9353013/diff/2001/ppapi/api/ppb_input_event.id...
ppapi/api/ppb_input_event.idl:234: * filtered mode so you can pass on the
events.
On 2012/02/07 23:48:13, Wez wrote:
> nit: I'm not convinced that this is true for low-level input.

No longer applicable.

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/event_c...
File webkit/plugins/ppapi/event_conversion.h (right):

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/event_c...
webkit/plugins/ppapi/event_conversion.h:31: // to be returned for key events
(instead of Windows virtual keycodes).
On 2012/02/07 23:48:13, Wez wrote:
> We should see if we can find a better way to do this.

Done.

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

http://codereview.chromium.org/9353013/diff/2001/webkit/plugins/ppapi/ppapi_p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:1905: event_classes |=
PP_INPUTEVENT_CLASS_KEYBOARD;
On 2012/02/07 23:48:13, Wez wrote:
> Can we not just require that the caller supply both flags, or define
> PP_INPUTEVENT_CLASS_LOWLEVEL_KEYBOARD_Dev as a combination of the two?

No longer applicable.

http://codereview.chromium.org/9353013/diff/7001/ppapi/native_client/src/shar...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/7001/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:43:
PP_COMPILE_ASSERT_SIZE_IN_BYTES(InputEventData, 72);
I naively expected this to be 68.

[brettw, 2012-02-09 19:19:43]

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.idl
File ppapi/api/ppb_input_event.idl (right):

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.id...
ppapi/api/ppb_input_event.idl:731: uint32_t GetUsbScanCode_Dev([in] PP_Resource
key_event);
We need to keep backwards compat for old users of this interface. You can see
how MouseInputEvent has two versions, 1.1 and 1.0, and two create functions.
You'll also need to set the version on your new function to be 1.1. 

Over IM, I thought we had discussed putting the accessor on a new dev interface.
We don't really have a facility to add dev functions to non-dev interfaces. So
either I would make a separate interface in the dev directory, or leave off the
_Dev for this name (knowing that we're committing to supporting the function).

[garykac, 2012-02-14 00:27:33]

On 2012/02/09 19:19:43, brettw wrote:
> http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.idl
> File ppapi/api/ppb_input_event.idl (right):
> 
>
http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.id...
> ppapi/api/ppb_input_event.idl:731: uint32_t GetUsbScanCode_Dev([in]
PP_Resource
> key_event);
> We need to keep backwards compat for old users of this interface. You can see
> how MouseInputEvent has two versions, 1.1 and 1.0, and two create functions.
> You'll also need to set the version on your new function to be 1.1. 
> 
> Over IM, I thought we had discussed putting the accessor on a new dev
interface.
> We don't really have a facility to add dev functions to non-dev interfaces. So
> either I would make a separate interface in the dev directory, or leave off
the
> _Dev for this name (knowing that we're committing to supporting the function).

Done. Moved into a separate dev interface and set to version 0.1.

[dmichael (off chromium), 2012-02-14 19:40:45]

http://codereview.chromium.org/9353013/diff/5002/ppapi/native_client/src/shar...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/5002/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:38: uint32_t
unused_padding;
drive-by...  please also initialize these (including unused_padding) in
input_event_data.cc

[garykac, 2012-02-14 23:21:16]

http://codereview.chromium.org/9353013/diff/5002/ppapi/native_client/src/shar...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/5002/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:38: uint32_t
unused_padding;
On 2012/02/14 19:40:46, dmichael wrote:
> drive-by...  please also initialize these (including unused_padding) in
> input_event_data.cc

Done.

[brettw, 2012-02-15 00:42:44]

lgtm

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

http://codereview.chromium.org/9353013/diff/6024/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:305: /// PPB_KeyboardInputEvent_Dev interface.
I prefer not to have Dev interfaces in the C++ wrappers except in a dev
directory. In this case, I don't think it's a huge problem to just not bother
with C++ wrappers for the USB stuff. For the few people that want to test this
out, they can just call into the C API.

[dmichael (off chromium), 2012-02-15 15:49:26]

LGTM (modulo brettw's comment about dev in the C++ wrapper)

[Wez, 2012-02-16 19:18:51]

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.idl
File ppapi/api/ppb_input_event.idl (right):

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.id...
ppapi/api/ppb_input_event.idl:731: uint32_t GetUsbScanCode_Dev([in] PP_Resource
key_event);
On 2012/02/09 19:19:44, brettw wrote:
> We need to keep backwards compat for old users of this interface. You can see
> how MouseInputEvent has two versions, 1.1 and 1.0, and two create functions.
> You'll also need to set the version on your new function to be 1.1. 
> 
> Over IM, I thought we had discussed putting the accessor on a new dev
interface.
> We don't really have a facility to add dev functions to non-dev interfaces. So
> either I would make a separate interface in the dev directory, or leave off
the
> _Dev for this name (knowing that we're committing to supporting the function).

Yes, the plan is to have a new dev interface with a single accessor that takes
an input-event PP_Resource and returns the USB-style key code.  This patch-set
pre-dates our IM, and deals more with the plumbing side of things.

Gary: Is the new patch-set almost ready for review?

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

http://codereview.chromium.org/9353013/diff/6024/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:305: /// PPB_KeyboardInputEvent_Dev interface.
On 2012/02/15 00:42:44, brettw wrote:
> I prefer not to have Dev interfaces in the C++ wrappers except in a dev
> directory. In this case, I don't think it's a huge problem to just not bother
> with C++ wrappers for the USB stuff. For the few people that want to test this
> out, they can just call into the C API.

Agreed; the API is sufficiently simple that this shouldn't pose a problem.  As
for the C APIs, the C++ wrappers outside a dev/ sub-directory should only
contain stuff that calling code can rely on.

[garykac, 2012-02-16 20:45:03]

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.idl
File ppapi/api/ppb_input_event.idl (right):

http://codereview.chromium.org/9353013/diff/7001/ppapi/api/ppb_input_event.id...
ppapi/api/ppb_input_event.idl:731: uint32_t GetUsbScanCode_Dev([in] PP_Resource
key_event);
On 2012/02/16 19:18:51, Wez wrote:
> On 2012/02/09 19:19:44, brettw wrote:
> > We need to keep backwards compat for old users of this interface. You can
see
> > how MouseInputEvent has two versions, 1.1 and 1.0, and two create functions.
> > You'll also need to set the version on your new function to be 1.1. 
> > 
> > Over IM, I thought we had discussed putting the accessor on a new dev
> interface.
> > We don't really have a facility to add dev functions to non-dev interfaces.
So
> > either I would make a separate interface in the dev directory, or leave off
> the
> > _Dev for this name (knowing that we're committing to supporting the
function).
> 
> Yes, the plan is to have a new dev interface with a single accessor that takes
> an input-event PP_Resource and returns the USB-style key code.  This patch-set
> pre-dates our IM, and deals more with the plumbing side of things.
> 
> Gary: Is the new patch-set almost ready for review?

Patchset 5 and beyond have the updated code (where there's a separate dev
interface that contain the USB accessors).

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

http://codereview.chromium.org/9353013/diff/6024/ppapi/cpp/input_event.h#newc...
ppapi/cpp/input_event.h:305: /// PPB_KeyboardInputEvent_Dev interface.
On 2012/02/15 00:42:44, brettw wrote:
> I prefer not to have Dev interfaces in the C++ wrappers except in a dev
> directory. In this case, I don't think it's a huge problem to just not bother
> with C++ wrappers for the USB stuff. For the few people that want to test this
> out, they can just call into the C API.

Done (patchset 13)

[commit-bot: I haz the power, 2012-02-16 23:15:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9353013/30003

[commit-bot: I haz the power, 2012-02-16 23:15:30]

Presubmit check for 9353013-30003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.cc \
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h \
  ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_input_event.h

Presubmit checks took 3.5s to calculate.

[Wez, 2012-02-17 01:31:01]

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
File ppapi/api/dev/ppb_keyboard_input_event_dev.idl (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:8: * which provides access to the
raw USB scancodes for key input events.
nit: It's not _the_ raw USB scancodes, but USB key codes identifying the
physical keys pressed - the interface doesn't get the details directly from USB.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:22: * This sets the usb_scan_code
in the given <code>resource</code>. It is
nit: ... the given PP_InputEvent resource?

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:22: * This sets the usb_scan_code
in the given <code>resource</code>. It is
nit: ... sets the usb_scan_code ... -> ... set a USB key code ...

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:28: * method and this method will
be removed.
nit: I don't think this comment is necessary to understand this interface, and
we might keep USB key codes as a separate interface.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:33: * @param[in] usb_scan_code
The USB scancode to assign for this
nit: assign for this -> assign to this (or associated with this)

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:36: * @return
<code>PP_TRUE</code> if the usb scancode was set successfully.
Under what circumstances might setting the scancode fail?

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:43: * that generated this
keyboard event.
nit: ... that corresponds ... this keyboard event -> ... that is associated with
this event.

If the intent is that GetUsbScanCode() is the API plugins will normally care
about, and SetUsbScanCode() is there only for completeness then consider putting
GetUsbScanCode() first, and having the comment on it describe what the USB key
code comes from.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:48: * @return The USB scancode
field for the keyboard event.
What if no USB key code was set, e.g. if the input event was generated by
JavaScript?

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.cc
File ppapi/cpp/input_event.cc (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.cc#ne...
ppapi/cpp/input_event.cc:26: return PPB_KEYBOARD_INPUT_EVENT_DEV_INTERFACE;
Nor this.

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

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.h#new...
ppapi/cpp/input_event.h:10: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
Don't need this.

http://codereview.chromium.org/9353013/diff/30003/ppapi/native_client/src/sha...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/native_client/src/sha...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:35: uint32_t
usb_scan_code;
nit: The term "scan code" is used for PS/2; the USB HID Usage Tables refer to
"key codes".  Strictly the codes we're producing are comprised of the USB Page
and Usage codes for the keys.

http://codereview.chromium.org/9353013/diff/30003/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:19: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
Do you need this here?

http://codereview.chromium.org/9353013/diff/30003/remoting/client/plugin/pepp...
File remoting/client/plugin/pepper_input_handler.cc (right):

http://codereview.chromium.org/9353013/diff/30003/remoting/client/plugin/pepp...
remoting/client/plugin/pepper_input_handler.cc:38: // TODO(garykac): Switch to
use |pp_key_event.GetUsbScanCode()|.
I don't think this comment belongs in this CL?

http://codereview.chromium.org/9353013/diff/30003/webkit/plugins/ppapi/plugin...
File webkit/plugins/ppapi/plugin_module.cc (right):

http://codereview.chromium.org/9353013/diff/30003/webkit/plugins/ppapi/plugin...
webkit/plugins/ppapi/plugin_module.cc:30: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
Is this needed for the #include <interface...> lines later on?

[garykac, 2012-02-17 18:25:30]

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
File ppapi/api/dev/ppb_keyboard_input_event_dev.idl (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:8: * which provides access to the
raw USB scancodes for key input events.
On 2012/02/17 01:31:01, Wez wrote:
> nit: It's not _the_ raw USB scancodes, but USB key codes identifying the
> physical keys pressed - the interface doesn't get the details directly from
USB.

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:22: * This sets the usb_scan_code
in the given <code>resource</code>. It is
On 2012/02/17 01:31:01, Wez wrote:
> nit: ... the given PP_InputEvent resource?

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:22: * This sets the usb_scan_code
in the given <code>resource</code>. It is
On 2012/02/17 01:31:01, Wez wrote:
> nit: ... sets the usb_scan_code ... -> ... set a USB key code ... 

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:28: * method and this method will
be removed.
On 2012/02/17 01:31:01, Wez wrote:
> nit: I don't think this comment is necessary to understand this interface, and
> we might keep USB key codes as a separate interface.

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:33: * @param[in] usb_scan_code
The USB scancode to assign for this
On 2012/02/17 01:31:01, Wez wrote:
> nit: assign for this -> assign to this (or associated with this)

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:36: * @return
<code>PP_TRUE</code> if the usb scancode was set successfully.
On 2012/02/17 01:31:01, Wez wrote:
> Under what circumstances might setting the scancode fail?

If the thunking can't acquire the lock to update the resource. This failure case
is true for all of these api calls.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:43: * that generated this
keyboard event.
On 2012/02/17 01:31:01, Wez wrote:
> nit: ... that corresponds ... this keyboard event -> ... that is associated
with
> this event.
> 
> If the intent is that GetUsbScanCode() is the API plugins will normally care
> about, and SetUsbScanCode() is there only for completeness then consider
putting
> GetUsbScanCode() first, and having the comment on it describe what the USB key
> code comes from.

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:48: * @return The USB scancode
field for the keyboard event.
On 2012/02/17 01:31:01, Wez wrote:
> What if no USB key code was set, e.g. if the input event was generated by
> JavaScript?

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.cc
File ppapi/cpp/input_event.cc (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.cc#ne...
ppapi/cpp/input_event.cc:26: return PPB_KEYBOARD_INPUT_EVENT_DEV_INTERFACE;
On 2012/02/17 01:31:01, Wez wrote:
> Nor this.

Done.

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

http://codereview.chromium.org/9353013/diff/30003/ppapi/cpp/input_event.h#new...
ppapi/cpp/input_event.h:10: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
On 2012/02/17 01:31:01, Wez wrote:
> Don't need this.

Done.

http://codereview.chromium.org/9353013/diff/30003/ppapi/native_client/src/sha...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/native_client/src/sha...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:35: uint32_t
usb_scan_code;
On 2012/02/17 01:31:01, Wez wrote:
> nit: The term "scan code" is used for PS/2; the USB HID Usage Tables refer to
> "key codes".  Strictly the codes we're producing are comprised of the USB Page
> and Usage codes for the keys. 

Changed to usb_key_code and added a comment describing the format.

http://codereview.chromium.org/9353013/diff/30003/ppapi/proxy/interface_list.cc
File ppapi/proxy/interface_list.cc (right):

http://codereview.chromium.org/9353013/diff/30003/ppapi/proxy/interface_list....
ppapi/proxy/interface_list.cc:19: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
On 2012/02/17 01:31:01, Wez wrote:
> Do you need this here?

Yes. Or else interfaces_ppb_public_dev.h gets unhappy.

http://codereview.chromium.org/9353013/diff/30003/remoting/client/plugin/pepp...
File remoting/client/plugin/pepper_input_handler.cc (right):

http://codereview.chromium.org/9353013/diff/30003/remoting/client/plugin/pepp...
remoting/client/plugin/pepper_input_handler.cc:38: // TODO(garykac): Switch to
use |pp_key_event.GetUsbScanCode()|.
On 2012/02/17 01:31:01, Wez wrote:
> I don't think this comment belongs in this CL?

Done.

http://codereview.chromium.org/9353013/diff/30003/webkit/plugins/ppapi/plugin...
File webkit/plugins/ppapi/plugin_module.cc (right):

http://codereview.chromium.org/9353013/diff/30003/webkit/plugins/ppapi/plugin...
webkit/plugins/ppapi/plugin_module.cc:30: #include
"ppapi/c/dev/ppb_keyboard_input_event_dev.h"
On 2012/02/17 01:31:01, Wez wrote:
> Is this needed for the #include <interface...> lines later on?

No longer needed. Removed.

[Wez, 2012-02-19 22:34:22]

lgtm

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
File ppapi/api/dev/ppb_keyboard_input_event_dev.idl (right):

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:8: * which provides access to USB
scancodes that identify the physical key being
nit: scancodes -> key codes

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:33: * @return
<code>PP_TRUE</code> if the usb key code was set successfully.
nit: usb -> USB

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:42: * @param[in] key_event A
<code>PP_Resource</code> corresponding to a
nit: The event for which to return the key code.

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:46: * no USB scancode associated
with this event.
What does it return if you pass a non key event?

http://codereview.chromium.org/9353013/diff/38001/ppapi/native_client/src/sha...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/38001/ppapi/native_client/src/sha...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:36: // The Usage
Page (upper 16-bits) and Usage Id (lower 16-bits) that encode
nit: Suggest "A code identifying the key by a USB Usage Page & Id.  See ..."

[garykac, 2012-02-21 18:42:21]

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
File ppapi/api/dev/ppb_keyboard_input_event_dev.idl (right):

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:8: * which provides access to USB
scancodes that identify the physical key being
On 2012/02/19 22:34:22, Wez wrote:
> nit: scancodes -> key codes

Done.

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:33: * @return
<code>PP_TRUE</code> if the usb key code was set successfully.
On 2012/02/19 22:34:22, Wez wrote:
> nit: usb -> USB

Done.

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:42: * @param[in] key_event A
<code>PP_Resource</code> corresponding to a
On 2012/02/19 22:34:22, Wez wrote:
> nit: The event for which to return the key code.

Done.

http://codereview.chromium.org/9353013/diff/38001/ppapi/api/dev/ppb_keyboard_...
ppapi/api/dev/ppb_keyboard_input_event_dev.idl:46: * no USB scancode associated
with this event.
On 2012/02/19 22:34:22, Wez wrote:
> What does it return if you pass a non key event?

Done.

http://codereview.chromium.org/9353013/diff/38001/ppapi/native_client/src/sha...
File ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h (right):

http://codereview.chromium.org/9353013/diff/38001/ppapi/native_client/src/sha...
ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h:36: // The Usage
Page (upper 16-bits) and Usage Id (lower 16-bits) that encode
On 2012/02/19 22:34:22, Wez wrote:
> nit: Suggest "A code identifying the key by a USB Usage Page & Id.  See ..."

Done.

[commit-bot: I haz the power, 2012-02-21 18:42:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9353013/10002

[commit-bot: I haz the power, 2012-02-21 18:42:52]

Presubmit check for 9353013-10002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.cc \
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h \
  ppapi/native_client/src/shared/ppapi_proxy/plugin_ppb_input_event.h

Presubmit checks took 2.7s to calculate.

[commit-bot: I haz the power, 2012-02-21 18:52:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9353013/48001

[commit-bot: I haz the power, 2012-02-21 18:52:25]

Presubmit check for 9353013-48001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.cc \
  ppapi/native_client/src/shared/ppapi_proxy/input_event_data.h

Presubmit checks took 2.9s to calculate.

[commit-bot: I haz the power, 2012-02-21 19:00:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9353013/35008

[commit-bot: I haz the power, 2012-02-21 20:51:13]

Try job failure for 9353013-35008 (retry) (retry) on linux_rel for step
"ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-02-21 21:16:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/garykac@chromium.org/9353013/35008

[commit-bot: I haz the power, 2012-02-21 22:43:01]

Change committed as 122892