aura: Allow fullscreen RWHVAs (i.e. Flash) to be focused.

aura: Allow fullscreen RWHVAs (i.e. Flash) to be focused.

This fixes a regression that caused fullscreen
RenderWidgetHostViewAuras to decline the focus, resulting in
them not receiving key events.

BUG=107804
TEST=manually checked that Space and Escape now work in fullscreen YouTube videos; also added a test

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

[Daniel Erat, 2011-12-22 19:33:19]

http://codereview.chromium.org/9026013/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

http://codereview.chromium.org/9026013/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_widget_host_view_aura.cc:589: return
is_fullscreen_;
I suspect that the desired approach is to leave this at false and override
ShouldActivate() (e.g. views::NativeWidgetAura::can_activate_), but looking at
the call stack when this gets created by fullscreening a Flash object, I'm
unsure that that's an option here:

#0  RenderWidgetHostViewAura::RenderWidgetHostViewAura (this=0xf89f8000,
host=0xfa684100) at
../../content/browser/renderer_host/render_widget_host_view_aura.cc:100
#1  0xf66c6fe4 in chrome::ChromeContentBrowserClient::CreateViewForWidget
(this=0xf807c5e4, widget=0xfa684100) at
../../chrome/browser/chrome_content_browser_client.cc:300
#2  0xf69c7b3c in RenderViewHostDelegateViewHelper::CreateNewFullscreenWidget
(this=0xfb004528, route_id=2, process=0xfc603500) at
../../chrome/browser/tab_contents/render_view_host_delegate_helper.cc:211
#3  0xf6a89f92 in TabContentsViewViews::CreateNewFullscreenWidget
(this=0xfb004480, route_id=2) at
../../chrome/browser/ui/views/tab_contents/tab_contents_view_views.cc:314
#4  0xf4bab184 in RenderViewHost::CreateNewFullscreenWidget (this=0xf817d000,
route_id=2) at ../../content/browser/renderer_host/render_view_host.cc:796
#5  0xf4bba214 in RenderWidgetHelper::OnCreateFullscreenWidgetOnUI
(this=0xfb0f7480, opener_id=1, route_id=2) at
../../content/browser/renderer_host/render_widget_helper.cc:278

Let me know if there's a better approach that I'm missing.

http://codereview.chromium.org/9026013/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

http://codereview.chromium.org/9026013/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:17: class
RenderWidgetHostViewAuraTest : public RenderViewHostTestHarness {
I could also believe that I should be testing this somewhere else instead.

[Ben Goodger (Google), 2011-12-22 22:28:53]

lgtm

http://codereview.chromium.org/9026013/diff/1/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/9026013/diff/1/content/content_tests.gypi#newc...
content/content_tests.gypi:224:
'browser/renderer_host/render_widget_host_view_aura_unittest.cc',
will this get filtered appropriately on non-Aura?

[Daniel Erat, 2011-12-22 23:15:35]

http://codereview.chromium.org/9026013/diff/1/content/content_tests.gypi
File content/content_tests.gypi (right):

http://codereview.chromium.org/9026013/diff/1/content/content_tests.gypi#newc...
content/content_tests.gypi:224:
'browser/renderer_host/render_widget_host_view_aura_unittest.cc',
On 2011/12/22 22:28:54, Ben Goodger (Google) wrote:
> will this get filtered appropriately on non-Aura?

I was wondering about this too.  It looks like the _unittest exclusion is there
for Mac but not Aura, so I've added it.

[Ben Goodger (Google), 2011-12-22 23:23:48]

lgtm