CrOS OOBE: Grab accelerators in native code instead of JS.

CrOS OOBE: Grab accelerators in native code instead of JS.

This way, we'll not miss keyboard events due to the focus being in an embedded iframe,  as used e.g. on the gaia signin page.

BUG=chromium-os:18736
TEST=Accelerators still work, even from within the gaia signin page.

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

[Mattias Nissler (ping if slow), 2011-08-05 19:41:20]

Anton: This is my attempt at making the accelerators work reliable. The problem
is that when the focus is in an iframe, i.e. inside the GAIA frame loaded by the
GAIA auth extension, we'll not see any key events, thus missing the
accelerators. I considered digging down into the iframes and adding handlers
there, but that's problematic due to same-origin restrictions. Also, it's
fragile regarding catching all iframes since they load asynchronously. So I
decided for this admittedly somewhat hacky approach :-/

James: OWNERS review for the oobe.js changes (assuming Anton is OK with the
rest)

[James Hawkins, 2011-08-05 20:19:48]

LGTM with nits.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/webui_login_view.h (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/webui_login_view.h:101: typedef
std::map<views::Accelerator, std::string> AccelMap;
Document the typedef.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/oobe.js (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:301: *
Remove blank line.

[whywhat, 2011-08-08 08:09:03]

LGTM

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/webui_login_view.cc (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/webui_login_view.cc:80: if (web_ui) {
Maybe change to:
if (webui_login_) {
  WebUI* web_ui = webui_login_->tab_contents()->web_ui();
  ...
}
?

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/oobe.js (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:54: if (name == 'accessibility')
{
optional: define accelerator names as constants above

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:302: * @param {string}
accelerator name.
nit: Should be @param {string} name Accelerator name

[Mattias Nissler (ping if slow), 2011-08-08 11:11:47]

Fixed all the nits and will be committing this soonish.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/webui_login_view.cc (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/webui_login_view.cc:80: if (web_ui) {
On 2011/08/08 08:09:08, whywhat wrote:
> Maybe change to:
> if (webui_login_) {
>   WebUI* web_ui = webui_login_->tab_contents()->web_ui();
>   ...
> }
> ?

I put a webui_login_ NULL check with early return in. The web_ui NULL check is
still present, so we don't crash when the web_ui is not yet present.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
File chrome/browser/chromeos/login/webui_login_view.h (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/chromeos/login/w...
chrome/browser/chromeos/login/webui_login_view.h:101: typedef
std::map<views::Accelerator, std::string> AccelMap;
On 2011/08/05 20:19:50, James Hawkins wrote:
> Document the typedef.

Done.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/oobe.js (right):

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:54: if (name == 'accessibility')
{
On 2011/08/08 08:09:08, whywhat wrote:
> optional: define accelerator names as constants above

Done.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:301: *
On 2011/08/05 20:19:50, James Hawkins wrote:
> Remove blank line.

Done.

http://codereview.chromium.org/7550043/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/oobe.js:302: * @param {string}
accelerator name.
On 2011/08/08 08:09:08, whywhat wrote:
> nit: Should be @param {string} name Accelerator name

Done.