Add EF_IS_REPEAT flag to KeyEvent to handle repeated accelerators correctly.

Add EF_IS_REPEAT flag to KeyEvent to handle repeated accelerators correctly.

Remove AcceleratorController::Context. Just remember previous accelerator in AcceleratorController.

BUG=379171
TEST=covered by unit test.
R=sadrul@chromium.org, sky@chromium.org, yusukes@chromium.org

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

[oshima, 2014-05-30 01:08:26]

still WIP. sadrul, can you take a quick look and tell me if this is ok with you?

[sadrul, 2014-05-30 01:19:19]

On 2014/05/30 01:08:26, oshima wrote:
> still WIP. sadrul, can you take a quick look and tell me if this is ok with
you?

We currently maintain state like this in ui/events/ for double-clicks (in
event.cc), modifier flags for events coming from floating devices
(XModifierStateWatcher in events_x.cc). Can we do the same for this too (i.e.
keep auto_repeat_detector in events_x.cc), so that *WindowTreeHostX11 doesn't
need to worry about it?

[oshima, 2014-05-30 02:18:57]

On 2014/05/30 01:19:19, sadrul wrote:
> On 2014/05/30 01:08:26, oshima wrote:
> > still WIP. sadrul, can you take a quick look and tell me if this is ok with
> you?
> 
> We currently maintain state like this in ui/events/ for double-clicks (in
> event.cc), modifier flags for events coming from floating devices
> (XModifierStateWatcher in events_x.cc). Can we do the same for this too (i.e.
> keep auto_repeat_detector in events_x.cc), so that *WindowTreeHostX11 doesn't
> need to worry about it?

I see, thank you for the suggestion. I'll update the patch tonight and let you
know when ready!

[oshima, 2014-05-30 15:57:35]

ok it's ready. please review.

yusukes@, could you please take a look at previous_accelerator change in
accelerator_controlller*?

thanks,

[sadrul, 2014-05-30 18:27:25]

https://codereview.chromium.org/308023002/diff/220001/ui/events/event_constan...
File ui/events/event_constants.h (right):

https://codereview.chromium.org/308023002/diff/220001/ui/events/event_constan...
ui/events/event_constants.h:95: EF_IS_REPEAT           = 1 << 12,
Move this down to KeyEventFlags?

[oshima, 2014-05-30 19:50:31]

https://codereview.chromium.org/308023002/diff/220001/ui/events/event_constan...
File ui/events/event_constants.h (right):

https://codereview.chromium.org/308023002/diff/220001/ui/events/event_constan...
ui/events/event_constants.h:95: EF_IS_REPEAT           = 1 << 12,
On 2014/05/30 18:27:26, sadrul wrote:
> Move this down to KeyEventFlags?

Done.

[sadrul, 2014-05-30 20:38:09]

ui/ changes LGTM

https://codereview.chromium.org/308023002/diff/260001/ui/events/event.cc
File ui/events/event.cc (right):

https://codereview.chromium.org/308023002/diff/260001/ui/events/event.cc#newc...
ui/events/event.cc:496: if (!last_key_event_) {
Maybe do a CHECK() here that type is PRESSED (and not, say TRANSLATED_*)

[oshima, 2014-05-30 23:44:22]

sky -> ui/wm/core, ui/base/accelerator owner

https://codereview.chromium.org/308023002/diff/260001/ui/events/event.cc
File ui/events/event.cc (right):

https://codereview.chromium.org/308023002/diff/260001/ui/events/event.cc#newc...
ui/events/event.cc:496: if (!last_key_event_) {
On 2014/05/30 20:38:09, sadrul wrote:
> Maybe do a CHECK() here that type is PRESSED (and not, say TRANSLATED_*)

Done.

[sky, 2014-06-01 19:57:05]

LGTM

[oshima, 2014-06-04 19:33:05]

sadrul, I need more changes in events_win.cc in order for
aura::test::EventGenerator to work correctly on windows.
Can you take another look? If you have a better approach, I'm happy to change
it. pls let me know. thanks.

[sadrul, 2014-06-04 19:55:20]

Could TestKeyEvent instead retain the 'generated' flags (i.e. IS_REPEAT) when it
does set_flags() instead? But this looks reasonable too. sky@ might have better
suggestions.

[oshima, 2014-06-04 19:59:18]

On 2014/06/04 19:55:20, sadrul wrote:
> Could TestKeyEvent instead retain the 'generated' flags (i.e. IS_REPEAT) when
it
> does set_flags() instead? But this looks reasonable too. sky@ might have
better
> suggestions.

IsRepeated() relies on the flags set by the test, so it don't work
unfortunately.
I'm open to suggestions though.

[Yusuke Sato, 2014-06-04 22:18:24]

LGTM (sorry for the delay)

[oshima, 2014-06-04 23:06:48]

I reverted the change in events_win.cc and event_generator.cc, and added
ui::EF_IS_REPEAT as suggested.

I'm going to land this as the rest of the code is same as before.

[oshima, 2014-06-04 23:07:06]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-06-04 23:09:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/308023002/440001

[commit-bot: I haz the power, 2014-06-05 04:03:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/16219)

[commit-bot: I haz the power, 2014-06-05 04:08:10]

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

[commit-bot: I haz the power, 2014-06-05 04:08:11]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[oshima, 2014-06-05 07:40:43]

The CQ bit was checked by oshima@chromium.org

[oshima, 2014-06-05 07:41:00]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2014-06-05 14:38:35]

Committed patchset #7 manually as r275101 (presubmit successful).