diff options
| -rw-r--r-- | content/browser/frame_host/frame_tree.cc | 21 | ||||
| -rw-r--r-- | content/browser/frame_host/frame_tree.h | 11 | ||||
| -rw-r--r-- | content/browser/frame_host/frame_tree_node.cc | 4 | ||||
| -rw-r--r-- | content/browser/frame_host/frame_tree_node.h | 5 | ||||
| -rw-r--r-- | content/browser/renderer_host/render_view_host_impl.cc | 6 | ||||
| -rw-r--r-- | content/browser/site_per_process_browsertest.cc | 62 |
6 files changed, 11 insertions, 98 deletions
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc index 0b3275a..c135da8 100644 --- a/content/browser/frame_host/frame_tree.cc +++ b/content/browser/frame_host/frame_tree.cc @@ -43,15 +43,6 @@ bool FrameTreeNodeForFrameId(int64 frame_id, return true; } -// Iterate over the FrameTree to reset any node affected by the loss of the -// given RenderViewHost's process. -bool ResetNodesForNewProcess(RenderViewHost* render_view_host, - FrameTreeNode* node) { - if (render_view_host == node->current_frame_host()->render_view_host()) - node->ResetForNewProcess(); - return true; -} - } // namespace FrameTree::FrameTree(Navigator* navigator, @@ -162,17 +153,7 @@ void FrameTree::SetFrameUrl(int64 frame_id, const GURL& url) { } void FrameTree::ResetForMainFrameSwap() { - root_->ResetForNewProcess(); -} - -void FrameTree::RenderProcessGone(RenderViewHost* render_view_host) { - // Walk the full tree looking for nodes that may be affected. Once a frame - // crashes, all of its child FrameTreeNodes go away. - // Note that the helper function may call ResetForNewProcess on a node, which - // clears its children before we iterate over them. That's ok, because - // ForEach does not add a node's children to the queue until after visiting - // the node itself. - ForEach(base::Bind(&ResetNodesForNewProcess, render_view_host)); + return root_->ResetForMainFrameSwap(); } RenderFrameHostImpl* FrameTree::GetMainFrame() const { diff --git a/content/browser/frame_host/frame_tree.h b/content/browser/frame_host/frame_tree.h index 4af9215..fb29df2 100644 --- a/content/browser/frame_host/frame_tree.h +++ b/content/browser/frame_host/frame_tree.h @@ -60,9 +60,7 @@ class CONTENT_EXPORT FrameTree { // Executes |on_node| on each node in the frame tree. If |on_node| returns // false, terminates the iteration immediately. Returning false is useful - // if |on_node| is just doing a search over the tree. The iteration proceeds - // top-down and visits a node before adding its children to the queue, making - // it safe to remove children during the callback. + // if |on_node| is just doing a search over the tree. void ForEach(const base::Callback<bool(FrameTreeNode*)>& on_node) const; // After the FrameTree is created, or after SwapMainFrame() has been called, @@ -94,13 +92,6 @@ class CONTENT_EXPORT FrameTree { // TODO(creis): Look into how we can remove the need for this method. void ResetForMainFrameSwap(); - // Update the frame tree after a process exits. Any nodes currently using the - // given |render_view_host| will lose all their children. - // TODO(creis): This should take a RenderProcessHost once RenderFrameHost - // knows its process. Until then, we would just be asking the RenderViewHost - // for its process, so we'll skip that step. - void RenderProcessGone(RenderViewHost* render_view_host); - // Convenience accessor for the main frame's RenderFrameHostImpl. RenderFrameHostImpl* GetMainFrame() const; diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc index db90529..faafe5f 100644 --- a/content/browser/frame_host/frame_tree_node.cc +++ b/content/browser/frame_host/frame_tree_node.cc @@ -72,13 +72,13 @@ void FrameTreeNode::RemoveChild(FrameTreeNode* child) { } } -void FrameTreeNode::ResetForNewProcess() { +void FrameTreeNode::ResetForMainFrameSwap() { frame_id_ = kInvalidFrameId; current_url_ = GURL(); // The children may not have been cleared if a cross-process navigation // commits before the old process cleans everything up. Make sure the child - // nodes get deleted before swapping to a new process. + // nodes get deleted. children_.clear(); } diff --git a/content/browser/frame_host/frame_tree_node.h b/content/browser/frame_host/frame_tree_node.h index 397343a..bd9790c 100644 --- a/content/browser/frame_host/frame_tree_node.h +++ b/content/browser/frame_host/frame_tree_node.h @@ -46,8 +46,9 @@ class CONTENT_EXPORT FrameTreeNode { void AddChild(scoped_ptr<FrameTreeNode> child, int frame_routing_id); void RemoveChild(FrameTreeNode* child); - // Clears process specific-state in this node to prepare for a new process. - void ResetForNewProcess(); + // Clears process specific-state after a main frame process swap. + // TODO(creis): Look into how we can remove the need for this method. + void ResetForMainFrameSwap(); FrameTree* frame_tree() const { return frame_tree_; diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index d0879ab..efbf5ec 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -1431,9 +1431,11 @@ void RenderViewHostImpl::OnRenderProcessGone(int status, int exit_code) { render_view_termination_status_ = static_cast<base::TerminationStatus>(status); - // Reset frame tree state associated with this process. + // Reset frame tree state. + // TODO(creis): Once subframes can be in different processes, we'll need to + // clear just the FrameTreeNodes affected by the crash (and their subtrees). main_frame_id_ = -1; - delegate_->GetFrameTree()->RenderProcessGone(this); + delegate_->GetFrameTree()->ResetForMainFrameSwap(); // Our base class RenderWidgetHost needs to reset some stuff. RendererExited(render_view_termination_status_, exit_code); diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index e536cf6..0bade5ca 100644 --- a/content/browser/site_per_process_browsertest.cc +++ b/content/browser/site_per_process_browsertest.cc @@ -258,68 +258,6 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) { child->current_frame_host()->GetProcess()); } -// Crash a subframe and ensures its children are cleared from the FrameTree. -// See http://crbug.com/338508. -// TODO(creis): Enable this on Android when we can kill the process there. -#if defined(OS_ANDROID) -#define MAYBE_CrashSubframe DISABLED_CrashSubframe -#else -#define MAYBE_CrashSubframe CrashSubframe -#endif -IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, MAYBE_CrashSubframe) { - host_resolver()->AddRule("*", "127.0.0.1"); - ASSERT_TRUE(test_server()->Start()); - GURL main_url(test_server()->GetURL("files/site_per_process_main.html")); - NavigateToURL(shell(), main_url); - - StartFrameAtDataURL(); - - // These must stay in scope with replace_host. - GURL::Replacements replace_host; - std::string foo_com("foo.com"); - - // Load cross-site page into iframe. - GURL cross_site_url(test_server()->GetURL("files/title2.html")); - replace_host.SetHostStr(foo_com); - cross_site_url = cross_site_url.ReplaceComponents(replace_host); - EXPECT_TRUE(NavigateIframeToURL(shell(), cross_site_url, "test")); - - // Check the subframe process. - FrameTreeNode* root = - static_cast<WebContentsImpl*>(shell()->web_contents())-> - GetFrameTree()->root(); - ASSERT_EQ(1U, root->child_count()); - FrameTreeNode* child = root->child_at(0); - EXPECT_NE(FrameTreeNode::kInvalidFrameId, root->frame_id()); - EXPECT_NE(FrameTreeNode::kInvalidFrameId, root->child_at(0)->frame_id()); - - // Crash the subframe process. - RenderProcessHost* root_process = root->current_frame_host()->GetProcess(); - RenderProcessHost* child_process = child->current_frame_host()->GetProcess(); - { - RenderProcessHostWatcher crash_observer( - child_process, - RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); - base::KillProcess(child_process->GetHandle(), 0, false); - crash_observer.Wait(); - } - - // Ensure that the child frame still exists but has been cleared. - EXPECT_EQ(1U, root->child_count()); - EXPECT_EQ(FrameTreeNode::kInvalidFrameId, root->child_at(0)->frame_id()); - - // Now crash the top-level page to clear the child frame. - { - RenderProcessHostWatcher crash_observer( - root_process, - RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); - base::KillProcess(root_process->GetHandle(), 0, false); - crash_observer.Wait(); - } - EXPECT_EQ(0U, root->child_count()); - EXPECT_EQ(FrameTreeNode::kInvalidFrameId, root->frame_id()); -} - // TODO(nasko): Disable this test until out-of-process iframes is ready and the // security checks are back in place. // TODO(creis): Replace SpawnedTestServer with host_resolver to get test to run |