WebUIHandler for chrome://keyboard

WebUIHandler for chrome://keyboard

This also moves processing of the KeyEvent JSON structure into
ui/keyboard/ from chrome/browser/extensions/api/input/input.cc.

The JS is setup with a separate implementation of send_key_event.js
(send_key_event_webui.js) that is mapped to the send_key_event.js
resource by the WebUIController.  This allows the WebUI and extension
code to be entirely shared.

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

[bryeung, 2013-04-17 18:11:33]

Here's the dual webui/extension implementation.  If it looks okay, I'll go
hunting for OWNERS.

[sadrul, 2013-04-17 19:04:34]

LGTM

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_ui_handl...
File ui/keyboard/keyboard_ui_handler.h (right):

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_ui_handl...
ui/keyboard/keyboard_ui_handler.h:23: KeyboardUIHandler();
Is having this useful?

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_util.cc
File ui/keyboard/keyboard_util.cc (right):

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_util.cc#...
ui/keyboard/keyboard_util.cc:127: // Views does not have a Meta event flag, so
return an error for now.
s/Views/UI/?

[bryeung, 2013-04-17 19:31:46]

+ben: ui/ OWNERS
+asargent: c/b/extensions OWNERS

Thanks Sadrul!

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_ui_handl...
File ui/keyboard/keyboard_ui_handler.h (right):

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_ui_handl...
ui/keyboard/keyboard_ui_handler.h:23: KeyboardUIHandler();
On 2013/04/17 19:04:35, sadrul wrote:
> Is having this useful?

Good point: might as well pass NULL below.  Removed.

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_util.cc
File ui/keyboard/keyboard_util.cc (right):

https://codereview.chromium.org/14161009/diff/1/ui/keyboard/keyboard_util.cc#...
ui/keyboard/keyboard_util.cc:127: // Views does not have a Meta event flag, so
return an error for now.
On 2013/04/17 19:04:35, sadrul wrote:
> s/Views/UI/?

Wow.  That really dates the original version of this code (back when KeyEvent
was in views::).  Fixed.

[asargent_no_longer_on_chrome, 2013-04-17 20:41:49]

extension parts lgtm

[sadrul, 2013-04-18 02:58:28]

https://codereview.chromium.org/14161009/diff/3002/ui/keyboard/keyboard_ui_ha...
File ui/keyboard/keyboard_ui_handler.cc (right):

https://codereview.chromium.org/14161009/diff/3002/ui/keyboard/keyboard_ui_ha...
ui/keyboard/keyboard_ui_handler.cc:42:
root_window_->AsRootWindowHostDelegate()->OnHostKeyEvent(event.get());
Does it make sense to use
web_ui()->GetWebContents()->GetView()->GetNativeView()->GetRootWindow() here
instead? That way, the KUIHandler/Controller doesn't need to carry the
RootWindow around

[bryeung, 2013-04-18 14:42:36]

https://codereview.chromium.org/14161009/diff/3002/ui/keyboard/keyboard_ui_ha...
File ui/keyboard/keyboard_ui_handler.cc (right):

https://codereview.chromium.org/14161009/diff/3002/ui/keyboard/keyboard_ui_ha...
ui/keyboard/keyboard_ui_handler.cc:42:
root_window_->AsRootWindowHostDelegate()->OnHostKeyEvent(event.get());
On 2013/04/18 02:58:28, sadrul wrote:
> Does it make sense to use
> web_ui()->GetWebContents()->GetView()->GetNativeView()->GetRootWindow() here
> instead? That way, the KUIHandler/Controller doesn't need to carry the
> RootWindow around

Clever!  This is a great simplification.  Done.

[sadrul, 2013-04-18 14:54:19]

Nice. Thanks! SLGTM

[Ben Goodger (Google), 2013-04-18 19:50:38]

lgtm

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_c...
File ui/keyboard/keyboard_ui_controller.h (right):

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_c...
ui/keyboard/keyboard_ui_controller.h:20: // Creates a WebUIController for
chrome://keyboard.
seems unnecessary given the class-level comment.

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
File ui/keyboard/keyboard_ui_handler.h (right):

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:18: // with the WebUI.
Merge this with the class level comment then delete this.

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:20: 
delete nl

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:24: virtual void RegisterMessages() OVERRIDE;
move -> private

[bryeung, 2013-04-18 20:05:07]

Thanks all!

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_c...
File ui/keyboard/keyboard_ui_controller.h (right):

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_c...
ui/keyboard/keyboard_ui_controller.h:20: // Creates a WebUIController for
chrome://keyboard.
On 2013/04/18 19:50:38, Ben Goodger (Google) wrote:
> seems unnecessary given the class-level comment.

Done.

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
File ui/keyboard/keyboard_ui_handler.h (right):

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:18: // with the WebUI.
On 2013/04/18 19:50:38, Ben Goodger (Google) wrote:
> Merge this with the class level comment then delete this.

Done.

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:20: 
On 2013/04/18 19:50:38, Ben Goodger (Google) wrote:
> delete nl

Done.

https://codereview.chromium.org/14161009/diff/10001/ui/keyboard/keyboard_ui_h...
ui/keyboard/keyboard_ui_handler.h:24: virtual void RegisterMessages() OVERRIDE;
On 2013/04/18 19:50:38, Ben Goodger (Google) wrote:
> move -> private

Done.

[commit-bot: I haz the power, 2013-04-18 21:36:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryeung@chromium.org/14161009/20001

[commit-bot: I haz the power, 2013-04-18 21:36:58]

Presubmit check for 14161009-20001 failed and returned exit status 1.

INFO:root:Found 13 file(s).
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Traceback (most recent call last):
  File "/b/commit-queue/verification/presubmit_shim.py", line 33, in <module>
    sys.exit(presubmit_support.Main(argv))
  File "/b/depot_tools/presubmit_support.py", line 1305, in Main
    rietveld_obj)
  File "/b/depot_tools/presubmit_support.py", line 1129, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "/b/depot_tools/presubmit_support.py", line 1046, in ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "<string>", line 865, in CheckChangeOnCommit
  File "<string>", line 700, in _CommonChecks
  File "<string>", line 258, in _CheckNoUNIT_TESTInSourceFiles
  File "/b/depot_tools/presubmit_support.py", line 549, in ChangedContents
    for line in self.GenerateScmDiff().splitlines():
  File "/b/depot_tools/presubmit_support.py", line 619, in GenerateScmDiff
    [self.LocalPath()], self._local_root, False, None)
  File "/b/depot_tools/scm.py", line 913, in GenerateDiff
    full_move, revision))
  File "/b/depot_tools/scm.py", line 837, in _DiffItemInternal
    data = SVN.Capture(command, cwd)
  File "/b/depot_tools/scm.py", line 479, in Capture
    ['svn'] + args, stderr=subprocess2.PIPE, cwd=cwd, **kwargs)
  File "/b/depot_tools/subprocess2.py", line 449, in check_output
    return check_call_out(args, stdout=PIPE, **kwargs)[0]
  File "/b/depot_tools/subprocess2.py", line 412, in check_call_out
    returncode, args, kwargs.get('cwd'), out[0], out[1])
subprocess2.CalledProcessError: Command svn diff --internal-diff
chrome/browser/extensions/api/input/input.cc returned non-zero exit status 1 in
/b/commit-queue/workdir/chromium
svn: invalid option: --internal-diff
Type 'svn help' for usage.

[commit-bot: I haz the power, 2013-04-18 21:43:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryeung@chromium.org/14161009/20001

[commit-bot: I haz the power, 2013-04-18 21:43:38]

Presubmit check for 14161009-20001 failed and returned exit status 1.

INFO:root:Found 13 file(s).
Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Traceback (most recent call last):
  File "/b/commit-queue/verification/presubmit_shim.py", line 33, in <module>
    sys.exit(presubmit_support.Main(argv))
  File "/b/depot_tools/presubmit_support.py", line 1305, in Main
    rietveld_obj)
  File "/b/depot_tools/presubmit_support.py", line 1129, in DoPresubmitChecks
    results += executer.ExecPresubmitScript(presubmit_script, filename)
  File "/b/depot_tools/presubmit_support.py", line 1046, in ExecPresubmitScript
    result = eval(function_name + '(*__args)', context)
  File "<string>", line 1, in <module>
  File "<string>", line 865, in CheckChangeOnCommit
  File "<string>", line 700, in _CommonChecks
  File "<string>", line 258, in _CheckNoUNIT_TESTInSourceFiles
  File "/b/depot_tools/presubmit_support.py", line 549, in ChangedContents
    for line in self.GenerateScmDiff().splitlines():
  File "/b/depot_tools/presubmit_support.py", line 619, in GenerateScmDiff
    [self.LocalPath()], self._local_root, False, None)
  File "/b/depot_tools/scm.py", line 913, in GenerateDiff
    full_move, revision))
  File "/b/depot_tools/scm.py", line 837, in _DiffItemInternal
    data = SVN.Capture(command, cwd)
  File "/b/depot_tools/scm.py", line 479, in Capture
    ['svn'] + args, stderr=subprocess2.PIPE, cwd=cwd, **kwargs)
  File "/b/depot_tools/subprocess2.py", line 449, in check_output
    return check_call_out(args, stdout=PIPE, **kwargs)[0]
  File "/b/depot_tools/subprocess2.py", line 412, in check_call_out
    returncode, args, kwargs.get('cwd'), out[0], out[1])
subprocess2.CalledProcessError: Command svn diff --internal-diff
chrome/browser/extensions/api/input/input.cc returned non-zero exit status 1 in
/b/commit-queue/workdir/chromium
svn: invalid option: --internal-diff
Type 'svn help' for usage.

[commit-bot: I haz the power, 2013-04-19 14:43:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryeung@chromium.org/14161009/20001

[commit-bot: I haz the power, 2013-04-19 14:43:37]

Presubmit check for 14161009-20001 failed and returned exit status 1.

INFO:root:Found 13 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/extensions/PRESUBMIT.py

** Presubmit Warnings **
License must match:
.*? Copyright (\(c\) )?(2013|2012|2011|2010|2009|2008|2007|2006|2006-2008) The
Chromium Authors\. All rights reserved\.\n.*? Use of this source code is
governed by a BSD-style license that can be\n.*? found in the LICENSE file\.(?:
\*/)?\n
Found a bad license header in these files:
  ui/keyboard/resources/send_key_event.js \
  ui/keyboard/resources/send_key_event_webui.js

Presubmit checks took 1.2s to calculate.

[commit-bot: I haz the power, 2013-04-19 14:45:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryeung@chromium.org/14161009/27001

[commit-bot: I haz the power, 2013-04-19 17:28:39]

Change committed as 195218