Handle the "key" field as opposed to keyIdentifier field.

Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove: https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPqAAAJ

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8b270d000b1dde4f6a15aad583ba8dd8fdcb4252
Cr-Commit-Position: refs/heads/master@{#399369}

[dtapuska, 2016-06-06 17:18:39]

Description was changed from

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
==========

to

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
==========

[dtapuska, 2016-06-06 17:18:40]

dtapuska@chromium.org changed reviewers:
+ bokan@chromium.org

[dtapuska, 2016-06-08 02:47:47]

Description was changed from

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
==========

to

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dtapuska, 2016-06-08 18:15:42]

PTAL

[bokan, 2016-06-09 15:51:56]

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html
(right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html:35:
eventSender.keyDown("Enter");
Are all layout tests now using the new codes? Can we delete the duplicate checks
in EventSender::KeyDown?

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2691: if (key ==
"ArrowDown")
Perhaps these kind of "ArrowDown", "Backspace" type constants should be defined
as AtomicString in some global place. As it is now, I think each compilation
unit will have its own copy of these strings so we might get some memory savings
out of that. Not necessary for this CL but food for thought.

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/DEPS (right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/DEPS:12: "+ui/events/keycodes/dom/dom_key.h"
This looks out of place here. Did you confirm with someone on the
architecture/onion soup team that this is fine?

[dtapuska, 2016-06-09 16:02:20]

Description was changed from

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dtapuska, 2016-06-09 16:02:20]

dtapuska@chromium.org changed reviewers:
+ esprehn@chromium.org

[dtapuska, 2016-06-09 16:05:01]

dtapuska@chromium.org changed reviewers:
- esprehn@chromium.org

[dtapuska, 2016-06-09 18:20:33]

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html
(right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html:35:
eventSender.keyDown("Enter");
On 2016/06/09 15:51:56, bokan wrote:
> Are all layout tests now using the new codes? Can we delete the duplicate
checks
> in EventSender::KeyDown?

No we can't. I do have an issue open for that though; 617606

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:2691: if (key ==
"ArrowDown")
On 2016/06/09 15:51:56, bokan wrote:
> Perhaps these kind of "ArrowDown", "Backspace" type constants should be
defined
> as AtomicString in some global place. As it is now, I think each compilation
> unit will have its own copy of these strings so we might get some memory
savings
> out of that. Not necessary for this CL but food for thought.

sgtm; I can create a bug for that.

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/DEPS (right):

https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/DEPS:12: "+ui/events/keycodes/dom/dom_key.h"
On 2016/06/09 15:51:56, bokan wrote:
> This looks out of place here. Did you confirm with someone on the
> architecture/onion soup team that this is fine?

Actually I think I can do it a different way; will do that instead.

[dtapuska, 2016-06-09 18:30:51]

On 2016/06/09 18:20:33, dtapuska wrote:
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html
> (right):
> 
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/fast/dom/fragment-activation-focuses-target.html:35:
> eventSender.keyDown("Enter");
> On 2016/06/09 15:51:56, bokan wrote:
> > Are all layout tests now using the new codes? Can we delete the duplicate
> checks
> > in EventSender::KeyDown?
> 
> No we can't. I do have an issue open for that though; 617606
> 
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/input/EventHandler.cpp (right):
> 
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/input/EventHandler.cpp:2691: if (key ==
> "ArrowDown")
> On 2016/06/09 15:51:56, bokan wrote:
> > Perhaps these kind of "ArrowDown", "Backspace" type constants should be
> defined
> > as AtomicString in some global place. As it is now, I think each compilation
> > unit will have its own copy of these strings so we might get some memory
> savings
> > out of that. Not necessary for this CL but food for thought.
> 
> sgtm; I can create a bug for that.
> 
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/tests/DEPS (right):
> 
>
https://codereview.chromium.org/2045603002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/tests/DEPS:12:
"+ui/events/keycodes/dom/dom_key.h"
> On 2016/06/09 15:51:56, bokan wrote:
> > This looks out of place here. Did you confirm with someone on the
> > architecture/onion soup team that this is fine?
> 
> Actually I think I can do it a different way; will do that instead.

PTAL; I've posted the change with the alternate way.

[bokan, 2016-06-09 18:44:52]

thanks, lgtm

[dtapuska, 2016-06-09 18:52:47]

dtapuska@chromium.org changed reviewers:
+ dtseng@chromium.org

[dtapuska, 2016-06-09 18:54:02]

On 2016/06/09 18:44:52, bokan wrote:
> thanks, lgtm

dtseng@ can you have a quick look at the chromevox stuff?

[dtapuska, 2016-06-10 17:57:31]

On 2016/06/09 18:54:02, dtapuska wrote:
> On 2016/06/09 18:44:52, bokan wrote:
> > thanks, lgtm
> 
> dtseng@ can you have a quick look at the chromevox stuff?

ping dtseng@ are you able to have a look at this or should I choose a different
owner?

[dmazzoni, 2016-06-10 20:38:48]

dmazzoni@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[dmazzoni, 2016-06-10 20:38:49]

lgtm for chromevox

https://codereview.chromium.org/2045603002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/testing/chromevox_e2e_test_base.js
(right):

https://codereview.chromium.org/2045603002/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/testing/chromevox_e2e_test_base.js:101:
var evt = new KeyboardEvent('keydown', {view: window, key: '$0'});
Is view: window necessary here? You didn't use it in other refactorings

[dtapuska, 2016-06-10 20:42:45]

https://codereview.chromium.org/2045603002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/testing/chromevox_e2e_test_base.js
(right):

https://codereview.chromium.org/2045603002/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/testing/chromevox_e2e_test_base.js:101:
var evt = new KeyboardEvent('keydown', {view: window, key: '$0'});
On 2016/06/10 20:38:49, dmazzoni wrote:
> Is view: window necessary here? You didn't use it in other refactorings

no it probably isn't needed. I'll remove it.

[dtapuska, 2016-06-10 20:43:00]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-06-10 20:43:00]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/2045603002/#ps100001
(title: "Remove initialization of the view")

[commit-bot: I haz the power, 2016-06-10 20:43:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2045603002/100001

[commit-bot: I haz the power, 2016-06-11 01:44:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-11 01:44:23]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dtapuska, 2016-06-11 10:49:51]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2016-06-11 10:50:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2045603002/100001

[commit-bot: I haz the power, 2016-06-11 16:06:36]

Description was changed from

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-06-11 16:06:38]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-11 16:10:00]

Description was changed from

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Handle the "key" field as opposed to keyIdentifier field.

keyIdentifier will be deprecated soon so we need to move over to the
standardized identifier.

Approved Intent to Remove:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/fqnFyoDCOaA/s4WIxZPq...

BUG=607349
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8b270d000b1dde4f6a15aad583ba8dd8fdcb4252
Cr-Commit-Position: refs/heads/master@{#399369}
==========

[commit-bot: I haz the power, 2016-06-11 16:10:02]

Patchset 6 (id:??) landed as
https://crrev.com/8b270d000b1dde4f6a15aad583ba8dd8fdcb4252
Cr-Commit-Position: refs/heads/master@{#399369}