Replace Textfield::Keystroke with views::KeyEvent

Replace Textfield::Keystroke with views::KeyEvent.
It looks big, but most of change is simple signature change.

 Note: I need your advice on how to deal with WM_CHAR event on Win. Please see my comment below.

BUG=none
TEST=no new functionality. All tests should still pass.

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

[oshima, 2011-01-04 20:26:34]

http://codereview.chromium.org/6034002/diff/17001/views/event.h
File views/event.h (right):

http://codereview.chromium.org/6034002/diff/17001/views/event.h#newcode358
views/event.h:358: app::KeyboardCode GetKeyCode() const {
I think we should rename this to GetKeyboardCode to be consistent with the type
and for better discover-ability.
If this is okay with you, I'll change it in follow up CL.

http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_win.cc (right):

http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
views/controls/textfield/native_textfield_win.cc:889: type = Event::ET_UNKNOWN;
I need your advice here. Event::EventType does not have a corresponding even
type for WM_CHAR. ET_UNKNOWN works as the typeis not used when message() is
used, but this is hacky/error prone and I prefer to have correct impl.
What would you suggest?

[Ben Goodger (Google), 2011-01-05 17:35:12]

Thanks for doing this cleanup!

http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
File views/controls/textfield/native_textfield_win.cc (right):

http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
views/controls/textfield/native_textfield_win.cc:889: type = Event::ET_UNKNOWN;
According to SDK docs, WM_CHAR is synthesized when TranslateMessage translates a
WM_KEYDOWN message. I would infer from that that you can just use
ET_KEY_PRESSED.

http://codereview.chromium.org/6034002/diff/22001/views/event_gtk.cc
File views/event_gtk.cc (right):

http://codereview.chromium.org/6034002/diff/22001/views/event_gtk.cc#newcode18
views/event_gtk.cc:18:
keyboard_code_(app::WindowsKeyCodeForGdkKeyCode(event->keyval)),
if anything I'd prefer renaming all these to "keycode" ... since we don't say
"keyboard" for other things, e.g. it's a KeyEvent not a KeyboardEvent. I think
you might find it easier to change app::KeyboardCode to app::KeyCode.

[oshima, 2011-01-05 19:27:35]

On 2011/01/05 17:35:12, Ben Goodger wrote:
> Thanks for doing this cleanup!
> 
>
http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
> File views/controls/textfield/native_textfield_win.cc (right):
> 
>
http://codereview.chromium.org/6034002/diff/22001/views/controls/textfield/na...
> views/controls/textfield/native_textfield_win.cc:889: type =
Event::ET_UNKNOWN;
> According to SDK docs, WM_CHAR is synthesized when TranslateMessage translates
a
> WM_KEYDOWN message. I would infer from that that you can just use
> ET_KEY_PRESSED.
Done

> 
> http://codereview.chromium.org/6034002/diff/22001/views/event_gtk.cc
> File views/event_gtk.cc (right):
> 
> http://codereview.chromium.org/6034002/diff/22001/views/event_gtk.cc#newcode18
> views/event_gtk.cc:18:
> keyboard_code_(app::WindowsKeyCodeForGdkKeyCode(event->keyval)),
> if anything I'd prefer renaming all these to "keycode" ... since we don't say
> "keyboard" for other things, e.g. it's a KeyEvent not a KeyboardEvent. I think
> you might find it easier to change app::KeyboardCode to app::KeyCode.

Ok, will do so in follow up CL.

[Ben Goodger (Google), 2011-01-05 20:55:01]

LGTM!

[Finnur, 2011-01-11 15:20:00]

This causes a NOTREACHED in native_textfield_win under certain circumstances
(http://crbug.com/69248)

On 2011/01/05 20:55:01, Ben Goodger wrote:
> LGTM!