Implement KeyboardEvent.repeat attribute

Implement KeyboardEvent.repeat attribute

Implement KeyboardEvent.repeat attribute as per the DOM level 3 specification:
https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#widl-KeyboardEvent-repeat

This attribute is already implemented in IE10 and is currently being worked on
for Firefox.

BUG=263724
TEST=fast/events/constructors/keyboard-event-constructor.html
TEST=fast/events/keyboardevent-repeat.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=158868

[haraken, 2013-09-18 07:52:46]

LGTM. Needs an approval from an API owner.

[tkent, 2013-09-18 22:19:44]

not lgtm.
In this implementation, 'repeat' IDL attribute doesn't represent actual key
repeat state.

[do-not-use, 2013-09-19 06:15:04]

On 2013/09/18 22:19:44, tkent wrote:
> not lgtm.
> In this implementation, 'repeat' IDL attribute doesn't represent actual key
> repeat state.

Why doesn't it? It is mapped to the Platform event's isAutoRepeat. It seemed
like the right thing to do.

[tkent, 2013-09-19 06:35:01]

On 2013/09/19 06:15:04, Christophe Dumez wrote:
> On 2013/09/18 22:19:44, tkent wrote:
> > not lgtm.
> > In this implementation, 'repeat' IDL attribute doesn't represent actual key
> > repeat state.
> 
> Why doesn't it? It is mapped to the Platform event's isAutoRepeat. It seemed
> like the right thing to do.

Sorry, I overlooked it.
Can you add a test for it?  We can add another argument to EventSender::keyDown,
or add a special modifier symbol to EventSender::keyDown.

[do-not-use, 2013-09-20 13:05:17]

On 2013/09/19 06:35:01, tkent wrote:
> On 2013/09/19 06:15:04, Christophe Dumez wrote:
> > On 2013/09/18 22:19:44, tkent wrote:
> > > not lgtm.
> > > In this implementation, 'repeat' IDL attribute doesn't represent actual
key
> > > repeat state.
> > 
> > Why doesn't it? It is mapped to the Platform event's isAutoRepeat. It seemed
> > like the right thing to do.
> 
> Sorry, I overlooked it.
> Can you add a test for it?  We can add another argument to
EventSender::keyDown,
> or add a special modifier symbol to EventSender::keyDown.

EventSender constructs a WebKeyboardEvent which it passes to the WebView.
Eventually, the WebKeyboardEvent is converted into a PlatformKeyboardEvent.

To implement this, I would have to add an isAutoRepeat boolean member to
WebKeyboardEvent, which is public API. Is this OK?
It just seemed to a bit odd to extend a public API only for the purpose of
testing.

[tkent, 2013-09-20 14:11:05]

On 2013/09/20 13:05:17, Christophe Dumez (OOO) wrote:
> To implement this, I would have to add an isAutoRepeat boolean member to
> WebKeyboardEvent, which is public API. Is this OK?
> It just seemed to a bit odd to extend a public API only for the purpose of
> testing.

You don't need to add new member. WebInputEvent::modifiers can have
IsAutoRepeat.

[do-not-use, 2013-09-20 14:14:25]

On 2013/09/20 14:11:05, tkent wrote:
> On 2013/09/20 13:05:17, Christophe Dumez (OOO) wrote:
> > To implement this, I would have to add an isAutoRepeat boolean member to
> > WebKeyboardEvent, which is public API. Is this OK?
> > It just seemed to a bit odd to extend a public API only for the purpose of
> > testing.
> 
> You don't need to add new member. WebInputEvent::modifiers can have
> IsAutoRepeat.

Oh right... Sorry about that, I overlooked the inheritance.

[do-not-use, 2013-09-20 14:30:29]

On 2013/09/20 14:14:25, Christophe Dumez (OOO) wrote:
> On 2013/09/20 14:11:05, tkent wrote:
> > On 2013/09/20 13:05:17, Christophe Dumez (OOO) wrote:
> > > To implement this, I would have to add an isAutoRepeat boolean member to
> > > WebKeyboardEvent, which is public API. Is this OK?
> > > It just seemed to a bit odd to extend a public API only for the purpose of
> > > testing.
> > 
> > You don't need to add new member. WebInputEvent::modifiers can have
> > IsAutoRepeat.
> 
> Oh right... Sorry about that, I overlooked the inheritance.

I added the layout test as suggested.

[tkent, 2013-09-24 22:40:38]

lgtm. I verified this feature was approved to ship in blink-dev.

Let's verify the feature works well in Google Chrome manually later.

[commit-bot: I haz the power, 2013-09-24 23:54:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23444080/12001

[commit-bot: I haz the power, 2013-09-24 23:55:01]

Failed to apply patch for Source/core/dom/KeyboardEvent.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: Source/core/dom/KeyboardEvent.cpp
  |diff --git a/Source/core/dom/KeyboardEvent.cpp
b/Source/core/dom/KeyboardEvent.cpp
  |index
b20d77ab0378abbf7ad1adc66de750301046858e..c7348ad21ea91e73322546075ce20a788cddbb9b
100644
  |--- a/Source/core/dom/KeyboardEvent.cpp
  |+++ b/Source/core/dom/KeyboardEvent.cpp
  --------------------------
  No file to patch.  Skipping patch.
  4 out of 4 hunks ignored

Patch:       Source/core/dom/KeyboardEvent.cpp
Index: Source/core/dom/KeyboardEvent.cpp
diff --git a/Source/core/dom/KeyboardEvent.cpp
b/Source/core/dom/KeyboardEvent.cpp
index
b20d77ab0378abbf7ad1adc66de750301046858e..c7348ad21ea91e73322546075ce20a788cddbb9b
100644
--- a/Source/core/dom/KeyboardEvent.cpp
+++ b/Source/core/dom/KeyboardEvent.cpp
@@ -92,12 +92,14 @@ KeyboardEventInit::KeyboardEventInit()
     , altKey(false)
     , shiftKey(false)
     , metaKey(false)
+    , repeat(false)
 {
 }
 
 KeyboardEvent::KeyboardEvent()
     : m_location(DOM_KEY_LOCATION_STANDARD)
     , m_altGraphKey(false)
+    , m_isAutoRepeat(false)
 {
     ScriptWrappable::init(this);
 }
@@ -109,6 +111,7 @@ KeyboardEvent::KeyboardEvent(const PlatformKeyboardEvent&
key, AbstractView* vie
     , m_keyIdentifier(key.keyIdentifier())
     , m_location(keyLocationCode(key))
     , m_altGraphKey(false)
+    , m_isAutoRepeat(key.isAutoRepeat())
 {
     ScriptWrappable::init(this);
 }
@@ -118,6 +121,7 @@ KeyboardEvent::KeyboardEvent(const AtomicString& eventType,
const KeyboardEventI
     , m_keyIdentifier(initializer.keyIdentifier)
     , m_location(initializer.location)
     , m_altGraphKey(false)
+    , m_isAutoRepeat(initializer.repeat)
 {
     ScriptWrappable::init(this);
 }
@@ -129,6 +133,7 @@ KeyboardEvent::KeyboardEvent(const AtomicString& eventType,
bool canBubble, bool
     , m_keyIdentifier(keyIdentifier)
     , m_location(location)
     , m_altGraphKey(altGraphKey)
+    , m_isAutoRepeat(false)
 {
     ScriptWrappable::init(this);
 }

[commit-bot: I haz the power, 2013-10-03 18:14:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23444080/19001

[commit-bot: I haz the power, 2013-10-04 02:22:08]

Change committed as 158868