Skip to content

GitLab

T
tangled
Commit 6204657c authored by miu@chromium.org's avatar miu@chromium.org
Browse files
Options

[Aura] Fix regressions breaking embedded flash fullscreen. 

1. Set WindowType to "control" in RWHVAura::InitAsChild().  Previously, the window type was not being set, and this caused a NOTREACHED() crash at ash/wm/stacking_controller.cc:101.

2. Infinite loop freezing the browser when entering fullscreen: In WebView, AttachWebContents() and DetachWebContents() were being indirectly called from FOR_EACH_OBSERVER().  Because they were calling WebContentsObserver::Observe(), the FOR_EACH_OBSERVER loop would never terminate.

3. Clean-up: Eliminated WebView::web_contents_ member since WebView inherits from WebContentsObserver, which already has it.

TEST=Run chrome with and without --embed-flash-fullscreen command-line arg, and test switching tabs, flash fullscreen, keyboard focus, etc.  Tested on Win and Linux Aura builds.

Review URL: https://codereview.chromium.org/120503007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243725 0039d316-1c4b-4281-b951-d872f2087c98
parent ab6a2391
Hide whitespace changes
Inline Side-by-side
Showing with 39 additions and 55 deletions
content/browser/renderer_host/render_widget_host_view_aura.cc
View file @ 6204657c
......@@ -496,6 +496,7 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host)
void RenderWidgetHostViewAura::InitAsChild(
gfx::NativeView parent_view) {
window_->SetType(ui::wm::WINDOW_TYPE_CONTROL);
window_->Init(aura::WINDOW_LAYER_TEXTURED);
window_->SetName("RenderWidgetHostViewAura");
}
......
ui/views/controls/webview/webview.cc
View file @ 6204657c
......@@ -30,7 +30,6 @@ const char WebView::kViewClassName[] = "WebView";
WebView::WebView(content::BrowserContext* browser_context)
: wcv_holder_(new NativeViewHost),
web_contents_(NULL),
embed_fullscreen_widget_mode_enabled_(false),
is_embedding_fullscreen_widget_(false),
browser_context_(browser_context),
......@@ -44,30 +43,25 @@ WebView::~WebView() {
}
content::WebContents* WebView::GetWebContents() {
CreateWebContentsWithSiteInstance(NULL);
return web_contents_;
}
void WebView::CreateWebContentsWithSiteInstance(
content::SiteInstance* site_instance) {
if (!web_contents_) {
wc_owner_.reset(CreateWebContents(browser_context_, site_instance));
web_contents_ = wc_owner_.get();
web_contents_->SetDelegate(this);
AttachWebContents();
if (!web_contents()) {
wc_owner_.reset(CreateWebContents(browser_context_));
wc_owner_->SetDelegate(this);
SetWebContents(wc_owner_.get());
}
return web_contents();
}
void WebView::SetWebContents(content::WebContents* web_contents) {
if (web_contents == web_contents_)
void WebView::SetWebContents(content::WebContents* replacement) {
if (replacement == web_contents())
return;
DetachWebContents();
if (wc_owner_ != web_contents)
if (wc_owner_ != replacement)
wc_owner_.reset();
web_contents_ = web_contents;
WebContentsObserver::Observe(replacement);
// web_contents() now returns |replacement| from here onwards.
if (embed_fullscreen_widget_mode_enabled_) {
is_embedding_fullscreen_widget_ =
web_contents_ && web_contents_->GetFullscreenRenderWidgetHostView();
web_contents() && web_contents()->GetFullscreenRenderWidgetHostView();
} else {
is_embedding_fullscreen_widget_ = false;
}
......@@ -80,7 +74,7 @@ void WebView::SetEmbedFullscreenWidgetMode(bool enable) {
DCHECK(!is_embedding_fullscreen_widget_);
embed_fullscreen_widget_mode_enabled_ = true;
should_be_embedded =
web_contents_ && web_contents_->GetFullscreenRenderWidgetHostView();
web_contents() && web_contents()->GetFullscreenRenderWidgetHostView();
} else if (embed_fullscreen_widget_mode_enabled_ && !enable) {
embed_fullscreen_widget_mode_enabled_ = false;
}
......@@ -135,31 +129,31 @@ bool WebView::SkipDefaultKeyEventProcessing(const ui::KeyEvent& event) {
// We'll first give the page a chance to process the key events. If it does
// not process them, they'll be returned to us and we'll treat them as
// accelerators then.
return web_contents_ && !web_contents_->IsCrashed();
return web_contents() && !web_contents()->IsCrashed();
}
bool WebView::IsFocusable() const {
// We need to be focusable when our contents is not a view hierarchy, as
// clicking on the contents needs to focus us.
return !!web_contents_;
return !!web_contents();
}
void WebView::OnFocus() {
if (!web_contents_)
if (!web_contents())
return;
if (is_embedding_fullscreen_widget_) {
content::RenderWidgetHostView* const current_fs_view =
web_contents_->GetFullscreenRenderWidgetHostView();
web_contents()->GetFullscreenRenderWidgetHostView();
if (current_fs_view)
current_fs_view->Focus();
} else {
web_contents_->GetView()->Focus();
web_contents()->GetView()->Focus();
}
}
void WebView::AboutToRequestFocusFromTabTraversal(bool reverse) {
if (web_contents_)
web_contents_->FocusThroughTabTraversal(reverse);
if (web_contents())
web_contents()->FocusThroughTabTraversal(reverse);
}
void WebView::GetAccessibleState(ui::AccessibleViewState* state) {
......@@ -167,9 +161,9 @@ void WebView::GetAccessibleState(ui::AccessibleViewState* state) {
}
gfx::NativeViewAccessible WebView::GetNativeViewAccessible() {
if (web_contents_) {
if (web_contents()) {
content::RenderWidgetHostView* host_view =
web_contents_->GetRenderWidgetHostView();
web_contents()->GetRenderWidgetHostView();
if (host_view)
return host_view->GetNativeViewAccessible();
}
......@@ -189,7 +183,7 @@ gfx::Size WebView::GetPreferredSize() {
void WebView::WebContentsFocused(content::WebContents* web_contents) {
DCHECK(wc_owner_.get());
// The WebView is only the delegate of WebContentses it creates itself.
OnWebContentsFocused(web_contents_);
OnWebContentsFocused(wc_owner_.get());
}
bool WebView::EmbedsFullscreenWidget() const {
......@@ -231,12 +225,12 @@ void WebView::DidDestroyFullscreenWidget(int routing_id) {
void WebView::AttachWebContents() {
// Prevents attachment if the WebView isn't already in a Widget, or it's
// already attached.
if (!GetWidget() || !web_contents_)
if (!GetWidget() || !web_contents())
return;
const gfx::NativeView view_to_attach = is_embedding_fullscreen_widget_ ?
web_contents_->GetFullscreenRenderWidgetHostView()->GetNativeView() :
web_contents_->GetView()->GetNativeView();
web_contents()->GetFullscreenRenderWidgetHostView()->GetNativeView() :
web_contents()->GetView()->GetNativeView();
if (wcv_holder_->native_view() == view_to_attach)
return;
wcv_holder_->Attach(view_to_attach);
......@@ -248,18 +242,16 @@ void WebView::AttachWebContents() {
if (focus_manager && focus_manager->GetFocusedView() == this)
OnFocus();
WebContentsObserver::Observe(web_contents_);
#if defined(OS_WIN) && defined(USE_AURA)
if (!is_embedding_fullscreen_widget_) {
web_contents_->SetParentNativeViewAccessible(
web_contents()->SetParentNativeViewAccessible(
parent()->GetNativeViewAccessible());
}
#endif
}
void WebView::DetachWebContents() {
if (web_contents_) {
if (web_contents()) {
wcv_holder_->Detach();
#if defined(OS_WIN)
if (!is_embedding_fullscreen_widget_) {
......@@ -271,35 +263,33 @@ void WebView::DetachWebContents() {
//
// Moving this out of here would also mean we wouldn't be potentially
// calling member functions on a half-destroyed WebContents.
ShowWindow(web_contents_->GetView()->GetNativeView(), SW_HIDE);
ShowWindow(web_contents()->GetView()->GetNativeView(), SW_HIDE);
#else
web_contents_->SetParentNativeViewAccessible(NULL);
web_contents()->SetParentNativeViewAccessible(NULL);
#endif
}
#endif
}
WebContentsObserver::Observe(NULL);
}
void WebView::ReattachForFullscreenChange(bool enter_fullscreen) {
DetachWebContents();
is_embedding_fullscreen_widget_ = enter_fullscreen &&
web_contents_ && web_contents_->GetFullscreenRenderWidgetHostView();
web_contents() && web_contents()->GetFullscreenRenderWidgetHostView();
AttachWebContents();
}
content::WebContents* WebView::CreateWebContents(
content::BrowserContext* browser_context,
content::SiteInstance* site_instance) {
content::BrowserContext* browser_context) {
content::WebContents* contents = NULL;
if (ViewsDelegate::views_delegate) {
contents = ViewsDelegate::views_delegate->CreateWebContents(
browser_context, site_instance);
browser_context, NULL);
}
if (!contents) {
content::WebContents::CreateParams create_params(
browser_context, site_instance);
browser_context, NULL);
return content::WebContents::Create(create_params);
}
......
ui/views/controls/webview/webview.h
View file @ 6204657c
......@@ -13,10 +13,6 @@
#include "ui/views/controls/webview/webview_export.h"
#include "ui/views/view.h"
namespace content {
class SiteInstance;
}
namespace views {
class NativeViewHost;
......@@ -34,10 +30,6 @@ class WEBVIEW_EXPORT WebView : public View,
// WebView owns this implicitly created WebContents.
content::WebContents* GetWebContents();
// Creates a WebContents if none is yet assocaited with this WebView, with the
// specified site instance. The WebView owns this WebContents.
void CreateWebContentsWithSiteInstance(content::SiteInstance* site_instance);
// WebView does not assume ownership of WebContents set via this method, only
// those it implicitly creates via GetWebContents() above.
void SetWebContents(content::WebContents* web_contents);
......@@ -48,7 +40,9 @@ class WEBVIEW_EXPORT WebView : public View,
// widget or restore the normal WebContentsView.
void SetEmbedFullscreenWidgetMode(bool mode);
content::WebContents* web_contents() { return web_contents_; }
content::WebContents* web_contents() const {
return content::WebContentsObserver::web_contents();
}
content::BrowserContext* browser_context() { return browser_context_; }
......@@ -125,12 +119,11 @@ class WEBVIEW_EXPORT WebView : public View,
// Create a regular or test web contents (based on whether we're running
// in a unit test or not).
content::WebContents* CreateWebContents(
content::BrowserContext* browser_context,
content::SiteInstance* site_instance);
content::BrowserContext* browser_context);
NativeViewHost* wcv_holder_;
// Non-NULL if |web_contents()| was created and is owned by this WebView.
scoped_ptr<content::WebContents> wc_owner_;
content::WebContents* web_contents_;
// When true, WebView auto-embeds fullscreen widgets as a child view.
bool embed_fullscreen_widget_mode_enabled_;
// Set to true while WebView is embedding a fullscreen widget view as a child
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment