Skip to content

Commit

Permalink
Reattach inner WebContents when recreating its RenderWidgetHostView.
Browse files Browse the repository at this point in the history
Bug: 772354
Change-Id: Ie82bb502cbfbf989d653d45ba7e73164de448d26
Reviewed-on: https://chromium-review.googlesource.com/713949
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508741}
(cherry picked from commit aed1fed)

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 <lfg@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#13}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
  • Loading branch information
lucasgadani committed Oct 16, 2017
1 parent 22dd92d commit ea41cce
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 12 deletions.
33 changes: 33 additions & 0 deletions chrome/browser/apps/guest_view/web_view_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand Down
34 changes: 22 additions & 12 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1631,25 +1631,14 @@ void WebContentsImpl::AttachToOuterWebContentsFrame(
render_manager->CreateOuterDelegateProxy(
outer_contents_frame->GetSiteInstance(), outer_contents_frame_impl);

render_manager->SetRWHViewForInnerContents(
render_manager->GetRenderWidgetHostView());

static_cast<RenderWidgetHostViewChildFrame*>(
render_manager->GetRenderWidgetHostView())
->RegisterFrameSinkId();
ReattachToOuterWebContentsFrame();

if (outer_web_contents_impl->frame_tree_.GetFocusedFrame() ==
outer_contents_frame_impl->frame_tree_node()) {
SetFocusedFrame(frame_tree_.root(),
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.
Expand All @@ -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<RenderWidgetHostViewChildFrame*>(
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);
Expand Down Expand Up @@ -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());
Expand Down
3 changes: 3 additions & 0 deletions content/browser/web_contents/web_contents_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions testing/buildbot/filters/mus.browser_tests.filter
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ea41cce

Please sign in to comment.