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 16a74e50c48a6881cf625594855eb5fc04402a9e..554195e3aa452589118f8bfc47299c565a2b4b35 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -7505,6 +7505,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 6986fcf0af06686ab0857acd2e306416a4d4580e..a3eb2e5f022e4ccaf04e1b4cd540219efc0ce98d 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -3708,21 +3708,25 @@ KeyboardEventProcessingResult WebContentsImpl::PreHandleKeyboardEvent( const 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) { @@ -3858,7 +3862,7 @@ void WebContentsImpl::EnterFullscreenMode( OPTIONAL_TRACE_EVENT0("content", "WebContentsImpl::EnterFullscreenMode"); DCHECK(CanEnterFullscreenMode(requesting_frame, options)); 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 0135a5340b1d1b81fc47a10abe362f5efdd0f222..488d04a9c40deabab47d6235dc36537f47b5d530 100644 --- a/third_party/blink/renderer/core/fullscreen/fullscreen.cc +++ b/third_party/blink/renderer/core/fullscreen/fullscreen.cc @@ -111,7 +111,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(