From ea41cce309d22e53535f9bf88c053e39ea209a7a Mon Sep 17 00:00:00 2001 From: Lucas Furukawa Gadani Date: Mon, 16 Oct 2017 17:02:16 +0000 Subject: [PATCH] Reattach inner WebContents when recreating its RenderWidgetHostView. Bug: 772354 Change-Id: Ie82bb502cbfbf989d653d45ba7e73164de448d26 Reviewed-on: https://chromium-review.googlesource.com/713949 Reviewed-by: James MacLean Reviewed-by: Charlie Reis Reviewed-by: Ken Buchanan Commit-Queue: Lucas Gadani Cr-Commit-Position: refs/heads/master@{#508741} (cherry picked from commit aed1fed1e341e7a39071560fbecedced31c65030) TBR=creis@chromium.org,kenrb@chromium.org,wjmaclean@chromium.org NOTRY=true NOPRESUBMIT=true Change-Id: Ib8f5ed55ed080a98b076c3ef9625fb670b6c21df Reviewed-on: https://chromium-review.googlesource.com/721682 Reviewed-by: Lucas Gadani Cr-Commit-Position: refs/branch-heads/3239@{#13} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} --- .../apps/guest_view/web_view_browsertest.cc | 33 ++++++++++++++++++ .../browser/web_contents/web_contents_impl.cc | 34 ++++++++++++------- .../browser/web_contents/web_contents_impl.h | 3 ++ .../buildbot/filters/mus.browser_tests.filter | 3 ++ 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/chrome/browser/apps/guest_view/web_view_browsertest.cc b/chrome/browser/apps/guest_view/web_view_browsertest.cc index 5d798a1916619..cc0bc99fa73d6 100644 --- a/chrome/browser/apps/guest_view/web_view_browsertest.cc +++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc @@ -63,6 +63,7 @@ #include "content/public/common/child_process_host.h" #include "content/public/common/content_features.h" #include "content/public/common/content_switches.h" +#include "content/public/common/result_codes.h" #include "content/public/common/url_constants.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/download_test_observer.h" @@ -3527,6 +3528,38 @@ IN_PROC_BROWSER_TEST_P(WebViewTest, LoadWebviewInsideIframe) { watcher.Wait(); } +// Makes sure that a webview will display correctly after reloading it after a +// crash. +IN_PROC_BROWSER_TEST_P(WebViewTest, ReloadAfterCrash) { + if (!base::FeatureList::IsEnabled(::features::kGuestViewCrossProcessFrames)) + return; + + // Load guest and wait for it to appear. + LoadAppWithGuest("web_view/simple"); + EXPECT_TRUE(GetGuestWebContents()->GetMainFrame()->GetView()); + WaitForChildFrameSurfaceReady(GetGuestWebContents()->GetMainFrame()); + + // Kill guest. + auto* rph = GetGuestWebContents()->GetMainFrame()->GetProcess(); + content::RenderProcessHostWatcher crash_observer( + rph, content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); + EXPECT_TRUE(rph->Shutdown(content::RESULT_CODE_KILLED, false)); + crash_observer.Wait(); + EXPECT_FALSE(GetGuestWebContents()->GetMainFrame()->GetView()); + + // Reload guest and make sure it appears. + content::TestNavigationObserver load_observer(GetGuestWebContents()); + EXPECT_TRUE(ExecuteScript(GetEmbedderWebContents(), + "document.querySelector('webview').reload()")); + load_observer.Wait(); + EXPECT_TRUE(GetGuestWebContents()->GetMainFrame()->GetView()); + + // When the frame passed has a RenderWidgetHostViewChildFrame, + // WaitForChildFrameSurfaceReady will only return when the guest is embedded + // within the root surface. + WaitForChildFrameSurfaceReady(GetGuestWebContents()->GetMainFrame()); +} + IN_PROC_BROWSER_TEST_P(WebViewAccessibilityTest, LoadWebViewAccessibility) { LoadAppWithGuest("web_view/focus_accessibility"); content::WebContents* web_contents = GetFirstAppWindowWebContents(); diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 7297db47cf2d4..f985ea2bceab8 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -1631,12 +1631,7 @@ void WebContentsImpl::AttachToOuterWebContentsFrame( render_manager->CreateOuterDelegateProxy( outer_contents_frame->GetSiteInstance(), outer_contents_frame_impl); - render_manager->SetRWHViewForInnerContents( - render_manager->GetRenderWidgetHostView()); - - static_cast( - render_manager->GetRenderWidgetHostView()) - ->RegisterFrameSinkId(); + ReattachToOuterWebContentsFrame(); if (outer_web_contents_impl->frame_tree_.GetFocusedFrame() == outer_contents_frame_impl->frame_tree_node()) { @@ -1644,12 +1639,6 @@ void WebContentsImpl::AttachToOuterWebContentsFrame( outer_contents_frame->GetSiteInstance()); } - // Set up the the guest's AX tree to point back at the embedder's AX tree. - auto* parent_frame = outer_contents_frame->GetParent(); - GetMainFrame()->set_browser_plugin_embedder_ax_tree_id( - parent_frame->GetAXTreeID()); - GetMainFrame()->UpdateAXTreeData(); - // At this point, we should destroy the TextInputManager which will notify all // the RWHV in this WebContents. The RWHV in this WebContents should use the // TextInputManager owned by the outer WebContents. @@ -1660,6 +1649,24 @@ void WebContentsImpl::AttachToOuterWebContentsFrame( text_input_manager_.reset(nullptr); } +void WebContentsImpl::ReattachToOuterWebContentsFrame() { + DCHECK(node_.outer_web_contents()); + auto* render_manager = GetRenderManager(); + auto* parent_frame = + node_.OuterContentsFrameTreeNode()->current_frame_host()->GetParent(); + render_manager->SetRWHViewForInnerContents( + render_manager->GetRenderWidgetHostView()); + + static_cast( + render_manager->GetRenderWidgetHostView()) + ->RegisterFrameSinkId(); + + // Set up the the guest's AX tree to point back at the embedder's AX tree. + GetMainFrame()->set_browser_plugin_embedder_ax_tree_id( + parent_frame->GetAXTreeID()); + GetMainFrame()->UpdateAXTreeData(); +} + void WebContentsImpl::DidChangeVisibleSecurityState() { if (delegate_) { delegate_->VisibleSecurityStateChanged(this); @@ -5533,6 +5540,9 @@ bool WebContentsImpl::CreateRenderViewForRenderManager( return false; } + if (proxy_routing_id == MSG_ROUTING_NONE && node_.outer_web_contents()) + ReattachToOuterWebContentsFrame(); + SetHistoryOffsetAndLengthForView(render_view_host, controller_.GetLastCommittedEntryIndex(), controller_.GetEntryCount()); diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index ae7f760fbdf64..afd40fc8389cd 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -1318,6 +1318,9 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents, // is that of the main frame. void SetVisibilityForChildViews(bool visible); + // Reattaches this inner WebContents to its outer WebContents. + void ReattachToOuterWebContentsFrame(); + // Data for core operation --------------------------------------------------- // Delegate for notifying our owner about stuff. Not owned by us. diff --git a/testing/buildbot/filters/mus.browser_tests.filter b/testing/buildbot/filters/mus.browser_tests.filter index 46ead00ab9b92..777bc3595ad10 100644 --- a/testing/buildbot/filters/mus.browser_tests.filter +++ b/testing/buildbot/filters/mus.browser_tests.filter @@ -34,3 +34,6 @@ -WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/0 -WebViewTests/WebViewSizeTest.Shim_TestAutosizeRemoveAttributes/1 -WebViewTests/WebViewTest.InterstitialPageFocusedWidget/1 + +# content::WaitForChildFrameSurfaceReady unsupported, https://crbug.com/774269. +-WebViewTests/WebViewTest.ReloadAfterCrash/1