Update PP_InputEvent_Modifier enum to match WebInputEvent::Modifiers

Update PP_InputEvent_Modifier enum to match WebInputEvent::Modifiers

This adds the IsLeft/IsRight enum values to PP_InputEvent_Modifier.

BUG=137597
TEST=compile
NOTRY=true

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

[yzshen, 2012-09-10 17:19:05]

LGTM

Brett: Since it is a stable API change, is it required to be presented
on the API review meeting?

On Mon, Sep 10, 2012 at 8:38 AM,  <raymes@chromium.org> wrote:
> Reviewers: yzshen1,
>
> Description:
> Update PP_InputEvent_Modifier enum to match WebInputEvent::Modifiers
>
> This adds the IsLeft/IsRight enum values to PP_InputEvent_Modifier.
>
> BUG=137597
> TEST=compile
>
>
> Please review this at https://chromiumcodereview.appspot.com/10905131/
>
> SVN Base: http://git.chromium.org/chromium/src.git@master
>
> Affected files:
>   M ppapi/api/ppb_input_event.idl
>   M ppapi/c/ppb_input_event.h
>   M webkit/plugins/ppapi/event_conversion.cc
>
>
> Index: ppapi/api/ppb_input_event.idl
> diff --git a/ppapi/api/ppb_input_event.idl b/ppapi/api/ppb_input_event.idl
> index
>
68ffadc38d48385fce50ea50597d6e154f9a95e9..73e279d8d144c53abf6f8a6ec2f15e7a23d8f73a
> 100644
> --- a/ppapi/api/ppb_input_event.idl
> +++ b/ppapi/api/ppb_input_event.idl
> @@ -196,7 +196,9 @@ enum PP_InputEvent_Modifier {
>    PP_INPUTEVENT_MODIFIER_MIDDLEBUTTONDOWN = 1 << 7,
>    PP_INPUTEVENT_MODIFIER_RIGHTBUTTONDOWN  = 1 << 8,
>    PP_INPUTEVENT_MODIFIER_CAPSLOCKKEY      = 1 << 9,
> -  PP_INPUTEVENT_MODIFIER_NUMLOCKKEY       = 1 << 10
> +  PP_INPUTEVENT_MODIFIER_NUMLOCKKEY       = 1 << 10,
> +  PP_INPUTEVENT_MODIFIER_ISLEFT           = 1 << 11,
> +  PP_INPUTEVENT_MODIFIER_ISRIGHT          = 1 << 12
>  };
>
>  /**
> @@ -400,7 +402,7 @@ interface PPB_InputEvent {
>     *
>     * @param[in] resource A <code>PP_Resource</code> corresponding to a
> generic
>     * resource.
> -   *
> +   *
>     * @return <code>PP_TRUE</code> if the given resource is a valid input
> event
>     * resource.
>     */
> @@ -408,12 +410,12 @@ interface PPB_InputEvent {
>
>    /**
>     * GetType() returns the type of input event for the given input event
> -   * resource.
> +   * resource.
>     *
>     * @param[in] resource A <code>PP_Resource</code> corresponding to an
> input
>     * event.
>     *
> -   * @return A <code>PP_InputEvent_Type</code> if its a valid input event
> or
> +   * @return A <code>PP_InputEvent_Type</code> if its a valid input event
> or
>     * <code>PP_INPUTEVENT_TYPE_UNDEFINED</code> if the resource is invalid.
>     */
>    PP_InputEvent_Type GetType([in] PP_Resource event);
> @@ -425,7 +427,7 @@ interface PPB_InputEvent {
>     * events without being sensitive to variable processing time.
>     *
>     * @param[in] resource A <code>PP_Resource</code> corresponding to the
> event.
> -   *
> +   *
>     * @return The return value is in time ticks, which is a monotonically
>     * increasing clock not related to the wall clock time. It will not
> change
>     * if the user changes their clock or daylight savings time starts, so
> can
> @@ -741,7 +743,7 @@ interface PPB_KeyboardInputEvent {
>
>    /**
>     * IsKeyboardInputEvent() determines if a resource is a keyboard event.
> -   *
> +   *
>     * @param[in] resource A <code>PP_Resource</code> corresponding to an
> event.
>     *
>     * @return <code>PP_TRUE</code> if the given resource is a valid input
> event.
> Index: ppapi/c/ppb_input_event.h
> diff --git a/ppapi/c/ppb_input_event.h b/ppapi/c/ppb_input_event.h
> index
>
382ba205de980ba66b5cb279d485744aa620c5a1..6122b586370b07e65f15457e5d52471495e64055
> 100644
> --- a/ppapi/c/ppb_input_event.h
> +++ b/ppapi/c/ppb_input_event.h
> @@ -3,7 +3,7 @@
>   * found in the LICENSE file.
>   */
>
> -/* From ppb_input_event.idl modified Fri Jun 22 15:22:14 2012. */
> +/* From ppb_input_event.idl modified Tue Aug 14 09:41:28 2012. */
>
>  #ifndef PPAPI_C_PPB_INPUT_EVENT_H_
>  #define PPAPI_C_PPB_INPUT_EVENT_H_
> @@ -208,7 +208,9 @@ typedef enum {
>    PP_INPUTEVENT_MODIFIER_MIDDLEBUTTONDOWN = 1 << 7,
>    PP_INPUTEVENT_MODIFIER_RIGHTBUTTONDOWN = 1 << 8,
>    PP_INPUTEVENT_MODIFIER_CAPSLOCKKEY = 1 << 9,
> -  PP_INPUTEVENT_MODIFIER_NUMLOCKKEY = 1 << 10
> +  PP_INPUTEVENT_MODIFIER_NUMLOCKKEY = 1 << 10,
> +  PP_INPUTEVENT_MODIFIER_ISLEFT = 1 << 11,
> +  PP_INPUTEVENT_MODIFIER_ISRIGHT = 1 << 12
>  } PP_InputEvent_Modifier;
>  PP_COMPILE_ASSERT_SIZE_IN_BYTES(PP_InputEvent_Modifier, 4);
>
> Index: webkit/plugins/ppapi/event_conversion.cc
> diff --git a/webkit/plugins/ppapi/event_conversion.cc
> b/webkit/plugins/ppapi/event_conversion.cc
> index
>
e88b25a2804f552226439c8d28c8bbddc57a6b77..c8ac36bffa134649ce6dfe2818a673733cf0c660
> 100644
> --- a/webkit/plugins/ppapi/event_conversion.cc
> +++ b/webkit/plugins/ppapi/event_conversion.cc
> @@ -76,6 +76,12 @@
> COMPILE_ASSERT(static_cast<int>(PP_INPUTEVENT_MODIFIER_CAPSLOCKKEY) ==
>  COMPILE_ASSERT(static_cast<int>(PP_INPUTEVENT_MODIFIER_NUMLOCKKEY) ==
>                 static_cast<int>(WebInputEvent::NumLockOn),
>                 NumLockMatches);
> +COMPILE_ASSERT(static_cast<int>(PP_INPUTEVENT_MODIFIER_ISLEFT) ==
> +               static_cast<int>(WebInputEvent::IsLeft),
> +               LeftMatches);
> +COMPILE_ASSERT(static_cast<int>(PP_INPUTEVENT_MODIFIER_ISRIGHT) ==
> +               static_cast<int>(WebInputEvent::IsRight),
> +               RightMatches);
>
>  PP_InputEvent_Type ConvertEventTypes(WebInputEvent::Type wetype) {
>    switch (wetype) {
>
>



-- 
Best regards,
Yuzhu Shen.

[brettw, 2012-09-10 19:50:47]

I think we can skip review for this, but maybe send a quick note to pepper-dev
noting this was added?

[dmichael (off chromium), 2012-09-11 15:20:01]

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

http://codereview.chromium.org/10905131/diff/1/ppapi/api/ppb_input_event.idl#...
ppapi/api/ppb_input_event.idl:12: M14 = 1.1
Should we consider reving this?

If we leave it alone, old plugins should still work, unless they're doing
something silly like asserting the other bits aren't set, or comparing the
modifiers to an absolute value. I think we can live with that.

New Flash built against this interface might try to use the new modifiers on old
Chrome, and they'll never be set. I'm guessing that's probably okay, but wanted
to make sure you guys had thought about it. The advantage to reving the
interface is you can at least tell which behavior you'll get.

[raymes, 2012-09-11 21:52:01]

Spoke to Brett and for now we will leave these as modifiers and we can consider
changing them to a separate struct later.

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

http://codereview.chromium.org/10905131/diff/1/ppapi/api/ppb_input_event.idl#...
ppapi/api/ppb_input_event.idl:12: M14 = 1.1
Thanks, yeh I did think about this and in practice it's not going to break
anything (as you mentioned). Talked to Brett and he's happy not revving it.

On 2012/09/11 15:20:01, dmichael wrote:
> Should we consider reving this?
> 
> If we leave it alone, old plugins should still work, unless they're doing
> something silly like asserting the other bits aren't set, or comparing the
> modifiers to an absolute value. I think we can live with that.
> 
> New Flash built against this interface might try to use the new modifiers on
old
> Chrome, and they'll never be set. I'm guessing that's probably okay, but
wanted
> to make sure you guys had thought about it. The advantage to reving the
> interface is you can at least tell which behavior you'll get.

[dmichael (off chromium), 2012-09-11 22:16:58]

lgtm

[commit-bot: I haz the power, 2012-09-11 22:21:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/10905131/1

[commit-bot: I haz the power, 2012-09-11 22:21:14]

Presubmit check for 10905131-1 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
TODOs found in stable public PPAPI files:

***************
ppapi/api/ppb_input_event.idl
ppapi/c/ppb_input_event.h
***************

Presubmit checks took 5.0s to calculate.