Grab inputs on WebUI screen locker.

Modify LockWindow to grab inputs when the WebUI lock screen is displayed.

BUG=104141
TEST=Tested on alex trying to push modifier keys to remove focus from lock screen.


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

[flackr, 2011-11-17 22:59:55]

PTAL, thanks!

[oshima, 2011-11-18 17:22:06]

cool. what was reason input grab wasn't working?

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_login_view.h (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_login_view.h:74: DOMView* dom_view() {
document. can you come up with better method name? dom_view is just a name of
the type.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_screen_locker.cc (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:153: int
grab_failure_count_;
new line after this

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:164:
tab_contents()->GetRenderWidgetHostView()->GetNativeView()->window;
Am I correct that grabbing input doesn't require state/variable in LockWindow?
If so, wouldn't WebUIScreenLocker be better place? (no need to expose dom_view).

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:168:
gtk_grab_add(webui_screen_locker_->dom_view()->native_view());
Have you tried using this dom_view()->native_view() as grabWidget?

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:171: }
nuke {}

[sadrul, 2011-11-18 17:36:16]

drive-by with a nit

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_screen_locker.cc (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:163: GdkWindow* grabWidget
= webui_screen_locker_->dom_view()->dom_contents()->
ew! grab_widget

[flackr, 2011-11-18 21:49:09]

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_login_view.h (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_login_view.h:74: DOMView* dom_view() {
On 2011/11/18 17:22:06, oshima wrote:
> document. can you come up with better method name? dom_view is just a name of
> the type.

Done. Not really sure about the name, the view displaying the DOM is what we're
after.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_screen_locker.cc (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:153: int
grab_failure_count_;
On 2011/11/18 17:22:06, oshima wrote:
> new line after this

Done.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:163: GdkWindow* grabWidget
= webui_screen_locker_->dom_view()->dom_contents()->
On 2011/11/18 17:36:17, sadrul wrote:
> ew! grab_widget

Done. Forgive me!

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:164:
tab_contents()->GetRenderWidgetHostView()->GetNativeView()->window;
On 2011/11/18 17:22:06, oshima wrote:
> Am I correct that grabbing input doesn't require state/variable in LockWindow?
> If so, wouldn't WebUIScreenLocker be better place? (no need to expose
dom_view).

Done.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:168:
gtk_grab_add(webui_screen_locker_->dom_view()->native_view());
On 2011/11/18 17:22:06, oshima wrote:
> Have you tried using this dom_view()->native_view() as grabWidget?
> 

I have, this didn't work.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:171: }
On 2011/11/18 17:22:06, oshima wrote:
> nuke {}

Done. The following if is a single statement, but multi-line, so it does not get
nuked?

[oshima, 2011-11-18 22:44:55]

one more.

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/webui_screen_locker.cc (right):

http://codereview.chromium.org/8573004/diff/7001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/webui_screen_locker.cc:171: }
On 2011/11/18 21:49:09, flackr wrote:
> On 2011/11/18 17:22:06, oshima wrote:
> > nuke {}
> 
> Done. The following if is a single statement, but multi-line, so it does not
get
> nuked?

correct

http://codereview.chromium.org/8573004/diff/12004/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/webui_screen_locker.h (right):

http://codereview.chromium.org/8573004/diff/12004/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/webui_screen_locker.h:97: void
FailedWithUnknownError();
These can be in anonymous namepsace in .cc.
(These are to tell how grab failed in crash report)

[flackr, 2011-11-18 22:57:32]

http://codereview.chromium.org/8573004/diff/12004/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/webui_screen_locker.h (right):

http://codereview.chromium.org/8573004/diff/12004/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/webui_screen_locker.h:97: void
FailedWithUnknownError();
On 2011/11/18 22:44:55, oshima wrote:
> These can be in anonymous namepsace in .cc.
> (These are to tell how grab failed in crash report)

Done.

[oshima, 2011-11-19 01:01:42]

LGTM

[commit-bot: I haz the power, 2011-11-22 15:22:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/8573004/16001

[commit-bot: I haz the power, 2011-11-22 17:03:03]

Change committed as 111169