From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 4 Oct 2018 14:57:02 -0700 Subject: fix: also propagate fullscreen state for outer frame When entering fullscreen with Element.requestFullscreen in child frames, the parent frame should also enter fullscreen mode too. Chromium handles this for iframes, but not for webviews as they are essentially main frames instead of child frames. This patch makes webviews propagate the fullscreen state to embedder.It also handles a DCHECK preventing guest webcontents from becoming the focused webContents. Note that we also need to manually update embedder's `api::WebContents::IsFullscreenForTabOrPending` value. diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc index 8bb82ef3c5cbb771416e3f693bfb49da7da98c1d..32ba94ebcb0c4a0a3b8c02e6613b8dfa8c29dde0 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -8834,6 +8834,17 @@ void RenderFrameHostImpl::EnterFullscreen( } } + // Entering fullscreen from webview should also notify its outer frame. + if (frame_tree_node()->render_manager()->IsMainFrameForInnerDelegate()) { + RenderFrameProxyHost* outer_proxy = + frame_tree_node()->render_manager()->GetProxyToOuterDelegate(); + DCHECK(outer_proxy); + if (outer_proxy->is_render_frame_proxy_live()) { + outer_proxy->GetAssociatedRemoteFrame()->WillEnterFullscreen( + options.Clone()); + } + } + // Focus the window if another frame may have delegated the capability. if (had_fullscreen_token && !GetView()->HasFocus()) GetView()->Focus(); diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 8ecd5178bbf35cd9200bf7f68d17f1115fdf7a2b..0a776b63168ba15d2df6e527a20aa21356a57ca6 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -4393,21 +4393,25 @@ KeyboardEventProcessingResult WebContentsImpl::PreHandleKeyboardEvent( const input::NativeWebKeyboardEvent& event) { OPTIONAL_TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("content.verbose"), "WebContentsImpl::PreHandleKeyboardEvent"); - auto* outermost_contents = GetOutermostWebContents(); - // TODO(wjmaclean): Generalize this to forward all key events to the outermost - // delegate's handler. - if (outermost_contents != this && IsFullscreen() && - event.windows_key_code == ui::VKEY_ESCAPE) { - // When an inner WebContents has focus and is fullscreen, redirect - // key events to the outermost WebContents so it can be handled by that - // WebContents' delegate. - if (outermost_contents->PreHandleKeyboardEvent(event) == - KeyboardEventProcessingResult::HANDLED) { + + auto handled = delegate_ ? delegate_->PreHandleKeyboardEvent(this, event) + : KeyboardEventProcessingResult::NOT_HANDLED; + + if (IsFullscreen() && event.windows_key_code == ui::VKEY_ESCAPE) { + if (handled == KeyboardEventProcessingResult::HANDLED) return KeyboardEventProcessingResult::HANDLED; + + // When an inner WebContents has focus and is fullscreen, traverse through + // containing webcontents to any that may handle the escape key. + while (auto* outer_web_contents = GetOuterWebContents()) { + auto result = outer_web_contents->PreHandleKeyboardEvent(event); + if (result == KeyboardEventProcessingResult::HANDLED) { + return KeyboardEventProcessingResult::HANDLED; + } } } - return delegate_ ? delegate_->PreHandleKeyboardEvent(this, event) - : KeyboardEventProcessingResult::NOT_HANDLED; + + return handled; } bool WebContentsImpl::HandleMouseEvent(const blink::WebMouseEvent& event) { @@ -4566,7 +4570,7 @@ void WebContentsImpl::EnterFullscreenMode( OPTIONAL_TRACE_EVENT0("content", "WebContentsImpl::EnterFullscreenMode"); DCHECK(CanEnterFullscreenMode(requesting_frame)); DCHECK(requesting_frame->IsActive()); - DCHECK(ContainsOrIsFocusedWebContents()); + DCHECK(ContainsOrIsFocusedWebContents() || IsGuest()); if (base::FeatureList::IsEnabled( features::kAutomaticFullscreenContentSetting)) { // Ensure the window is made active to take input focus. The user may have diff --git a/third_party/blink/renderer/core/fullscreen/fullscreen.cc b/third_party/blink/renderer/core/fullscreen/fullscreen.cc index 4bfea29fb8bcdf8af69fa686d1474c3530e2d3f9..e5403852d06bfe00fc1ed2ea097245b942d2c056 100644 --- a/third_party/blink/renderer/core/fullscreen/fullscreen.cc +++ b/third_party/blink/renderer/core/fullscreen/fullscreen.cc @@ -105,7 +105,7 @@ void FullscreenElementChanged(Document& document, // is the iframe element for the out-of-process frame that contains the // fullscreen element. Hence, it must match :-webkit-full-screen-ancestor. if (new_request_type & FullscreenRequestType::kForCrossProcessDescendant) { - DCHECK(IsA(new_element)); + // DCHECK(IsA(new_element)); new_element->SetContainsFullScreenElement(true); } new_element->SetContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(