Apply Ash's key rewriter to an aura::MouseEvent so that Modifler+Click is handled correctly

Apply Ash's key rewriter to an aura::MouseEvent so that Modifler+Click is handled correctly.

For example, when Ctrl is remapped to Alt, Ctrl+LeftButton should be handled as a RightButton click.

BUG=133510
TEST=mostly covered by existing unit_tests for key_rewriter. Also checked that crbug.com/126529 does not reproduce with this CL.

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

[Daniel Erat, 2012-07-09 18:18:28]

LGTM

Should KeyRewriter be renamed to something like EventRewriter, InputRewriter, or
InputEventRewriter?  It can happen in a different CL.

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
File chrome/browser/ui/views/ash/key_rewriter.cc (right):

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
chrome/browser/ui/views/ash/key_rewriter.cc:201: return kActionRewrite;
nit: just simplify this to three lines?

... KeyRewriter::RewriteOrFilterLocatedEvent(...) {
  if (event->HasNativeEvent())
    RewriteLocatedEvent(event);
  return ash::KeyRewriterDelegate::ACTION_REWRITE_EVENT;
}

or did you mean to return a different value in the !HasNativeEvent() case?

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
File chrome/browser/ui/views/ash/key_rewriter.h (right):

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
chrome/browser/ui/views/ash/key_rewriter.h:137: // Remaps |original_flags| to
|remapped_flags|, |original_native_modifiers| to
nit: s/,/ and/

[Yusuke Sato, 2012-07-09 20:36:49]

Fixed all.

+sky
Could you review ui/aura/?

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
File chrome/browser/ui/views/ash/key_rewriter.cc (right):

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
chrome/browser/ui/views/ash/key_rewriter.cc:201: return kActionRewrite;
On 2012/07/09 18:18:28, Daniel Erat wrote:
> nit: just simplify this to three lines?
> 
> ... KeyRewriter::RewriteOrFilterLocatedEvent(...) {
>   if (event->HasNativeEvent())
>     RewriteLocatedEvent(event);
>   return ash::KeyRewriterDelegate::ACTION_REWRITE_EVENT;
> }
> 
> or did you mean to return a different value in the !HasNativeEvent() case?

Done.

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
File chrome/browser/ui/views/ash/key_rewriter.h (right):

http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
chrome/browser/ui/views/ash/key_rewriter.h:137: // Remaps |original_flags| to
|remapped_flags|, |original_native_modifiers| to
On 2012/07/09 18:18:28, Daniel Erat wrote:
> nit: s/,/ and/

Done.

[Yusuke Sato, 2012-07-10 16:30:56]

+ben for ui/aura/.

On 2012/07/09 20:36:49, Yusuke Sato wrote:
> Fixed all.
> 
> +sky
> Could you review ui/aura/?
> 
>
http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
> File chrome/browser/ui/views/ash/key_rewriter.cc (right):
> 
>
http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
> chrome/browser/ui/views/ash/key_rewriter.cc:201: return kActionRewrite;
> On 2012/07/09 18:18:28, Daniel Erat wrote:
> > nit: just simplify this to three lines?
> > 
> > ... KeyRewriter::RewriteOrFilterLocatedEvent(...) {
> >   if (event->HasNativeEvent())
> >     RewriteLocatedEvent(event);
> >   return ash::KeyRewriterDelegate::ACTION_REWRITE_EVENT;
> > }
> > 
> > or did you mean to return a different value in the !HasNativeEvent() case?
> 
> Done.
> 
>
http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
> File chrome/browser/ui/views/ash/key_rewriter.h (right):
> 
>
http://codereview.chromium.org/10756003/diff/1/chrome/browser/ui/views/ash/ke...
> chrome/browser/ui/views/ash/key_rewriter.h:137: // Remaps |original_flags| to
> |remapped_flags|, |original_native_modifiers| to
> On 2012/07/09 18:18:28, Daniel Erat wrote:
> > nit: s/,/ and/
> 
> Done.

[Ben Goodger (Google), 2012-07-11 15:40:45]

lgtm

[commit-bot: I haz the power, 2012-07-11 16:18:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusukes@chromium.org/10756003/8001

[commit-bot: I haz the power, 2012-07-11 17:19:22]

Change committed as 146132