Portals: Fix crash when detaching OOPIF from portal.
Prior to this change, any detachment of a proxy would cause the
inner WebContents to get detached from the outer WebContents. After
this change, we only detach the inner WebContents from the outer
WebContents when the main frame's proxy is detached.
Bug: 955392
Change-Id: I41fe61961a26636132ce2f17153b2c08b93af1e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1577636
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654071}
diff --git a/content/browser/frame_host/cross_process_frame_connector.cc b/content/browser/frame_host/cross_process_frame_connector.cc
index ddffe8c..82510f19 100644
--- a/content/browser/frame_host/cross_process_frame_connector.cc
+++ b/content/browser/frame_host/cross_process_frame_connector.cc
@@ -403,7 +403,7 @@
// Show/Hide on all the RenderWidgetHostViews (including this) one.
if (frame_proxy_in_parent_renderer_->frame_tree_node()
->render_manager()
- ->ForInnerDelegate()) {
+ ->IsMainFrameForInnerDelegate()) {
view_->host()->delegate()->OnRenderFrameProxyVisibilityChanged(visibility_);
return;
}
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 56aa573..2314c6d 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -175,14 +175,15 @@
return nullptr;
}
-bool RenderFrameHostManager::ForInnerDelegate() {
- return delegate_->GetOuterDelegateFrameTreeNodeId() !=
- FrameTreeNode::kFrameTreeNodeInvalidId;
+bool RenderFrameHostManager::IsMainFrameForInnerDelegate() {
+ return frame_tree_node_->IsMainFrame() &&
+ delegate_->GetOuterDelegateFrameTreeNodeId() !=
+ FrameTreeNode::kFrameTreeNodeInvalidId;
}
RenderWidgetHostImpl*
RenderFrameHostManager::GetOuterRenderWidgetHostForKeyboardInput() {
- if (!ForInnerDelegate() || !frame_tree_node_->IsMainFrame())
+ if (!IsMainFrameForInnerDelegate())
return nullptr;
FrameTreeNode* outer_contents_frame_tree_node =
@@ -211,6 +212,8 @@
}
RenderFrameProxyHost* RenderFrameHostManager::GetProxyToOuterDelegate() {
+ // Only the main frame should be able to reach the outer WebContents.
+ DCHECK(frame_tree_node_->IsMainFrame());
int outer_contents_frame_tree_node_id =
delegate_->GetOuterDelegateFrameTreeNodeId();
FrameTreeNode* outer_contents_frame_tree_node =
@@ -226,6 +229,9 @@
}
void RenderFrameHostManager::RemoveOuterDelegateFrame() {
+ // Removing the outer delegate frame will destroy the inner WebContents. This
+ // should only be called on the main frame.
+ DCHECK(frame_tree_node_->IsMainFrame());
FrameTreeNode* outer_delegate_frame_tree_node =
FrameTreeNode::GloballyFindByID(
delegate_->GetOuterDelegateFrameTreeNodeId());
@@ -2000,7 +2006,7 @@
void RenderFrameHostManager::CreateProxiesForChildFrame(FrameTreeNode* child) {
RenderFrameProxyHost* outer_delegate_proxy =
- ForInnerDelegate() ? GetProxyToOuterDelegate() : nullptr;
+ IsMainFrameForInnerDelegate() ? GetProxyToOuterDelegate() : nullptr;
for (const auto& pair : proxy_hosts_) {
// Do not create proxies for subframes in the outer delegate's process,
// since the outer delegate does not need to interact with them.
@@ -2061,7 +2067,7 @@
void RenderFrameHostManager::SetRWHViewForInnerContents(
RenderWidgetHostView* child_rwhv) {
- DCHECK(ForInnerDelegate() && frame_tree_node_->IsMainFrame());
+ DCHECK(IsMainFrameForInnerDelegate());
GetProxyToOuterDelegate()->SetChildRWHView(child_rwhv, nullptr);
}
@@ -2728,7 +2734,7 @@
// When sending a PageMessage for an inner WebContents, we don't want to also
// send it to the outer WebContent's frame as well.
RenderFrameProxyHost* outer_delegate_proxy =
- ForInnerDelegate() ? GetProxyToOuterDelegate() : nullptr;
+ IsMainFrameForInnerDelegate() ? GetProxyToOuterDelegate() : nullptr;
for (const auto& pair : proxy_hosts_) {
if (outer_delegate_proxy != pair.second.get()) {
send_msg(pair.second.get(), pair.second->GetRoutingID(), msg,
diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h
index 4b1a001..9aaa8af 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -203,9 +203,9 @@
// there is no current one.
RenderWidgetHostView* GetRenderWidgetHostView() const;
- // Returns whether this manager belongs to a FrameTreeNode that belongs to an
- // inner WebContents.
- bool ForInnerDelegate();
+ // Returns whether this manager is a main frame and belongs to a FrameTreeNode
+ // that belongs to an inner WebContents.
+ bool IsMainFrameForInnerDelegate();
// Returns the RenderWidgetHost of the outer WebContents (if any) that can be
// used to fetch the last keyboard event.
@@ -213,8 +213,9 @@
// remote frames.
RenderWidgetHostImpl* GetOuterRenderWidgetHostForKeyboardInput();
- // Return the FrameTreeNode for the frame in the outer WebContents (if any)
- // that contains the inner WebContents.
+ // If this is a RenderFrameHostManager for a main frame, this method returns
+ // the FrameTreeNode for the frame in the outer WebContents (if any) that
+ // contains the inner WebContents.
FrameTreeNode* GetOuterDelegateNode();
// Return a proxy for this frame in the parent frame's SiteInstance. Returns
@@ -222,13 +223,13 @@
// example, if this frame is same-site with its parent).
RenderFrameProxyHost* GetProxyToParent();
- // Returns the proxy to inner WebContents in the outer WebContents's
- // SiteInstance. Returns nullptr if this WebContents isn't part of inner/outer
- // relationship.
+ // If this is a RenderFrameHostManager for a main frame, returns the proxy to
+ // inner WebContents in the outer WebContents's SiteInstance. Returns nullptr
+ // if this WebContents isn't part of inner/outer relationship.
RenderFrameProxyHost* GetProxyToOuterDelegate();
- // Removes the FrameTreeNode in the outer WebContents that represents this
- // FrameTreeNode.
+ // If this is a RenderFrameHostManager for a main frame, removes the
+ // FrameTreeNode in the outer WebContents that represents this FrameTreeNode.
// TODO(lazyboy): This does not belong to RenderFrameHostManager, move it to
// somehwere else.
void RemoveOuterDelegateFrame();
diff --git a/content/browser/frame_host/render_frame_proxy_host.cc b/content/browser/frame_host/render_frame_proxy_host.cc
index f0ede24b..f93a9b77 100644
--- a/content/browser/frame_host/render_frame_proxy_host.cc
+++ b/content/browser/frame_host/render_frame_proxy_host.cc
@@ -69,8 +69,7 @@
RenderFrameProxyHostID(GetProcess()->GetID(), routing_id_),
this)).second);
CHECK(render_view_host ||
- (frame_tree_node_->render_manager()->ForInnerDelegate() &&
- frame_tree_node_->IsMainFrame()));
+ frame_tree_node_->render_manager()->IsMainFrameForInnerDelegate());
if (render_view_host)
frame_tree_node_->frame_tree()->AddRenderViewHostRef(render_view_host_);
@@ -80,8 +79,7 @@
->current_frame_host()
->GetSiteInstance() == site_instance;
bool is_proxy_to_outer_delegate =
- frame_tree_node_->IsMainFrame() &&
- frame_tree_node_->render_manager()->ForInnerDelegate();
+ frame_tree_node_->render_manager()->IsMainFrameForInnerDelegate();
// If this is a proxy to parent frame or this proxy is for the inner
// WebContents's FrameTreeNode in outer WebContents's SiteInstance, then we
@@ -269,9 +267,7 @@
}
void RenderFrameProxyHost::OnDetach() {
- if (frame_tree_node_->render_manager()->ForInnerDelegate()) {
- // Only main frame proxy can detach for inner WebContents.
- DCHECK(frame_tree_node_->IsMainFrame());
+ if (frame_tree_node_->render_manager()->IsMainFrameForInnerDelegate()) {
frame_tree_node_->render_manager()->RemoveOuterDelegateFrame();
return;
}
diff --git a/content/browser/portal/portal_browsertest.cc b/content/browser/portal/portal_browsertest.cc
index 52f0532..870787b 100644
--- a/content/browser/portal/portal_browsertest.cc
+++ b/content/browser/portal/portal_browsertest.cc
@@ -482,4 +482,58 @@
<< "Note: The portal's FrameSinkId is " << portal_view->GetFrameSinkId();
}
+class PortalOOPIFBrowserTest : public PortalBrowserTest {
+ protected:
+ PortalOOPIFBrowserTest() {}
+
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ IsolateAllSitesForTesting(command_line);
+ }
+};
+
+// Tests that creating and destroying OOPIFs inside the portal works as
+// intended.
+IN_PROC_BROWSER_TEST_F(PortalOOPIFBrowserTest, OOPIFInsidePortal) {
+ EXPECT_TRUE(NavigateToURL(
+ shell(), embedded_test_server()->GetURL("portal.test", "/title1.html")));
+ WebContentsImpl* web_contents_impl =
+ static_cast<WebContentsImpl*>(shell()->web_contents());
+ RenderFrameHostImpl* main_frame = web_contents_impl->GetMainFrame();
+
+ // Create portal and wait for navigation.
+ PortalCreatedObserver portal_created_observer(main_frame);
+ GURL a_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+ EXPECT_TRUE(ExecJs(main_frame,
+ JsReplace("var portal = document.createElement('portal');"
+ "portal.src = $1;"
+ "document.body.appendChild(portal);",
+ a_url)));
+ Portal* portal = portal_created_observer.WaitUntilPortalCreated();
+ WebContentsImpl* portal_contents = portal->GetPortalContents();
+ RenderFrameHostImpl* portal_main_frame = portal_contents->GetMainFrame();
+ TestNavigationObserver portal_navigation_observer(portal_contents);
+ portal_navigation_observer.Wait();
+
+ // Add an out-of-process iframe to the portal.
+ GURL b_url(embedded_test_server()->GetURL("b.com", "/title1.html"));
+ TestNavigationObserver iframe_navigation_observer(portal_contents);
+ EXPECT_TRUE(ExecJs(portal_main_frame,
+ JsReplace("var iframe = document.createElement('iframe');"
+ "iframe.src = $1;"
+ "document.body.appendChild(iframe);",
+ b_url)));
+ iframe_navigation_observer.Wait();
+ EXPECT_EQ(b_url, iframe_navigation_observer.last_navigation_url());
+ RenderFrameHostImpl* portal_iframe =
+ portal_main_frame->child_at(0)->current_frame_host();
+ EXPECT_NE(portal_main_frame->GetSiteInstance(),
+ portal_iframe->GetSiteInstance());
+
+ // Remove the OOPIF from the portal.
+ RenderFrameDeletedObserver deleted_observer(portal_iframe);
+ EXPECT_TRUE(
+ ExecJs(portal_main_frame, "document.querySelector('iframe').remove();"));
+ deleted_observer.WaitUntilDeleted();
+}
+
} // namespace content
