<webview>: Focusing <webview> should propagate to BrowserPlugin.

<webview>: Focusing <webview> should propagate to BrowserPlugin.


This patch does two things:

1. It gives <webview> a tabIndex if it doesn't already have one so that it can
   participate in tab order and keyboard focus.
2. When <webview> gets keyboard focus then we pass the focus to BrowserPlugin.
   When <webview> loses keyboard focus then BrowserPlugin loses focus.

BUG=231633
Test=WebViewTest.Focus

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

[lazyboy, 2013-04-15 21:46:58]

lgtm

https://chromiumcodereview.appspot.com/14272003/diff/3001/chrome/renderer/res...
File chrome/renderer/resources/extensions/web_view.js (right):

https://chromiumcodereview.appspot.com/14272003/diff/3001/chrome/renderer/res...
chrome/renderer/resources/extensions/web_view.js:69:
this.node_.setAttribute('tabIndex', 0);
This feels too hacky though, and would trigger incorrect ordering of tab focus
iiuc.
I don't know focusing code enough, I think we should file a bug and fix this
properly soon.
Also, instead of setting the value explicitly to 0, would just adding the
attribute to this.node_ work? i.e. <webview tabIndex src="..."/>, that way at
least we can expect not to break tab ordering too much.

[miket_OOO, 2013-04-15 22:00:51]

https://codereview.chromium.org/14272003/diff/3001/chrome/test/data/extension...
File chrome/test/data/extensions/platform_apps/web_view/focus/manifest.json
(right):

https://codereview.chromium.org/14272003/diff/3001/chrome/test/data/extension...
chrome/test/data/extensions/platform_apps/web_view/focus/manifest.json:2:
"name": "Platform App Test: <webview> Focus",
For this new file, would you mind changing to the preferred "Packaged App" name?

[Fady Samuel, 2013-04-15 22:15:28]

Thanks, CQ'ing.

https://codereview.chromium.org/14272003/diff/3001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/14272003/diff/3001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/web_view.js:69:
this.node_.setAttribute('tabIndex', 0);
On 2013/04/15 21:46:58, lazyboy wrote:
> This feels too hacky though, and would trigger incorrect ordering of tab focus
> iiuc.
> I don't know focusing code enough, I think we should file a bug and fix this
> properly soon.
> Also, instead of setting the value explicitly to 0, would just adding the
> attribute to this.node_ work? i.e. <webview tabIndex src="..."/>, that way at
> least we can expect not to break tab ordering too much.

Unfortunately, that didn't work. I've added a bug to the comment that we should
fix this in Blink.

https://codereview.chromium.org/14272003/diff/3001/chrome/test/data/extension...
File chrome/test/data/extensions/platform_apps/web_view/focus/manifest.json
(right):

https://codereview.chromium.org/14272003/diff/3001/chrome/test/data/extension...
chrome/test/data/extensions/platform_apps/web_view/focus/manifest.json:2:
"name": "Platform App Test: <webview> Focus",
On 2013/04/15 22:00:51, miket wrote:
> For this new file, would you mind changing to the preferred "Packaged App"
name?

Done.

[commit-bot: I haz the power, 2013-04-18 00:14:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/14272003/3002

[commit-bot: I haz the power, 2013-04-18 03:34:50]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-04-18 15:25:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/14272003/3002

[commit-bot: I haz the power, 2013-04-18 17:23:09]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-04-19 15:14:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/14272003/32001

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

Presubmit check for 14272003-32001 failed and returned exit status 1.

INFO:root:Found 7 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 ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/extensions/web_view_interactive_browsertest.cc

Presubmit checks took 1.5s to calculate.

[miket_OOO, 2013-04-19 16:56:04]

lgtm

[commit-bot: I haz the power, 2013-04-19 18:00:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/14272003/32001

[commit-bot: I haz the power, 2013-04-19 21:29:05]

Change committed as 195294