diff options
| author | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-18 00:07:41 +0000 |
|---|---|---|
| committer | creis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-18 00:07:41 +0000 |
| commit | 94d0cc1e283b81a8eca337d411289fcef9ef4780 (patch) | |
| tree | 4f7984262bacb0a8ab351e13865a24fd7ce804da /content | |
| parent | d0820564192ab9ffa781592271d3dec073dbe0cd (diff) | |
| download | chromium_src-94d0cc1e283b81a8eca337d411289fcef9ef4780.zip chromium_src-94d0cc1e283b81a8eca337d411289fcef9ef4780.tar.gz chromium_src-94d0cc1e283b81a8eca337d411289fcef9ef4780.tar.bz2 | |
Make RenderFrameHostManager swap RenderFrameHosts, not RenderViewHosts.
We still only have a RFHM for the main frame and not subframes, unless
the --site-per-process flag is passed. To external callers, RFHM is
still effectively swapping RenderViewHosts.
RenderFrameHosts now indirectly keep their RenderViewHosts alive.
Second attempt, after fixing memory leak from http://crrev.com/241151.
R=nasko@chromium.org
TBR=jam@chromium.org
BUG=314791
TEST=No visible behavior change.
Review URL: https://codereview.chromium.org/117693002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241423 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
30 files changed, 924 insertions, 684 deletions
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc index fef82ee..0dff87b 100644 --- a/content/browser/frame_host/frame_tree.cc +++ b/content/browser/frame_host/frame_tree.cc @@ -12,6 +12,8 @@ #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/render_frame_host_factory.h" #include "content/browser/frame_host/render_frame_host_impl.h" +#include "content/browser/renderer_host/render_view_host_factory.h" +#include "content/browser/renderer_host/render_view_host_impl.h" namespace content { @@ -52,7 +54,8 @@ FrameTree::FrameTree(Navigator* navigator, render_view_delegate_(render_view_delegate), render_widget_delegate_(render_widget_delegate), manager_delegate_(manager_delegate), - root_(new FrameTreeNode(navigator, + root_(new FrameTreeNode(this, + navigator, render_frame_delegate, render_view_delegate, render_widget_delegate, @@ -94,7 +97,7 @@ void FrameTree::OnFirstNavigationAfterSwap(int main_frame_id) { root_->set_frame_id(main_frame_id); } -RenderFrameHostImpl* FrameTree::AddFrame(int render_frame_host_id, +RenderFrameHostImpl* FrameTree::AddFrame(int frame_routing_id, int64 parent_frame_id, int64 frame_id, const std::string& frame_name) { @@ -104,10 +107,11 @@ RenderFrameHostImpl* FrameTree::AddFrame(int render_frame_host_id, if (!parent) return NULL; - scoped_ptr<FrameTreeNode> node(CreateNode( - frame_id, frame_name, render_frame_host_id, parent)); - RenderFrameHostImpl* render_frame = node->render_frame_host(); - parent->AddChild(node.Pass()); + scoped_ptr<FrameTreeNode> node(new FrameTreeNode( + this, parent->navigator(), render_frame_delegate_, render_view_delegate_, + render_widget_delegate_, manager_delegate_, frame_id, frame_name)); + RenderFrameHostImpl* render_frame = node->current_frame_host(); + parent->AddChild(node.Pass(), frame_routing_id); return render_frame; } @@ -147,12 +151,12 @@ void FrameTree::SetFrameUrl(int64 frame_id, const GURL& url) { node->set_current_url(url); } -void FrameTree::SwapMainFrame(RenderFrameHostImpl* render_frame_host) { - return root_->ResetForMainFrame(render_frame_host); +void FrameTree::ResetForMainFrameSwap() { + return root_->ResetForMainFrameSwap(); } RenderFrameHostImpl* FrameTree::GetMainFrame() const { - return root_->render_frame_host(); + return root_->current_frame_host(); } void FrameTree::SetFrameRemoveListener( @@ -160,32 +164,78 @@ void FrameTree::SetFrameRemoveListener( on_frame_removed_ = on_frame_removed; } +void FrameTree::ClearFrameRemoveListenerForTesting() { + on_frame_removed_.Reset(); +} + +RenderViewHostImpl* FrameTree::CreateRenderViewHostForMainFrame( + SiteInstance* site_instance, + int routing_id, + int main_frame_routing_id, + bool swapped_out, + bool hidden) { + DCHECK(main_frame_routing_id != MSG_ROUTING_NONE); + RenderViewHostMap::iterator iter = + render_view_host_map_.find(site_instance->GetId()); + CHECK(iter == render_view_host_map_.end()); + RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>( + RenderViewHostFactory::Create(site_instance, + render_view_delegate_, + render_widget_delegate_, + routing_id, + main_frame_routing_id, + swapped_out, + hidden)); + + render_view_host_map_[site_instance->GetId()] = + RenderViewHostRefCount(rvh, 0); + return rvh; +} + +RenderViewHostImpl* FrameTree::GetRenderViewHostForSubFrame( + SiteInstance* site_instance) { + RenderViewHostMap::iterator iter = + render_view_host_map_.find(site_instance->GetId()); + CHECK(iter != render_view_host_map_.end()); + RenderViewHostRefCount rvh_refcount = iter->second; + return rvh_refcount.first; +} + +void FrameTree::RegisterRenderFrameHost( + RenderFrameHostImpl* render_frame_host) { + SiteInstance* site_instance = + render_frame_host->render_view_host()->GetSiteInstance(); + RenderViewHostMap::iterator iter = + render_view_host_map_.find(site_instance->GetId()); + CHECK(iter != render_view_host_map_.end()); + + // Increment the refcount. + CHECK_GE(iter->second.second, 0); + iter->second.second++; +} + +void FrameTree::UnregisterRenderFrameHost( + RenderFrameHostImpl* render_frame_host) { + SiteInstance* site_instance = + render_frame_host->render_view_host()->GetSiteInstance(); + RenderViewHostMap::iterator iter = + render_view_host_map_.find(site_instance->GetId()); + CHECK(iter != render_view_host_map_.end()); + + // Decrement the refcount and shutdown the RenderViewHost if no one else is + // using it. + CHECK_GT(iter->second.second, 0); + iter->second.second--; + if (iter->second.second == 0) { + iter->second.first->Shutdown(); + render_view_host_map_.erase(iter); + } +} + FrameTreeNode* FrameTree::FindByFrameID(int64 frame_id) { FrameTreeNode* node = NULL; ForEach(base::Bind(&FrameTreeNodeForFrameId, frame_id, &node)); return node; } -scoped_ptr<FrameTreeNode> FrameTree::CreateNode( - int64 frame_id, - const std::string& frame_name, - int render_frame_host_id, - FrameTreeNode* parent_node) { - scoped_ptr<FrameTreeNode> frame_tree_node(new FrameTreeNode( - parent_node->navigator(), render_frame_delegate_, render_view_delegate_, - render_widget_delegate_, manager_delegate_, frame_id, frame_name)); - - scoped_ptr<RenderFrameHostImpl> render_frame_host( - RenderFrameHostFactory::Create( - parent_node->render_frame_host()->render_view_host(), - parent_node->render_frame_host()->delegate(), - this, - frame_tree_node.get(), - render_frame_host_id, - false)); - - frame_tree_node->set_render_frame_host(render_frame_host.release(), true); - return frame_tree_node.Pass(); -} - } // namespace content diff --git a/content/browser/frame_host/frame_tree.h b/content/browser/frame_host/frame_tree.h index 3ef2d4c..2ee1cdd 100644 --- a/content/browser/frame_host/frame_tree.h +++ b/content/browser/frame_host/frame_tree.h @@ -53,6 +53,8 @@ class CONTENT_EXPORT FrameTree { RenderFrameHostManager::Delegate* manager_delegate); ~FrameTree(); + FrameTreeNode* root() const { return root_.get(); } + // Returns the FrameTreeNode with the given |frame_tree_node_id|. FrameTreeNode* FindByID(int64 frame_tree_node_id); @@ -74,7 +76,7 @@ class CONTENT_EXPORT FrameTree { // Frame tree manipulation routines. // TODO(creis): These should take in RenderFrameHost routing IDs. - RenderFrameHostImpl* AddFrame(int render_frame_host_id, + RenderFrameHostImpl* AddFrame(int frame_routing_id, int64 parent_frame_tree_node_id, int64 frame_id, const std::string& frame_name); @@ -83,18 +85,12 @@ class CONTENT_EXPORT FrameTree { int64 frame_id); void SetFrameUrl(int64 frame_id, const GURL& url); - // Resets the FrameTree and changes RenderFrameHost for the main frame. + // Clears process specific-state after a main frame process swap. // This destroys most of the frame tree but retains the root node so that // navigation state may be kept on it between process swaps. Used to // support bookkeeping for top-level navigations. - // - // If |main_frame| is NULL, reset tree to initially constructed state. - // - // TODO(ajwong): This function should not be given a |main_frame|. This is - // required currently because the RenderViewHost owns its main frame. When - // that relation is fixed, the FrameTree should be responsible for - // created/destroying the main frame on the swap. - void SwapMainFrame(RenderFrameHostImpl* main_frame); + // TODO(creis): Look into how we can remove the need for this method. + void ResetForMainFrameSwap(); // Convenience accessor for the main frame's RenderFrameHostImpl. RenderFrameHostImpl* GetMainFrame() const; @@ -106,19 +102,37 @@ class CONTENT_EXPORT FrameTree { void SetFrameRemoveListener( const base::Callback<void(RenderViewHostImpl*, int64)>& on_frame_removed); - FrameTreeNode* root() const { return root_.get(); } + void ClearFrameRemoveListenerForTesting(); + + // Creates a RenderViewHost for a new main frame RenderFrameHost in the given + // |site_instance|. The RenderViewHost will have its Shutdown method called + // when all of the RenderFrameHosts using it are deleted. + RenderViewHostImpl* CreateRenderViewHostForMainFrame( + SiteInstance* site_instance, + int routing_id, + int main_frame_routing_id, + bool swapped_out, + bool hidden); + + // Returns the existing RenderViewHost for a new subframe RenderFrameHost. + // There should always be such a RenderViewHost, because the main frame + // RenderFrameHost for each SiteInstance should be created before subframes. + RenderViewHostImpl* GetRenderViewHostForSubFrame(SiteInstance* site_instance); + + // Keeps track of which RenderFrameHosts are using each RenderViewHost. When + // the number drops to zero, we call Shutdown on the RenderViewHost. + void RegisterRenderFrameHost(RenderFrameHostImpl* render_frame_host); + void UnregisterRenderFrameHost(RenderFrameHostImpl* render_frame_host); private: + typedef std::pair<RenderViewHostImpl*, int> RenderViewHostRefCount; + typedef base::hash_map<int, RenderViewHostRefCount> RenderViewHostMap; + // Returns the FrameTreeNode with the given renderer-specific |frame_id|. // For internal use only. // TODO(creis): Replace this with a version that takes in a routing ID. FrameTreeNode* FindByFrameID(int64 frame_id); - scoped_ptr<FrameTreeNode> CreateNode(int64 frame_id, - const std::string& frame_name, - int render_frame_host_id, - FrameTreeNode* parent_node); - // These delegates are installed into all the RenderViewHosts and // RenderFrameHosts that we create. RenderFrameHostDelegate* render_frame_delegate_; @@ -126,6 +140,15 @@ class CONTENT_EXPORT FrameTree { RenderWidgetHostDelegate* render_widget_delegate_; RenderFrameHostManager::Delegate* manager_delegate_; + // Map of SiteInstance ID to a (RenderViewHost, refcount) pair. This allows + // us to look up the RenderViewHost for a given SiteInstance when creating + // RenderFrameHosts, and it allows us to call Shutdown on the RenderViewHost + // and remove it from the map when no more RenderFrameHosts are using it. + // + // Must be declared before |root_| so that it is deleted afterward. Otherwise + // the map will be cleared before we delete the RenderFrameHosts in the tree. + RenderViewHostMap render_view_host_map_; + scoped_ptr<FrameTreeNode> root_; base::Callback<void(RenderViewHostImpl*, int64)> on_frame_removed_; diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc index 37b61b4..cca17fd 100644 --- a/content/browser/frame_host/frame_tree_node.cc +++ b/content/browser/frame_host/frame_tree_node.cc @@ -7,39 +7,53 @@ #include <queue> #include "base/stl_util.h" +#include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/navigator.h" #include "content/browser/frame_host/render_frame_host_impl.h" +#include "content/browser/renderer_host/render_view_host_impl.h" namespace content { const int64 FrameTreeNode::kInvalidFrameId = -1; int64 FrameTreeNode::next_frame_tree_node_id_ = 1; -FrameTreeNode::FrameTreeNode(Navigator* navigator, +FrameTreeNode::FrameTreeNode(FrameTree* frame_tree, + Navigator* navigator, RenderFrameHostDelegate* render_frame_delegate, RenderViewHostDelegate* render_view_delegate, RenderWidgetHostDelegate* render_widget_delegate, RenderFrameHostManager::Delegate* manager_delegate, int64 frame_id, const std::string& name) - : navigator_(navigator), - render_manager_(render_frame_delegate, + : frame_tree_(frame_tree), + navigator_(navigator), + render_manager_(this, + render_frame_delegate, render_view_delegate, render_widget_delegate, manager_delegate), frame_tree_node_id_(next_frame_tree_node_id_++), frame_id_(frame_id), - frame_name_(name), - owns_render_frame_host_(true), - render_frame_host_(NULL) { + frame_name_(name) { } FrameTreeNode::~FrameTreeNode() { - if (owns_render_frame_host_) - delete render_frame_host_; } -void FrameTreeNode::AddChild(scoped_ptr<FrameTreeNode> child) { +bool FrameTreeNode::IsMainFrame() const { + return frame_tree_->root() == this; +} + +void FrameTreeNode::AddChild(scoped_ptr<FrameTreeNode> child, + int frame_routing_id) { + // Initialize the RenderFrameHost for the new node. We always create child + // frames in the same SiteInstance as the current frame, and they can swap to + // a different one if they navigate away. + child->render_manager()->Init( + render_manager_.current_host()->GetSiteInstance()->GetBrowserContext(), + render_manager_.current_host()->GetSiteInstance(), + render_manager_.current_host()->GetRoutingID(), + frame_routing_id); children_.push_back(child.release()); } @@ -55,9 +69,7 @@ void FrameTreeNode::RemoveChild(FrameTreeNode* child) { children_.erase(iter); } -void FrameTreeNode::ResetForMainFrame( - RenderFrameHostImpl* new_render_frame_host) { - owns_render_frame_host_ = false; +void FrameTreeNode::ResetForMainFrameSwap() { frame_id_ = kInvalidFrameId; current_url_ = GURL(); @@ -65,8 +77,6 @@ void FrameTreeNode::ResetForMainFrame( // commits before the old process cleans everything up. Make sure the child // nodes get deleted. children_.clear(); - - render_frame_host_ = new_render_frame_host; } } // namespace content diff --git a/content/browser/frame_host/frame_tree_node.h b/content/browser/frame_host/frame_tree_node.h index 9c22b12..e7b0800 100644 --- a/content/browser/frame_host/frame_tree_node.h +++ b/content/browser/frame_host/frame_tree_node.h @@ -18,6 +18,7 @@ namespace content { +class FrameTree; class Navigator; class RenderFrameHostImpl; @@ -29,7 +30,8 @@ class CONTENT_EXPORT FrameTreeNode { public: static const int64 kInvalidFrameId; - FrameTreeNode(Navigator* navigator, + FrameTreeNode(FrameTree* frame_tree, + Navigator* navigator, RenderFrameHostDelegate* render_frame_delegate, RenderViewHostDelegate* render_view_delegate, RenderWidgetHostDelegate* render_widget_delegate, @@ -39,27 +41,18 @@ class CONTENT_EXPORT FrameTreeNode { ~FrameTreeNode(); - void AddChild(scoped_ptr<FrameTreeNode> child); + bool IsMainFrame() const; + + void AddChild(scoped_ptr<FrameTreeNode> child, int frame_routing_id); void RemoveChild(FrameTreeNode* child); - // TODO(nasko): This method should be removed once RenderFrameHosts are - // created by RenderFrameHostManager. - void set_render_frame_host( - RenderFrameHostImpl* render_frame_host, - bool owns_render_frame_host) { - render_frame_host_ = render_frame_host; - owns_render_frame_host_ = owns_render_frame_host; - } + // 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(); - // Transitional API allowing the RenderFrameHost of a FrameTreeNode - // representing the main frame to be provided by someone else. After - // this is called, the FrameTreeNode no longer owns its RenderFrameHost. - // - // This should only be used for the main frame (aka root) in a frame tree. - // - // TODO(ajwong): Remove this method once the main frame RenderFrameHostImpl is - // no longer owned by the RenderViewHostImpl. - void ResetForMainFrame(RenderFrameHostImpl* new_render_frame_host); + FrameTree* frame_tree() const { + return frame_tree_; + } Navigator* navigator() { return navigator_.get(); @@ -103,23 +96,25 @@ class CONTENT_EXPORT FrameTreeNode { current_url_ = url; } - RenderFrameHostImpl* render_frame_host() const { - return render_frame_host_; + RenderFrameHostImpl* current_frame_host() const { + return render_manager_.current_frame_host(); } private: // The next available browser-global FrameTreeNode ID. static int64 next_frame_tree_node_id_; + // The FrameTree that owns us. + FrameTree* frame_tree_; // not owned. + // The Navigator object responsible for managing navigations at this node // of the frame tree. scoped_refptr<Navigator> navigator_; - // Manages creation and swapping of RenderViewHosts for this frame. This must - // be declared before |children_| so that it gets deleted after them. That's - // currently necessary so that RenderFrameHostImpl's destructor can call - // GetProcess. - // TODO(creis): This will eliminate the need for |render_frame_host_| below. + // Manages creation and swapping of RenderFrameHosts for this frame. This + // must be declared before |children_| so that it gets deleted after them. + // That's currently necessary so that RenderFrameHostImpl's destructor can + // call GetProcess. RenderFrameHostManager render_manager_; // A browser-global identifier for the frame in the page, which stays stable @@ -139,22 +134,6 @@ class CONTENT_EXPORT FrameTreeNode { // The immediate children of this specific frame. ScopedVector<FrameTreeNode> children_; - // When ResetForMainFrame() is called, this is set to false and the - // |render_frame_host_| below is not deleted on destruction. - // - // For the mainframe, the FrameTree does not own the |render_frame_host_|. - // This is a transitional wart because RenderFrameHostManager does not yet - // have the bookkeeping logic to handle creating a pending RenderFrameHost - // along with a pending RenderViewHost. Thus, for the main frame, the - // RenderViewHost currently retains ownership and the FrameTreeNode should - // not delete it on destruction. - bool owns_render_frame_host_; - - // The active RenderFrameHost for this frame. The FrameTreeNode does not - // always own this pointer. See comments above |owns_render_frame_host_|. - // TODO(ajwong): Replace with RenderFrameHostManager. - RenderFrameHostImpl* render_frame_host_; - // Track the current frame's last committed URL, so we can estimate the // process impact of out-of-process iframes. // TODO(creis): Remove this when we can store subframe URLs in the diff --git a/content/browser/frame_host/frame_tree_unittest.cc b/content/browser/frame_host/frame_tree_unittest.cc index 6f1bb33..ede3513 100644 --- a/content/browser/frame_host/frame_tree_unittest.cc +++ b/content/browser/frame_host/frame_tree_unittest.cc @@ -10,6 +10,7 @@ #include "content/browser/frame_host/render_frame_host_factory.h" #include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_view_host_impl.h" +#include "content/browser/web_contents/web_contents_impl.h" #include "content/public/test/mock_render_process_host.h" #include "content/public/test/test_browser_context.h" #include "content/public/test/test_browser_thread_bundle.h" @@ -47,35 +48,6 @@ class FrameTreeTest : public RenderViewHostTestHarness { } }; -// The root node never changes during navigation even though its -// RenderFrameHost does. -// - Swapping main frame doesn't change root node. -// - Swapping back to NULL doesn't crash (easier tear-down for interstitials). -// - Main frame does not own RenderFrameHost. -TEST_F(FrameTreeTest, RootNode) { - FrameTree frame_tree(new NavigatorImpl(NULL, NULL), NULL, NULL, NULL, NULL); - - // Initial state has empty node. - FrameTreeNode* root = frame_tree.root(); - ASSERT_TRUE(root); - EXPECT_FALSE(frame_tree.GetMainFrame()); - - // Swap in main frame. - RenderFrameHostImpl* dummy = reinterpret_cast<RenderFrameHostImpl*>(0x1); - frame_tree.SwapMainFrame(dummy); - EXPECT_EQ(root, frame_tree.root()); - EXPECT_EQ(dummy, frame_tree.GetMainFrame()); - - // Move back to NULL. - frame_tree.SwapMainFrame(NULL); - EXPECT_EQ(root, frame_tree.root()); - EXPECT_FALSE(frame_tree.GetMainFrame()); - - // Move back to an invalid pointer, let the FrameTree go out of scope. Test - // should not crash because the main frame isn't owned. - frame_tree.SwapMainFrame(dummy); -} - // Test that swapping the main frame resets the renderer-assigned frame id. // - On creation, frame id is unassigned. // - After a swap, frame id is unassigned. @@ -89,7 +61,7 @@ TEST_F(FrameTreeTest, FirstNavigationAfterSwap) { EXPECT_FALSE(frame_tree.IsFirstNavigationAfterSwap()); EXPECT_EQ(1, frame_tree.root()->frame_id()); - frame_tree.SwapMainFrame(NULL); + frame_tree.ResetForMainFrameSwap(); EXPECT_TRUE(frame_tree.IsFirstNavigationAfterSwap()); EXPECT_EQ(FrameTreeNode::kInvalidFrameId, frame_tree.root()->frame_id()); @@ -99,49 +71,43 @@ TEST_F(FrameTreeTest, FirstNavigationAfterSwap) { // - Add a series of nodes and verify tree structure. // - Remove a series of nodes and verify tree structure. TEST_F(FrameTreeTest, Shape) { - FrameTree frame_tree(new NavigatorImpl(NULL, NULL), NULL, NULL, NULL, NULL); + // Use the FrameTree of the WebContents so that it has all the delegates it + // needs. We may want to consider a test version of this. + FrameTree* frame_tree = + static_cast<WebContentsImpl*>(web_contents())->GetFrameTree(); std::string no_children_node("no children node"); std::string deep_subtree("node with deep subtree"); - // Ensure the top-level node of the FrameTree is initialized by simulating a - // main frame swap here. - scoped_ptr<RenderFrameHostImpl> render_frame_host = - RenderFrameHostFactory::Create(static_cast<RenderViewHostImpl*>(rvh()), - NULL, - &frame_tree, - frame_tree.root(), - process()->GetNextRoutingID(), - false); - frame_tree.SwapMainFrame(render_frame_host.get()); - frame_tree.OnFirstNavigationAfterSwap(5); + frame_tree->OnFirstNavigationAfterSwap(5); - ASSERT_EQ("5: []", GetTreeState(&frame_tree)); + ASSERT_EQ("5: []", GetTreeState(frame_tree)); // Simulate attaching a series of frames to build the frame tree. - frame_tree.AddFrame(process()->GetNextRoutingID(), 5, 14, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 5, 15, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 5, 16, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 5, 14, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 5, 15, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 5, 16, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 14, 244, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 15, 255, no_children_node); - frame_tree.AddFrame(process()->GetNextRoutingID(), 14, 245, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 14, 244, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 15, 255, + no_children_node); + frame_tree->AddFrame(process()->GetNextRoutingID(), 14, 245, std::string()); ASSERT_EQ("5: [14: [244: [], 245: []], " "15: [255 'no children node': []], " "16: []]", - GetTreeState(&frame_tree)); + GetTreeState(frame_tree)); - frame_tree.AddFrame(process()->GetNextRoutingID(), 16, 264, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 16, 265, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 16, 266, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 16, 267, deep_subtree); - frame_tree.AddFrame(process()->GetNextRoutingID(), 16, 268, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 16, 264, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 16, 265, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 16, 266, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 16, 267, deep_subtree); + frame_tree->AddFrame(process()->GetNextRoutingID(), 16, 268, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 267, 365, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 365, 455, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 455, 555, std::string()); - frame_tree.AddFrame(process()->GetNextRoutingID(), 555, 655, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 267, 365, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 365, 455, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 455, 555, std::string()); + frame_tree->AddFrame(process()->GetNextRoutingID(), 555, 655, std::string()); // Now that's it's fully built, verify the tree structure is as expected. ASSERT_EQ("5: [14: [244: [], 245: []], " @@ -149,31 +115,33 @@ TEST_F(FrameTreeTest, Shape) { "16: [264: [], 265: [], 266: [], " "267 'node with deep subtree': " "[365: [455: [555: [655: []]]]], 268: []]]", - GetTreeState(&frame_tree)); + GetTreeState(frame_tree)); - // Test removing of nodes. - frame_tree.RemoveFrame(NULL, 555, 655); + // Test removing of nodes. Clear the frame removal listener so we can pass a + // NULL RFH here. + frame_tree->ClearFrameRemoveListenerForTesting(); + frame_tree->RemoveFrame(NULL, 555, 655); ASSERT_EQ("5: [14: [244: [], 245: []], " "15: [255 'no children node': []], " "16: [264: [], 265: [], 266: [], " "267 'node with deep subtree': " "[365: [455: [555: []]]], 268: []]]", - GetTreeState(&frame_tree)); + GetTreeState(frame_tree)); - frame_tree.RemoveFrame(NULL, 16, 265); + frame_tree->RemoveFrame(NULL, 16, 265); ASSERT_EQ("5: [14: [244: [], 245: []], " "15: [255 'no children node': []], " "16: [264: [], 266: [], " "267 'node with deep subtree': " "[365: [455: [555: []]]], 268: []]]", - GetTreeState(&frame_tree)); + GetTreeState(frame_tree)); - frame_tree.RemoveFrame(NULL, 5, 15); + frame_tree->RemoveFrame(NULL, 5, 15); ASSERT_EQ("5: [14: [244: [], 245: []], " "16: [264: [], 266: [], " "267 'node with deep subtree': " "[365: [455: [555: []]]], 268: []]]", - GetTreeState(&frame_tree)); + GetTreeState(frame_tree)); } } // namespace diff --git a/content/browser/frame_host/interstitial_page_impl.cc b/content/browser/frame_host/interstitial_page_impl.cc index 7e56e4f..8d33a46 100644 --- a/content/browser/frame_host/interstitial_page_impl.cc +++ b/content/browser/frame_host/interstitial_page_impl.cc @@ -162,7 +162,7 @@ InterstitialPageImpl::InterstitialPageImpl( // TODO(creis): We will also need to pass delegates for the RVHM as we // start to use it. frame_tree_(new InterstitialPageNavigatorImpl(this, controller_), - NULL, NULL, NULL, NULL), + this, this, this, NULL), original_child_id_(web_contents->GetRenderProcessHost()->GetID()), original_rvh_id_(web_contents->GetRenderViewHost()->GetRoutingID()), should_revert_web_contents_title_(false), @@ -284,15 +284,15 @@ void InterstitialPageImpl::Hide() { controller_->delegate()->GetRenderViewHost()->GetView())->Focus(); } - // Shutdown the RVH asynchronously, as we may have been called from a RVH - // delegate method, and we can't delete the RVH out from under itself. + // Delete this and call Shutdown on the RVH asynchronously, as we may have + // been called from a RVH delegate method, and we can't delete the RVH out + // from under itself. base::MessageLoop::current()->PostNonNestableTask( FROM_HERE, base::Bind(&InterstitialPageImpl::Shutdown, - weak_ptr_factory_.GetWeakPtr(), - render_view_host_)); + weak_ptr_factory_.GetWeakPtr())); render_view_host_ = NULL; - frame_tree_.SwapMainFrame(NULL); + frame_tree_.ResetForMainFrameSwap(); controller_->delegate()->DetachInterstitialPage(); // Let's revert to the original title if necessary. NavigationEntry* entry = controller_->GetVisibleEntry(); @@ -487,7 +487,8 @@ WebPreferences InterstitialPageImpl::GetWebkitPrefs() { void InterstitialPageImpl::RenderWidgetDeleted( RenderWidgetHostImpl* render_widget_host) { - delete this; + // TODO(creis): Remove this method once we verify the shutdown path is sane. + CHECK(!web_contents_); } bool InterstitialPageImpl::PreHandleKeyboardEvent( @@ -532,14 +533,10 @@ RenderViewHost* InterstitialPageImpl::CreateRenderViewHost() { session_storage_namespace_ = new SessionStorageNamespaceImpl(dom_storage_context); - return RenderViewHostFactory::Create(site_instance.get(), - this, - this, - this, - MSG_ROUTING_NONE, - MSG_ROUTING_NONE, - false, - false); + // Use the RenderViewHost from our FrameTree. + frame_tree_.root()->render_manager()->Init( + browser_context, site_instance.get(), MSG_ROUTING_NONE, MSG_ROUTING_NONE); + return frame_tree_.root()->current_frame_host()->render_view_host(); } WebContentsView* InterstitialPageImpl::CreateWebContentsView() { @@ -559,7 +556,7 @@ WebContentsView* InterstitialPageImpl::CreateWebContentsView() { MSG_ROUTING_NONE, max_page_id); controller_->delegate()->RenderFrameForInterstitialPageCreated( - frame_tree_.root()->render_frame_host()); + frame_tree_.root()->current_frame_host()); view->SetSize(web_contents_view->GetContainerSize()); // Don't show the interstitial until we have navigated to it. view->Hide(); @@ -757,9 +754,8 @@ void InterstitialPageImpl::Disable() { enabled_ = false; } -void InterstitialPageImpl::Shutdown(RenderViewHostImpl* render_view_host) { - render_view_host->Shutdown(); - // We are deleted now. +void InterstitialPageImpl::Shutdown() { + delete this; } void InterstitialPageImpl::OnNavigatingAwayOrTabClosing() { diff --git a/content/browser/frame_host/interstitial_page_impl.h b/content/browser/frame_host/interstitial_page_impl.h index 28d2a0f..a6e95a1 100644 --- a/content/browser/frame_host/interstitial_page_impl.h +++ b/content/browser/frame_host/interstitial_page_impl.h @@ -181,8 +181,8 @@ class CONTENT_EXPORT InterstitialPageImpl // - any command sent by the RenderViewHost will be ignored. void Disable(); - // Shutdown the RVH. We will be deleted by the time this method returns. - void Shutdown(RenderViewHostImpl* render_view_host); + // Delete ourselves, causing Shutdown on the RVH to be called. + void Shutdown(); void OnNavigatingAwayOrTabClosing(); @@ -230,6 +230,8 @@ class CONTENT_EXPORT InterstitialPageImpl // The RenderViewHost displaying the interstitial contents. This is valid // until Hide is called, at which point it will be set to NULL, signifying // that shutdown has started. + // TODO(creis): This is now owned by the FrameTree. We should route things + // through the tree's root RenderFrameHost instead. RenderViewHostImpl* render_view_host_; // The frame tree structure of the current page. diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 53b1b5a..135bfd6 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -54,6 +54,7 @@ RenderFrameHostImpl::RenderFrameHostImpl( frame_tree_node_(frame_tree_node), routing_id_(routing_id), is_swapped_out_(is_swapped_out) { + frame_tree_->RegisterRenderFrameHost(this); GetProcess()->AddRoute(routing_id_, this); g_routing_id_frame_map.Get().insert(std::make_pair( RenderFrameHostID(GetProcess()->GetID(), routing_id_), @@ -66,6 +67,10 @@ RenderFrameHostImpl::~RenderFrameHostImpl() { RenderFrameHostID(GetProcess()->GetID(), routing_id_)); if (delegate_) delegate_->RenderFrameDeleted(this); + + // Notify the FrameTree that this RFH is going away, allowing it to shut down + // the corresponding RenderViewHost if it is no longer needed. + frame_tree_->UnregisterRenderFrameHost(this); } RenderProcessHost* RenderFrameHostImpl::GetProcess() { diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 8225aa33..e12d07a 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -76,8 +76,13 @@ class CONTENT_EXPORT RenderFrameHostImpl : public RenderFrameHost { bool is_swapped_out() { return is_swapped_out_; } - // TODO(nasko): This should be removed and replaced by RenderProcessHost. - RenderViewHostImpl* render_view_host_; // Not owned. + // For now, RenderFrameHosts indirectly keep RenderViewHosts alive via a + // refcount that calls Shutdown when it reaches zero. This allows each + // RenderFrameHostManager to just care about RenderFrameHosts, while ensuring + // we have a RenderViewHost for each RenderFrameHost. + // TODO(creis): RenderViewHost will eventually go away and be replaced with + // some form of page context. + RenderViewHostImpl* render_view_host_; RenderFrameHostDelegate* delegate_; diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc index 8b7448d..b5e1d48 100644 --- a/content/browser/frame_host/render_frame_host_manager.cc +++ b/content/browser/frame_host/render_frame_host_manager.cc @@ -14,6 +14,8 @@ #include "content/browser/frame_host/interstitial_page_impl.h" #include "content/browser/frame_host/navigation_controller_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h" +#include "content/browser/frame_host/render_frame_host_factory.h" +#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_process_host_impl.h" #include "content/browser/renderer_host/render_view_host_factory.h" #include "content/browser/renderer_host/render_view_host_impl.h" @@ -57,53 +59,55 @@ RenderFrameHostManager::PendingNavigationParams::PendingNavigationParams( RenderFrameHostManager::PendingNavigationParams::~PendingNavigationParams() {} RenderFrameHostManager::RenderFrameHostManager( + FrameTreeNode* frame_tree_node, RenderFrameHostDelegate* render_frame_delegate, RenderViewHostDelegate* render_view_delegate, RenderWidgetHostDelegate* render_widget_delegate, Delegate* delegate) - : delegate_(delegate), + : frame_tree_node_(frame_tree_node), + delegate_(delegate), cross_navigation_pending_(false), render_frame_delegate_(render_frame_delegate), render_view_delegate_(render_view_delegate), render_widget_delegate_(render_widget_delegate), - render_view_host_(NULL), - pending_render_view_host_(NULL), + render_frame_host_(NULL), + pending_render_frame_host_(NULL), interstitial_page_(NULL) { } RenderFrameHostManager::~RenderFrameHostManager() { - if (pending_render_view_host_) + if (pending_render_frame_host_) CancelPending(); - // We should always have a main RenderViewHost except in some tests. - RenderViewHostImpl* render_view_host = render_view_host_; - render_view_host_ = NULL; - if (render_view_host) - render_view_host->Shutdown(); + // We should always have a current RenderFrameHost except in some tests. + // TODO(creis): Now that we aren't using Shutdown, make render_frame_host_ and + // RenderFrameHostMap use scoped_ptrs. + RenderFrameHostImpl* render_frame_host = render_frame_host_; + render_frame_host_ = NULL; + if (render_frame_host) + delete render_frame_host; - // Shut down any swapped out RenderViewHosts. - for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + // Delete any swapped out RenderFrameHosts. + for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin(); iter != swapped_out_hosts_.end(); ++iter) { - iter->second->Shutdown(); + delete iter->second; } } void RenderFrameHostManager::Init(BrowserContext* browser_context, SiteInstance* site_instance, - int routing_id, - int main_frame_routing_id) { - // Create a RenderViewHost, once we have an instance. It is important to - // immediately give this SiteInstance to a RenderViewHost so that it is - // ref counted. + int view_routing_id, + int frame_routing_id) { + // Create a RenderViewHost and RenderFrameHost, once we have an instance. It + // is important to immediately give this SiteInstance to a RenderViewHost so + // that the SiteInstance is ref counted. if (!site_instance) site_instance = SiteInstance::Create(browser_context); - render_view_host_ = static_cast<RenderViewHostImpl*>( - RenderViewHostFactory::Create( - site_instance, render_view_delegate_, render_frame_delegate_, - render_widget_delegate_, routing_id, main_frame_routing_id, false, - delegate_->IsHidden())); - render_view_host_->AttachToFrameTree(); + + // TODO(creis): Make render_frame_host_ a scoped_ptr. + render_frame_host_ = CreateRenderFrameHost(site_instance, view_routing_id, + frame_routing_id, false, false); // Keep track of renderer processes as they start to shut down or are // crashed/killed. @@ -114,19 +118,23 @@ void RenderFrameHostManager::Init(BrowserContext* browser_context, } RenderViewHostImpl* RenderFrameHostManager::current_host() const { - return render_view_host_; + if (!render_frame_host_) + return NULL; + return render_frame_host_->render_view_host(); } RenderViewHostImpl* RenderFrameHostManager::pending_render_view_host() const { - return pending_render_view_host_; + if (!pending_render_frame_host_) + return NULL; + return pending_render_frame_host_->render_view_host(); } RenderWidgetHostView* RenderFrameHostManager::GetRenderWidgetHostView() const { if (interstitial_page_) return interstitial_page_->GetView(); - if (!render_view_host_) + if (!render_frame_host_) return NULL; - return render_view_host_->GetView(); + return render_frame_host_->render_view_host()->GetView(); } void RenderFrameHostManager::SetPendingWebUI(const NavigationEntryImpl& entry) { @@ -149,103 +157,111 @@ void RenderFrameHostManager::SetPendingWebUI(const NavigationEntryImpl& entry) { RenderViewHostImpl* RenderFrameHostManager::Navigate( const NavigationEntryImpl& entry) { TRACE_EVENT0("browser", "RenderFrameHostManager:Navigate"); - // Create a pending RenderViewHost. It will give us the one we should use - RenderViewHostImpl* dest_render_view_host = - static_cast<RenderViewHostImpl*>(UpdateRendererStateForNavigate(entry)); - if (!dest_render_view_host) - return NULL; // We weren't able to create a pending render view host. - - // If the current render_view_host_ isn't live, we should create it so - // that we don't show a sad tab while the dest_render_view_host fetches + // Create a pending RenderFrameHost to use for the navigation. + RenderFrameHostImpl* dest_render_frame_host = + UpdateRendererStateForNavigate(entry); + if (!dest_render_frame_host) + return NULL; // We weren't able to create a pending render frame host. + + // If the current render_frame_host_ isn't live, we should create it so + // that we don't show a sad tab while the dest_render_frame_host fetches // its first page. (Bug 1145340) - if (dest_render_view_host != render_view_host_ && - !render_view_host_->IsRenderViewLive()) { + if (dest_render_frame_host != render_frame_host_ && + !render_frame_host_->render_view_host()->IsRenderViewLive()) { // Note: we don't call InitRenderView here because we are navigating away // soon anyway, and we don't have the NavigationEntry for this host. - delegate_->CreateRenderViewForRenderManager(render_view_host_, - MSG_ROUTING_NONE); + delegate_->CreateRenderViewForRenderManager( + render_frame_host_->render_view_host(), MSG_ROUTING_NONE); } // If the renderer crashed, then try to create a new one to satisfy this // navigation request. - if (!dest_render_view_host->IsRenderViewLive()) { + if (!dest_render_frame_host->render_view_host()->IsRenderViewLive()) { // Recreate the opener chain. int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager( - dest_render_view_host->GetSiteInstance()); - if (!InitRenderView(dest_render_view_host, opener_route_id)) + dest_render_frame_host->render_view_host()->GetSiteInstance()); + if (!InitRenderView(dest_render_frame_host->render_view_host(), + opener_route_id)) return NULL; // Now that we've created a new renderer, be sure to hide it if it isn't // our primary one. Otherwise, we might crash if we try to call Show() // on it later. - if (dest_render_view_host != render_view_host_ && - dest_render_view_host->GetView()) { - dest_render_view_host->GetView()->Hide(); + if (dest_render_frame_host != render_frame_host_ && + dest_render_frame_host->render_view_host()->GetView()) { + dest_render_frame_host->render_view_host()->GetView()->Hide(); } else { // This is our primary renderer, notify here as we won't be calling // CommitPending (which does the notify). - delegate_->NotifySwappedFromRenderManager(NULL, render_view_host_); + delegate_->NotifySwappedFromRenderManager( + NULL, render_frame_host_->render_view_host()); } } - return dest_render_view_host; + // TODO(creis): Return the RFH instead, once we can navigate RFHs. + return dest_render_frame_host->render_view_host(); } void RenderFrameHostManager::Stop() { - render_view_host_->Stop(); + render_frame_host_->render_view_host()->Stop(); // If we are cross-navigating, we should stop the pending renderers. This // will lead to a DidFailProvisionalLoad, which will properly destroy them. if (cross_navigation_pending_) { - pending_render_view_host_->Send( - new ViewMsg_Stop(pending_render_view_host_->GetRoutingID())); + pending_render_frame_host_->render_view_host()->Send(new ViewMsg_Stop( + pending_render_frame_host_->render_view_host()->GetRoutingID())); } } void RenderFrameHostManager::SetIsLoading(bool is_loading) { - render_view_host_->SetIsLoading(is_loading); - if (pending_render_view_host_) - pending_render_view_host_->SetIsLoading(is_loading); + render_frame_host_->render_view_host()->SetIsLoading(is_loading); + if (pending_render_frame_host_) + pending_render_frame_host_->render_view_host()->SetIsLoading(is_loading); } bool RenderFrameHostManager::ShouldCloseTabOnUnresponsiveRenderer() { if (!cross_navigation_pending_) return true; - // We should always have a pending RVH when there's a cross-process navigation + // We should always have a pending RFH when there's a cross-process navigation // in progress. Sanity check this for http://crbug.com/276333. - CHECK(pending_render_view_host_); + CHECK(pending_render_frame_host_); // If the tab becomes unresponsive during {before}unload while doing a // cross-site navigation, proceed with the navigation. (This assumes that - // the pending RenderViewHost is still responsive.) - if (render_view_host_->is_waiting_for_unload_ack()) { + // the pending RenderFrameHost is still responsive.) + if (render_frame_host_->render_view_host()->is_waiting_for_unload_ack()) { // The request has been started and paused while we're waiting for the // unload handler to finish. We'll pretend that it did. The pending // renderer will then be swapped in as part of the usual DidNavigate logic. // (If the unload handler later finishes, this call will be ignored because // the pending_nav_params_ state will already be cleaned up.) current_host()->OnSwappedOut(true); - } else if (render_view_host_->is_waiting_for_beforeunload_ack()) { + } else if (render_frame_host_->render_view_host()-> + is_waiting_for_beforeunload_ack()) { // Haven't gotten around to starting the request, because we're still // waiting for the beforeunload handler to finish. We'll pretend that it // did finish, to let the navigation proceed. Note that there's a danger // that the beforeunload handler will later finish and possibly return // false (meaning the navigation should not proceed), but we'll ignore it // in this case because it took too long. - if (pending_render_view_host_->are_navigations_suspended()) - pending_render_view_host_->SetNavigationsSuspended( + if (pending_render_frame_host_->render_view_host()-> + are_navigations_suspended()) { + pending_render_frame_host_->render_view_host()->SetNavigationsSuspended( false, base::TimeTicks::Now()); + } } return false; } +// TODO(creis): This should take in a RenderFrameHost. void RenderFrameHostManager::SwappedOut(RenderViewHost* render_view_host) { // Make sure this is from our current RVH, and that we have a pending // navigation from OnCrossSiteResponse. (There may be no pending navigation // for data URLs that don't make network requests, for example.) If not, // just return early and ignore. - if (render_view_host != render_view_host_ || !pending_nav_params_.get()) { + if (render_view_host != render_frame_host_->render_view_host() || + !pending_nav_params_.get()) { pending_nav_params_.reset(); return; } @@ -275,23 +291,24 @@ void RenderFrameHostManager::SwappedOut(RenderViewHost* render_view_host) { pending_nav_params_->global_request_id, pending_nav_params_->should_replace_current_entry, true); - } else if (pending_render_view_host_) { + } else if (pending_render_frame_host_) { RenderProcessHostImpl* pending_process = static_cast<RenderProcessHostImpl*>( - pending_render_view_host_->GetProcess()); + pending_render_frame_host_->GetProcess()); pending_process->ResumeDeferredNavigation( pending_nav_params_->global_request_id); } pending_nav_params_.reset(); } +// TODO(creis): This should take in a RenderFrameHost. void RenderFrameHostManager::DidNavigateMainFrame( RenderViewHost* render_view_host) { if (!cross_navigation_pending_) { - DCHECK(!pending_render_view_host_); + DCHECK(!pending_render_frame_host_); // We should only hear this from our current renderer. - DCHECK(render_view_host == render_view_host_); + DCHECK(render_view_host == render_frame_host_->render_view_host()); // Even when there is no pending RVH, there may be a pending Web UI. if (pending_web_ui()) @@ -299,17 +316,18 @@ void RenderFrameHostManager::DidNavigateMainFrame( return; } - if (render_view_host == pending_render_view_host_) { + if (render_view_host == pending_render_frame_host_->render_view_host()) { // The pending cross-site navigation completed, so show the renderer. // If it committed without sending network requests (e.g., data URLs), - // then we still need to swap out the old RVH first and run its unload + // then we still need to swap out the old RFH first and run its unload // handler. OK for that to happen in the background. - if (pending_render_view_host_->HasPendingCrossSiteRequest()) + if (pending_render_frame_host_->render_view_host()-> + HasPendingCrossSiteRequest()) SwapOutOldPage(); CommitPending(); cross_navigation_pending_ = false; - } else if (render_view_host == render_view_host_) { + } else if (render_view_host == render_frame_host_->render_view_host()) { // A navigation in the original page has taken place. Cancel the pending // one. CancelPending(); @@ -320,14 +338,15 @@ void RenderFrameHostManager::DidNavigateMainFrame( } } +// TODO(creis): Take in RenderFrameHost instead, since frames can have openers. void RenderFrameHostManager::DidDisownOpener(RenderViewHost* render_view_host) { // Notify all swapped out hosts, including the pending RVH. - for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin(); iter != swapped_out_hosts_.end(); ++iter) { - DCHECK_NE(iter->second->GetSiteInstance(), + DCHECK_NE(iter->second->render_view_host()->GetSiteInstance(), current_host()->GetSiteInstance()); - iter->second->DisownOpener(); + iter->second->render_view_host()->DisownOpener(); } } @@ -351,7 +370,7 @@ void RenderFrameHostManager::RendererProcessClosing( // swap them back in while the process is exiting. Start by finding them, // since there could be more than one. std::list<int> ids_to_remove; - for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin(); iter != swapped_out_hosts_.end(); ++iter) { if (iter->second->GetProcess() == render_process_host) @@ -360,7 +379,7 @@ void RenderFrameHostManager::RendererProcessClosing( // Now delete them. while (!ids_to_remove.empty()) { - swapped_out_hosts_[ids_to_remove.back()]->Shutdown(); + delete swapped_out_hosts_[ids_to_remove.back()]; swapped_out_hosts_.erase(ids_to_remove.back()); ids_to_remove.pop_back(); } @@ -381,9 +400,11 @@ void RenderFrameHostManager::ShouldClosePage( // might be because the renderer was deemed unresponsive and this call was // already made by ShouldCloseTabOnUnresponsiveRenderer. In that case, it // is ok to do nothing here. - if (pending_render_view_host_ && - pending_render_view_host_->are_navigations_suspended()) { - pending_render_view_host_->SetNavigationsSuspended(false, proceed_time); + if (pending_render_frame_host_ && + pending_render_frame_host_->render_view_host()-> + are_navigations_suspended()) { + pending_render_frame_host_->render_view_host()-> + SetNavigationsSuspended(false, proceed_time); } } else { // Current page says to cancel. @@ -397,20 +418,21 @@ void RenderFrameHostManager::ShouldClosePage( &proceed_to_fire_unload); if (proceed_to_fire_unload) { - // If we're about to close the tab and there's a pending RVH, cancel it. - // Otherwise, if the navigation in the pending RVH completes before the - // close in the current RVH, we'll lose the tab close. - if (pending_render_view_host_) { + // If we're about to close the tab and there's a pending RFH, cancel it. + // Otherwise, if the navigation in the pending RFH completes before the + // close in the current RFH, we'll lose the tab close. + if (pending_render_frame_host_) { CancelPending(); cross_navigation_pending_ = false; } // This is not a cross-site navigation, the tab is being closed. - render_view_host_->ClosePage(); + render_frame_host_->render_view_host()->ClosePage(); } } } +// TODO(creis): Take in a RenderFrameHost from CSRH. void RenderFrameHostManager::OnCrossSiteResponse( RenderViewHost* pending_render_view_host, const GlobalRequestID& global_request_id, @@ -422,9 +444,9 @@ void RenderFrameHostManager::OnCrossSiteResponse( bool should_replace_current_entry) { // This should be called either when the pending RVH is ready to commit or // when we realize that the current RVH's request requires a transfer. - DCHECK( - pending_render_view_host == pending_render_view_host_ || - pending_render_view_host == render_view_host_); + DCHECK(pending_render_view_host == render_frame_host_->render_view_host() || + pending_render_view_host == + pending_render_frame_host_->render_view_host()); // TODO(creis): Eventually we will want to check all navigation responses // here, but currently we pass information for a transfer if @@ -446,7 +468,7 @@ void RenderFrameHostManager::SwapOutOldPage() { // Tell the renderer to suppress any further modal dialogs so that we can swap // it out. This must be done before canceling any current dialog, in case // there is a loop creating additional dialogs. - render_view_host_->SuppressDialogsUntilSwapOut(); + render_frame_host_->render_view_host()->SuppressDialogsUntilSwapOut(); // Now close any modal dialogs that would prevent us from swapping out. This // must be done separately from SwapOut, so that the PageGroupLoadDeferrer is @@ -458,14 +480,18 @@ void RenderFrameHostManager::SwapOutOldPage() { // unload handler finishes and the navigation completes, we will send a // message to the ResourceDispatcherHost, allowing the pending RVH's response // to resume. - render_view_host_->SwapOut(); + // TODO(creis): We should do this on the RFH or else we'll swap out the + // top-level page when subframes navigate. + render_frame_host_->render_view_host()->SwapOut(); // ResourceDispatcherHost has told us to run the onunload handler, which // means it is not a download or unsafe page, and we are going to perform the // navigation. Thus, we no longer need to remember that the RenderViewHost // is part of a pending cross-site request. - if (pending_render_view_host_) - pending_render_view_host_->SetHasPendingCrossSiteRequest(false); + if (pending_render_frame_host_) { + pending_render_frame_host_->render_view_host()-> + SetHasPendingCrossSiteRequest(false); + } } void RenderFrameHostManager::Observe( @@ -484,6 +510,17 @@ void RenderFrameHostManager::Observe( } } +bool RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance( + int32 site_instance_id, + FrameTreeNode* node) { + RenderFrameHostMap::iterator iter = + node->render_manager()->swapped_out_hosts_.find(site_instance_id); + if (iter != node->render_manager()->swapped_out_hosts_.end()) + delete iter->second; + + return true; +} + bool RenderFrameHostManager::ShouldTransitionCrossSite() { // False in the single-process mode, as it makes RVHs to accumulate // in swapped_out_hosts_. @@ -516,7 +553,7 @@ bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation( const GURL& current_url = (current_entry) ? SiteInstanceImpl::GetEffectiveURL(browser_context, current_entry->GetURL()) : - render_view_host_->GetSiteInstance()->GetSiteURL(); + render_frame_host_->render_view_host()->GetSiteInstance()->GetSiteURL(); const GURL& new_url = SiteInstanceImpl::GetEffectiveURL(browser_context, new_entry->GetURL()); @@ -541,7 +578,8 @@ bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation( // Check with the content client as well. Important to pass current_url here, // which uses the SiteInstance's site if there is no current_entry. if (GetContentClient()->browser()->ShouldSwapBrowsingInstancesForNavigation( - render_view_host_->GetSiteInstance(), current_url, new_url)) { + render_frame_host_->render_view_host()->GetSiteInstance(), + current_url, new_url)) { return true; } @@ -721,7 +759,38 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry( return current_instance->GetRelatedSiteInstance(dest_url); } -int RenderFrameHostManager::CreateRenderView( +RenderFrameHostImpl* RenderFrameHostManager::CreateRenderFrameHost( + SiteInstance* site_instance, + int view_routing_id, + int frame_routing_id, + bool swapped_out, + bool hidden) { + if (frame_routing_id == MSG_ROUTING_NONE) + frame_routing_id = site_instance->GetProcess()->GetNextRoutingID(); + + // Create a RVH for main frames, or find the existing one for subframes. + FrameTree* frame_tree = frame_tree_node_->frame_tree(); + RenderViewHostImpl* render_view_host = NULL; + if (frame_tree_node_->IsMainFrame()) { + render_view_host = frame_tree->CreateRenderViewHostForMainFrame( + site_instance, view_routing_id, frame_routing_id, swapped_out, hidden); + } else { + render_view_host = frame_tree->GetRenderViewHostForSubFrame(site_instance); + } + + // TODO(creis): Make render_frame_host a scoped_ptr. + // TODO(creis): Pass hidden to RFH. + RenderFrameHostImpl* render_frame_host = + RenderFrameHostFactory::Create(render_view_host, + render_frame_delegate_, + frame_tree, + frame_tree_node_, + frame_routing_id, + swapped_out).release(); + return render_frame_host; +} + +int RenderFrameHostManager::CreateRenderFrame( SiteInstance* instance, int opener_route_id, bool swapped_out, @@ -729,59 +798,61 @@ int RenderFrameHostManager::CreateRenderView( CHECK(instance); DCHECK(!swapped_out || hidden); // Swapped out views should always be hidden. - // We are creating a pending or swapped out RVH here. We should never create - // it in the same SiteInstance as our current RVH. - CHECK_NE(render_view_host_->GetSiteInstance(), instance); + // We are creating a pending or swapped out RFH here. We should never create + // it in the same SiteInstance as our current RFH. + CHECK_NE(render_frame_host_->render_view_host()->GetSiteInstance(), instance); - // Check if we've already created an RVH for this SiteInstance. If so, try + // Check if we've already created an RFH for this SiteInstance. If so, try // to re-use the existing one, which has already been initialized. We'll // remove it from the list of swapped out hosts if it commits. - RenderViewHostImpl* new_render_view_host = static_cast<RenderViewHostImpl*>( - GetSwappedOutRenderViewHost(instance)); - if (new_render_view_host) { + RenderFrameHostImpl* new_render_frame_host = + GetSwappedOutRenderFrameHost(instance); + if (new_render_frame_host) { // Prevent the process from exiting while we're trying to use it. if (!swapped_out) - new_render_view_host->GetProcess()->AddPendingView(); + new_render_frame_host->GetProcess()->AddPendingView(); } else { - // Create a new RenderViewHost if we don't find an existing one. - new_render_view_host = static_cast<RenderViewHostImpl*>( - RenderViewHostFactory::Create(instance, - render_view_delegate_, - render_frame_delegate_, - render_widget_delegate_, - MSG_ROUTING_NONE, - MSG_ROUTING_NONE, - swapped_out, - hidden)); - - // If the new RVH is swapped out already, store it. Otherwise prevent the + // Create a new RenderFrameHost if we don't find an existing one. + // TODO(creis): Make new_render_frame_host a scoped_ptr. + new_render_frame_host = CreateRenderFrameHost(instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE, swapped_out, + hidden); + + // If the new RFH is swapped out already, store it. Otherwise prevent the // process from exiting while we're trying to navigate in it. if (swapped_out) { - swapped_out_hosts_[instance->GetId()] = new_render_view_host; + swapped_out_hosts_[instance->GetId()] = new_render_frame_host; } else { - new_render_view_host->GetProcess()->AddPendingView(); + new_render_frame_host->GetProcess()->AddPendingView(); } - bool success = InitRenderView(new_render_view_host, opener_route_id); - if (success) { - // Don't show the view until we get a DidNavigate from it. - new_render_view_host->GetView()->Hide(); + RenderViewHostImpl* render_view_host = + new_render_frame_host->render_view_host(); + bool success = InitRenderView(render_view_host, opener_route_id); + if (success && frame_tree_node_->IsMainFrame()) { + // Don't show the main frame's view until we get a DidNavigate from it. + render_view_host->GetView()->Hide(); } else if (!swapped_out) { CancelPending(); } } - // Use this as our new pending RVH if it isn't swapped out. + // Use this as our new pending RFH if it isn't swapped out. if (!swapped_out) - pending_render_view_host_ = new_render_view_host; + pending_render_frame_host_ = new_render_frame_host; - return new_render_view_host->GetRoutingID(); + return new_render_frame_host->render_view_host()->GetRoutingID(); } bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host, int opener_route_id) { + // We may have initialized this RenderViewHost for another RenderFrameHost. + if (render_view_host->IsRenderViewLive()) + return true; + // If the pending navigation is to a WebUI and the RenderView is not in a - // guest process, tell the RenderView about any bindings it will need enabled. + // guest process, tell the RenderViewHost about any bindings it will need + // enabled. if (pending_web_ui() && !render_view_host->GetProcess()->IsGuest()) { render_view_host->AllowBindings(pending_web_ui()->GetBindings()); } else { @@ -822,10 +893,10 @@ void RenderFrameHostManager::CommitPending() { else if (!pending_and_current_web_ui_.get()) web_ui_.reset(); - // It's possible for the pending_render_view_host_ to be NULL when we aren't + // It's possible for the pending_render_frame_host_ to be NULL when we aren't // crossing process boundaries. If so, we just needed to handle the Web UI // committing above and we're done. - if (!pending_render_view_host_) { + if (!pending_render_frame_host_) { if (will_focus_location_bar) delegate_->SetFocusToLocationBar(false); return; @@ -834,89 +905,110 @@ void RenderFrameHostManager::CommitPending() { // Remember if the page was focused so we can focus the new renderer in // that case. bool focus_render_view = !will_focus_location_bar && - render_view_host_->GetView() && render_view_host_->GetView()->HasFocus(); + render_frame_host_->render_view_host()->GetView() && + render_frame_host_->render_view_host()->GetView()->HasFocus(); - // Swap in the pending view and make it active. Also ensure the FrameTree + // Swap in the pending frame and make it active. Also ensure the FrameTree // stays in sync. - RenderViewHostImpl* old_render_view_host = render_view_host_; - render_view_host_ = pending_render_view_host_; - pending_render_view_host_ = NULL; - render_view_host_->AttachToFrameTree(); + RenderFrameHostImpl* old_render_frame_host = render_frame_host_; + render_frame_host_ = pending_render_frame_host_; + pending_render_frame_host_ = NULL; + render_frame_host_->render_view_host()->AttachToFrameTree(); // The process will no longer try to exit, so we can decrement the count. - render_view_host_->GetProcess()->RemovePendingView(); + render_frame_host_->GetProcess()->RemovePendingView(); + + // TODO(creis): As long as show/hide are on RVH, we don't want to do them for + // subframe navigations or they'll interfere with the top-level page. + bool is_main_frame = frame_tree_node_->IsMainFrame(); // If the view is gone, then this RenderViewHost died while it was hidden. // We ignored the RenderProcessGone call at the time, so we should send it now // to make sure the sad tab shows up, etc. - if (!render_view_host_->GetView()) - delegate_->RenderProcessGoneFromRenderManager(render_view_host_); - else if (!delegate_->IsHidden()) - render_view_host_->GetView()->Show(); + if (!render_frame_host_->render_view_host()->GetView()) { + delegate_->RenderProcessGoneFromRenderManager( + render_frame_host_->render_view_host()); + } else if (!delegate_->IsHidden() && is_main_frame) { + render_frame_host_->render_view_host()->GetView()->Show(); + } // Hide the old view now that the new one is visible. - if (old_render_view_host->GetView()) { - old_render_view_host->GetView()->Hide(); - old_render_view_host->WasSwappedOut(); + if (old_render_frame_host->render_view_host()->GetView()) { + old_render_frame_host->render_view_host()->GetView()->Hide(); + old_render_frame_host->render_view_host()->WasSwappedOut(); } // Make sure the size is up to date. (Fix for bug 1079768.) delegate_->UpdateRenderViewSizeForRenderManager(); - if (will_focus_location_bar) + if (will_focus_location_bar) { delegate_->SetFocusToLocationBar(false); - else if (focus_render_view && render_view_host_->GetView()) - RenderWidgetHostViewPort::FromRWHV(render_view_host_->GetView())->Focus(); - - // Notify that we've swapped RenderViewHosts. We do this - // before shutting down the RVH so that we can clean up - // RendererResources related to the RVH first. - delegate_->NotifySwappedFromRenderManager(old_render_view_host, - render_view_host_); - - // If the pending view was on the swapped out list, we can remove it. - swapped_out_hosts_.erase(render_view_host_->GetSiteInstance()->GetId()); - - // If there are no active RVHs in this SiteInstance, it means that - // this RVH was the last active one in the SiteInstance. Now that we - // know that all RVHs are swapped out, we can delete all the RVHs in - // this SiteInstance. - if (!static_cast<SiteInstanceImpl*>(old_render_view_host->GetSiteInstance())-> - active_view_count()) { - ShutdownRenderViewHostsInSiteInstance( - old_render_view_host->GetSiteInstance()->GetId()); - // This is deleted while cleaning up the SitaInstance's views. - old_render_view_host = NULL; - } else if (old_render_view_host->IsRenderViewLive()) { - // If the old RVH is live, we are swapping it out and should keep track of + } else if (focus_render_view && + render_frame_host_->render_view_host()->GetView()) { + RenderWidgetHostViewPort::FromRWHV( + render_frame_host_->render_view_host()->GetView())->Focus(); + } + + // Notify that we've swapped RenderFrameHosts. We do this before shutting down + // the RFH so that we can clean up RendererResources related to the RFH first. + // TODO(creis): Only do this on top-level RFHs for now, and later update it to + // pass the RFHs. + if (is_main_frame) { + delegate_->NotifySwappedFromRenderManager( + old_render_frame_host->render_view_host(), + render_frame_host_->render_view_host()); + } + + // If the pending frame was on the swapped out list, we can remove it. + swapped_out_hosts_.erase(render_frame_host_->render_view_host()-> + GetSiteInstance()->GetId()); + + if (old_render_frame_host->render_view_host()->IsRenderViewLive()) { + // If the old RFH is live, we are swapping it out and should keep track of // it in case we navigate back to it. - DCHECK(old_render_view_host->is_swapped_out()); + DCHECK(old_render_frame_host->render_view_host()->is_swapped_out()); // Temp fix for http://crbug.com/90867 until we do a better cleanup to make // sure we don't get different rvh instances for the same site instance // in the same rvhmgr. // TODO(creis): Clean this up. int32 old_site_instance_id = - old_render_view_host->GetSiteInstance()->GetId(); - RenderViewHostMap::iterator iter = + old_render_frame_host->render_view_host()->GetSiteInstance()->GetId(); + RenderFrameHostMap::iterator iter = swapped_out_hosts_.find(old_site_instance_id); if (iter != swapped_out_hosts_.end() && - iter->second != old_render_view_host) { - // Shutdown the RVH that will be replaced in the map to avoid a leak. - iter->second->Shutdown(); + iter->second != old_render_frame_host) { + // Delete the RFH that will be replaced in the map to avoid a leak. + delete iter->second; + } + swapped_out_hosts_[old_site_instance_id] = old_render_frame_host; + + // If there are no active views in this SiteInstance, it means that + // this RFH was the last active one in the SiteInstance. Now that we + // know that all RFHs are swapped out, we can delete all the RFHs and RVHs + // in this SiteInstance. We do this after ensuring the RFH is on the + // swapped out list to simplify the deletion. + if (!static_cast<SiteInstanceImpl*>( + old_render_frame_host->render_view_host()->GetSiteInstance())-> + active_view_count()) { + ShutdownRenderFrameHostsInSiteInstance(old_site_instance_id); + // This is deleted while cleaning up the SiteInstance's views. + old_render_frame_host = NULL; } - swapped_out_hosts_[old_site_instance_id] = old_render_view_host; } else { - old_render_view_host->Shutdown(); - old_render_view_host = NULL; // Shutdown() deletes it. + delete old_render_frame_host; } } -void RenderFrameHostManager::ShutdownRenderViewHostsInSiteInstance( +void RenderFrameHostManager::ShutdownRenderFrameHostsInSiteInstance( int32 site_instance_id) { - // First remove any swapped out RVH for this SiteInstance from our - // list. - swapped_out_hosts_.erase(site_instance_id); - + // First remove any swapped out RFH for this SiteInstance from our own list. + ClearSwappedOutRFHsInSiteInstance(site_instance_id, frame_tree_node_); + + // Use the safe RenderWidgetHost iterator for now to find all RenderViewHosts + // in the SiteInstance, then tell their respective FrameTrees to remove all + // swapped out RenderFrameHosts corresponding to them. + // TODO(creis): Replace this with a RenderFrameHostIterator that protects + // against use-after-frees if a later element is deleted before getting to it. scoped_ptr<RenderWidgetHostIterator> widgets( RenderWidgetHostImpl::GetAllRenderWidgetHosts()); while (RenderWidgetHost* widget = widgets->GetNextHost()) { @@ -924,25 +1016,33 @@ void RenderFrameHostManager::ShutdownRenderViewHostsInSiteInstance( continue; RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(RenderViewHost::From(widget)); - if (site_instance_id == rvh->GetSiteInstance()->GetId()) - rvh->Shutdown(); + if (site_instance_id == rvh->GetSiteInstance()->GetId()) { + // This deletes all RenderFrameHosts using the |rvh|, which then causes + // |rvh| to Shutdown. + FrameTree* tree = rvh->GetDelegate()->GetFrameTree(); + tree->ForEach(base::Bind( + &RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance, + site_instance_id)); + // rvh is now deleted. + } } } -RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( +RenderFrameHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( const NavigationEntryImpl& entry) { // If we are currently navigating cross-process, we want to get back to normal // and then navigate as usual. if (cross_navigation_pending_) { - if (pending_render_view_host_) + if (pending_render_frame_host_) CancelPending(); cross_navigation_pending_ = false; } - // render_view_host_'s SiteInstance and new_instance will not be deleted + // render_frame_host_'s SiteInstance and new_instance will not be deleted // before the end of this method, so we don't have to worry about their ref // counts dropping to zero. - SiteInstance* current_instance = render_view_host_->GetSiteInstance(); + SiteInstance* current_instance = + render_frame_host_->render_view_host()->GetSiteInstance(); SiteInstance* new_instance = current_instance; // We do not currently swap processes for navigations in webview tag guests. @@ -950,7 +1050,7 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( // Determine if we need a new BrowsingInstance for this entry. If true, this // implies that it will get a new SiteInstance (and likely process), and that - // other tabs in the current BrosingInstance will be unalbe to script it. + // other tabs in the current BrowsingInstance will be unable to script it. // This is used for cases that require a process swap even in the // process-per-tab model, such as WebUI pages. const NavigationEntry* current_entry = @@ -961,13 +1061,13 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap); // If force_swap is true, we must use a different SiteInstance. If we didn't, - // we would have two RenderViewHosts in the same SiteInstance and the same - // tab, resulting in page_id conflicts for their NavigationEntries. + // we would have two RenderFrameHosts in the same SiteInstance and the same + // frame, resulting in page_id conflicts for their NavigationEntries. if (force_swap) CHECK_NE(new_instance, current_instance); if (new_instance != current_instance) { - // New SiteInstance: create a pending RVH to navigate. + // New SiteInstance: create a pending RFH to navigate. DCHECK(!cross_navigation_pending_); // This will possibly create (set to NULL) a Web UI object for the pending @@ -978,8 +1078,8 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( // not have its bindings set appropriately. SetPendingWebUI(entry); - // Ensure that we have created RVHs for the new RVH's opener chain if - // we are staying in the same BrowsingInstance. This allows the pending RVH + // Ensure that we have created RFHs for the new RFH's opener chain if + // we are staying in the same BrowsingInstance. This allows the pending RFH // to send cross-process script calls to its opener(s). int opener_route_id = MSG_ROUTING_NONE; if (new_instance->IsRelatedSiteInstance(current_instance)) { @@ -987,26 +1087,26 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( delegate_->CreateOpenerRenderViewsForRenderManager(new_instance); } - // Create a non-swapped-out pending RVH with the given opener and navigate + // Create a non-swapped-out pending RFH with the given opener and navigate // it. - int route_id = CreateRenderView(new_instance, opener_route_id, false, - delegate_->IsHidden()); + int route_id = CreateRenderFrame(new_instance, opener_route_id, false, + delegate_->IsHidden()); if (route_id == MSG_ROUTING_NONE) return NULL; - // Check if our current RVH is live before we set up a transition. - if (!render_view_host_->IsRenderViewLive()) { + // Check if our current RFH is live before we set up a transition. + if (!render_frame_host_->render_view_host()->IsRenderViewLive()) { if (!cross_navigation_pending_) { - // The current RVH is not live. There's no reason to sit around with a - // sad tab or a newly created RVH while we wait for the pending RVH to - // navigate. Just switch to the pending RVH now and go back to non + // The current RFH is not live. There's no reason to sit around with a + // sad tab or a newly created RFH while we wait for the pending RFH to + // navigate. Just switch to the pending RFH now and go back to non // cross-navigating (Note that we don't care about on{before}unload - // handlers if the current RVH isn't live.) + // handlers if the current RFH isn't live.) CommitPending(); - return render_view_host_; + return render_frame_host_; } else { NOTREACHED(); - return render_view_host_; + return render_frame_host_; } } // Otherwise, it's safe to treat this as a pending cross-site transition. @@ -1017,7 +1117,8 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( // Navigate message) until we hear back from the old renderer's // beforeunload handler. If the handler returns false, we'll have to // cancel the request. - DCHECK(!pending_render_view_host_->are_navigations_suspended()); + DCHECK(!pending_render_frame_host_->render_view_host()-> + are_navigations_suspended()); bool is_transfer = entry.transferred_global_request_id() != GlobalRequestID(); if (is_transfer) { @@ -1029,19 +1130,21 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( // Also make sure the old render view stops, in case a load is in // progress. (We don't want to do this for transfers, since it will // interrupt the transfer with an unexpected DidStopLoading.) - render_view_host_->Send( - new ViewMsg_Stop(render_view_host_->GetRoutingID())); + render_frame_host_->render_view_host()->Send(new ViewMsg_Stop( + render_frame_host_->render_view_host()->GetRoutingID())); - pending_render_view_host_->SetNavigationsSuspended(true, - base::TimeTicks()); + pending_render_frame_host_->render_view_host()->SetNavigationsSuspended( + true, base::TimeTicks()); // Tell the CrossSiteRequestManager that this RVH has a pending cross-site // request, so that ResourceDispatcherHost will know to tell us to run the // old page's unload handler before it sends the response. - pending_render_view_host_->SetHasPendingCrossSiteRequest(true); + // TODO(creis): This needs to be on the RFH. + pending_render_frame_host_->render_view_host()-> + SetHasPendingCrossSiteRequest(true); } - // We now have a pending RVH. + // We now have a pending RFH. DCHECK(!cross_navigation_pending_); cross_navigation_pending_ = true; @@ -1050,12 +1153,12 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( // doesn't otherwise know that the cross-site request is happening. This // will trigger a call to ShouldClosePage with the reply. if (!is_transfer) - render_view_host_->FirePageBeforeUnload(true); + render_frame_host_->render_view_host()->FirePageBeforeUnload(true); - return pending_render_view_host_; + return pending_render_frame_host_; } - // Otherwise the same SiteInstance can be used. Navigate render_view_host_. + // Otherwise the same SiteInstance can be used. Navigate render_frame_host_. DCHECK(!cross_navigation_pending_); if (ShouldReuseWebUI(current_entry, &entry)) { pending_web_ui_.reset(); @@ -1064,46 +1167,53 @@ RenderViewHostImpl* RenderFrameHostManager::UpdateRendererStateForNavigate( SetPendingWebUI(entry); // Make sure the new RenderViewHost has the right bindings. - if (pending_web_ui() && !render_view_host_->GetProcess()->IsGuest()) - render_view_host_->AllowBindings(pending_web_ui()->GetBindings()); + if (pending_web_ui() && !render_frame_host_->GetProcess()->IsGuest()) { + render_frame_host_->render_view_host()->AllowBindings( + pending_web_ui()->GetBindings()); + } } - if (pending_web_ui() && render_view_host_->IsRenderViewLive()) - pending_web_ui()->GetController()->RenderViewReused(render_view_host_); + if (pending_web_ui() && + render_frame_host_->render_view_host()->IsRenderViewLive()) { + pending_web_ui()->GetController()->RenderViewReused( + render_frame_host_->render_view_host()); + } // The renderer can exit view source mode when any error or cancellation // happen. We must overwrite to recover the mode. if (entry.IsViewSourceMode()) { - render_view_host_->Send( - new ViewMsg_EnableViewSourceMode(render_view_host_->GetRoutingID())); + render_frame_host_->render_view_host()->Send( + new ViewMsg_EnableViewSourceMode( + render_frame_host_->render_view_host()->GetRoutingID())); } - return render_view_host_; + return render_frame_host_; } void RenderFrameHostManager::CancelPending() { - RenderViewHostImpl* pending_render_view_host = pending_render_view_host_; - pending_render_view_host_ = NULL; + RenderFrameHostImpl* pending_render_frame_host = pending_render_frame_host_; + pending_render_frame_host_ = NULL; RenderViewDevToolsAgentHost::OnCancelPendingNavigation( - pending_render_view_host, - render_view_host_); + pending_render_frame_host->render_view_host(), + render_frame_host_->render_view_host()); // We no longer need to prevent the process from exiting. - pending_render_view_host->GetProcess()->RemovePendingView(); + pending_render_frame_host->GetProcess()->RemovePendingView(); - // The pending RVH may already be on the swapped out list if we started to + // The pending RFH may already be on the swapped out list if we started to // swap it back in and then canceled. If so, make sure it gets swapped out // again. If it's not on the swapped out list (e.g., aborting a pending // load), then it's safe to shut down. - if (IsOnSwappedOutList(pending_render_view_host)) { + if (IsOnSwappedOutList(pending_render_frame_host)) { // Any currently suspended navigations are no longer needed. - pending_render_view_host->CancelSuspendedNavigations(); + pending_render_frame_host->render_view_host()->CancelSuspendedNavigations(); - pending_render_view_host->SwapOut(); + // TODO(creis): We need to swap out the RFH. + pending_render_frame_host->render_view_host()->SwapOut(); } else { // We won't be coming back, so shut this one down. - pending_render_view_host->Shutdown(); + delete pending_render_frame_host; } pending_web_ui_.reset(); @@ -1112,47 +1222,69 @@ void RenderFrameHostManager::CancelPending() { void RenderFrameHostManager::RenderViewDeleted(RenderViewHost* rvh) { // We are doing this in order to work around and to track a crasher - // (http://crbug.com/23411) where it seems that pending_render_view_host_ is + // (http://crbug.com/23411) where it seems that pending_render_frame_host_ is // deleted (not sure from where) but not NULLed. - if (rvh == pending_render_view_host_) { + if (pending_render_frame_host_ && + rvh == pending_render_frame_host_->render_view_host()) { // If you hit this NOTREACHED, please report it in the following bug // http://crbug.com/23411 Make sure to include what you were doing when it // happened (navigating to a new page, closing a tab...) and if you can // reproduce. NOTREACHED(); - pending_render_view_host_ = NULL; + pending_render_frame_host_ = NULL; } // Make sure deleted RVHs are not kept in the swapped out list while we are - // still alive. (If render_view_host_ is null, we're already being deleted.) - if (!render_view_host_) + // still alive. (If render_frame_host_ is null, we're already being deleted.) + if (!render_frame_host_) return; + // We can't look it up by SiteInstance ID, which may no longer be valid. - for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin(); iter != swapped_out_hosts_.end(); ++iter) { - if (iter->second == rvh) { + if (iter->second->render_view_host() == rvh) { swapped_out_hosts_.erase(iter); break; } } } -bool RenderFrameHostManager::IsOnSwappedOutList(RenderViewHost* rvh) const { - if (!rvh->GetSiteInstance()) +bool RenderFrameHostManager::IsRVHOnSwappedOutList( + RenderViewHostImpl* rvh) const { + RenderFrameHostImpl* render_frame_host = GetSwappedOutRenderFrameHost( + rvh->GetSiteInstance()); + if (!render_frame_host) return false; + return IsOnSwappedOutList(render_frame_host); +} - RenderViewHostMap::const_iterator iter = swapped_out_hosts_.find( - rvh->GetSiteInstance()->GetId()); +bool RenderFrameHostManager::IsOnSwappedOutList( + RenderFrameHostImpl* rfh) const { + if (!rfh->render_view_host()->GetSiteInstance()) + return false; + + RenderFrameHostMap::const_iterator iter = swapped_out_hosts_.find( + rfh->render_view_host()->GetSiteInstance()->GetId()); if (iter == swapped_out_hosts_.end()) return false; - return iter->second == rvh; + return iter->second == rfh; } RenderViewHostImpl* RenderFrameHostManager::GetSwappedOutRenderViewHost( - SiteInstance* instance) { - RenderViewHostMap::iterator iter = swapped_out_hosts_.find(instance->GetId()); + SiteInstance* instance) const { + RenderFrameHostImpl* render_frame_host = + GetSwappedOutRenderFrameHost(instance); + if (render_frame_host) + return render_frame_host->render_view_host(); + return NULL; +} + +RenderFrameHostImpl* RenderFrameHostManager::GetSwappedOutRenderFrameHost( + SiteInstance* instance) const { + RenderFrameHostMap::const_iterator iter = + swapped_out_hosts_.find(instance->GetId()); if (iter != swapped_out_hosts_.end()) return iter->second; diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h index 94a4998..0360bd0 100644 --- a/content/browser/frame_host/render_frame_host_manager.h +++ b/content/browser/frame_host/render_frame_host_manager.h @@ -20,10 +20,12 @@ namespace content { class BrowserContext; class InterstitialPageImpl; +class FrameTreeNode; class NavigationControllerImpl; class NavigationEntry; class NavigationEntryImpl; class RenderFrameHostDelegate; +class RenderFrameHostImpl; class RenderFrameHostManagerTest; class RenderViewHost; class RenderViewHostImpl; @@ -114,6 +116,7 @@ class CONTENT_EXPORT RenderFrameHostManager // // You must call Init() before using this class. RenderFrameHostManager( + FrameTreeNode* frame_tree_node, RenderFrameHostDelegate* render_frame_delegate, RenderViewHostDelegate* render_view_delegate, RenderWidgetHostDelegate* render_widget_delegate, @@ -123,21 +126,31 @@ class CONTENT_EXPORT RenderFrameHostManager // For arguments, see WebContentsImpl constructor. void Init(BrowserContext* browser_context, SiteInstance* site_instance, - int routing_id, - int main_frame_routing_id); + int view_routing_id, + int frame_routing_id); - // Returns the currently active RenderViewHost. + // Returns the currently active RenderFrameHost. // // This will be non-NULL between Init() and Shutdown(). You may want to NULL // check it in many cases, however. Windows can send us messages during the // destruction process after it has been shut down. + RenderFrameHostImpl* current_frame_host() const { + return render_frame_host_; + } + + // TODO(creis): Remove this when we no longer use RVH for navigation. RenderViewHostImpl* current_host() const; // Returns the view associated with the current RenderViewHost, or NULL if // there is no current one. RenderWidgetHostView* GetRenderWidgetHostView() const; - // Returns the pending render view host, or NULL if there is no pending one. + // Returns the pending RenderFrameHost, or NULL if there is no pending one. + RenderFrameHostImpl* pending_frame_host() const { + return pending_render_frame_host_; + } + + // TODO(creis): Remove this when we no longer use RVH for navigation. RenderViewHostImpl* pending_render_view_host() const; // Returns the current committed Web UI or NULL if none applies. @@ -183,13 +196,13 @@ class CONTENT_EXPORT RenderFrameHostManager // Called when a renderer sets its opener to null. void DidDisownOpener(RenderViewHost* render_view_host); - // Helper method to create a RenderViewHost. If |swapped_out| is true, it - // will be initially placed on the swapped out hosts list. Otherwise, it - // will be used for a pending cross-site navigation. - int CreateRenderView(SiteInstance* instance, - int opener_route_id, - bool swapped_out, - bool hidden); + // Helper method to create and initialize a RenderFrameHost. If |swapped_out| + // is true, it will be initially placed on the swapped out hosts list. + // Otherwise, it will be used for a pending cross-site navigation. + int CreateRenderFrame(SiteInstance* instance, + int opener_route_id, + bool swapped_out, + bool hidden); // Called when a provisional load on the given renderer is aborted. void RendererAbortedProvisionalLoad(RenderViewHost* render_view_host); @@ -236,12 +249,16 @@ class CONTENT_EXPORT RenderFrameHostManager // Called when a RenderViewHost is about to be deleted. void RenderViewDeleted(RenderViewHost* rvh); - // Returns whether the given RenderViewHost is on the list of swapped out - // RenderViewHosts. - bool IsOnSwappedOutList(RenderViewHost* rvh) const; + // Returns whether the given RenderFrameHost (or its associated + // RenderViewHost) is on the list of swapped out RenderFrameHosts. + bool IsRVHOnSwappedOutList(RenderViewHostImpl* rvh) const; + bool IsOnSwappedOutList(RenderFrameHostImpl* rfh) const; - // Returns the swapped out RenderViewHost for the given SiteInstance, if any. - RenderViewHostImpl* GetSwappedOutRenderViewHost(SiteInstance* instance); + // Returns the swapped out RenderViewHost or RenderFrameHost for the given + // SiteInstance, if any. + RenderViewHostImpl* GetSwappedOutRenderViewHost(SiteInstance* instance) const; + RenderFrameHostImpl* GetSwappedOutRenderFrameHost( + SiteInstance* instance) const; // Runs the unload handler in the current page, when we know that a pending // cross-process navigation is going to commit. We may initiate a transfer @@ -253,7 +270,7 @@ class CONTENT_EXPORT RenderFrameHostManager friend class TestWebContents; // Tracks information about a navigation while a cross-process transition is - // in progress, in case we need to transfer it to a new RenderViewHost. + // in progress, in case we need to transfer it to a new RenderFrameHost. struct PendingNavigationParams { PendingNavigationParams(); PendingNavigationParams(const GlobalRequestID& global_request_id, @@ -295,6 +312,11 @@ class CONTENT_EXPORT RenderFrameHostManager bool should_replace_current_entry; }; + // Used with FrameTree::ForEach to erase inactive RenderFrameHosts from a + // FrameTreeNode's RenderFrameHostManager. + static bool ClearSwappedOutRFHsInSiteInstance(int32 site_instance_id, + FrameTreeNode* node); + // Returns whether this tab should transition to a new renderer for // cross-site URLs. Enabled unless we see the --process-per-tab command line // switch. Can be overridden in unit tests. @@ -325,60 +347,76 @@ class CONTENT_EXPORT RenderFrameHostManager SiteInstance* current_instance, bool force_browsing_instance_swap); - // Sets up the necessary state for a new RenderViewHost with the given opener. + // Creates a RenderFrameHost and corresponding RenderViewHost if necessary. + RenderFrameHostImpl* CreateRenderFrameHost(SiteInstance* instance, + int view_routing_id, + int frame_routing_id, + bool swapped_out, + bool hidden); + + // Sets up the necessary state for a new RenderViewHost with the given opener, + // if necessary. Returns early if the RenderViewHost has already been + // initialized for another RenderFrameHost. + // TODO(creis): opener_route_id is currently for the RenderViewHost but should + // be for the RenderFrame, since frames can have openers. bool InitRenderView(RenderViewHost* render_view_host, int opener_route_id); - // Sets the pending RenderViewHost/WebUI to be the active one. Note that this - // doesn't require the pending render_view_host_ pointer to be non-NULL, since - // there could be Web UI switching as well. Call this for every commit. + // Sets the pending RenderFrameHost/WebUI to be the active one. Note that this + // doesn't require the pending render_frame_host_ pointer to be non-NULL, + // since there could be Web UI switching as well. Call this for every commit. void CommitPending(); - // Shutdown all RenderViewHosts in a SiteInstance. This is called - // to shutdown views when all the views in a SiteInstance are - // confirmed to be swapped out. - void ShutdownRenderViewHostsInSiteInstance(int32 site_instance_id); + // Shutdown all RenderFrameHosts in a SiteInstance. This is called to shutdown + // frames when all the frames in a SiteInstance are confirmed to be swapped + // out. + void ShutdownRenderFrameHostsInSiteInstance(int32 site_instance_id); // Helper method to terminate the pending RenderViewHost. void CancelPending(); - RenderViewHostImpl* UpdateRendererStateForNavigate( + RenderFrameHostImpl* UpdateRendererStateForNavigate( const NavigationEntryImpl& entry); // Called when a renderer process is starting to close. We should not - // schedule new navigations in its swapped out RenderViewHosts after this. + // schedule new navigations in its swapped out RenderFrameHosts after this. void RendererProcessClosing(RenderProcessHost* render_process_host); + // For use in creating RenderFrameHosts. + FrameTreeNode* frame_tree_node_; + // Our delegate, not owned by us. Guaranteed non-NULL. Delegate* delegate_; - // Whether a navigation requiring different RenderView's is pending. This is - // either cross-site request is (in the new process model), or when required - // for the view type (like view source versus not). + // Whether a navigation requiring different RenderFrameHosts is pending. This + // is either for cross-site requests or when required for the process type + // (like WebUI). bool cross_navigation_pending_; - // Implemented by the owner of this class, these delegates are installed into - // all the RenderViewHosts that we create. + // Implemented by the owner of this class. These delegates are installed into + // all the RenderFrameHosts that we create. RenderFrameHostDelegate* render_frame_delegate_; RenderViewHostDelegate* render_view_delegate_; RenderWidgetHostDelegate* render_widget_delegate_; - // Our RenderView host and its associated Web UI (if any, will be NULL for - // non-DOM-UI pages). This object is responsible for all communication with - // a child RenderView instance. - RenderViewHostImpl* render_view_host_; + // Our RenderFrameHost and its associated Web UI (if any, will be NULL for + // non-WebUI pages). This object is responsible for all communication with + // a child RenderFrame instance. + // For now, RenderFrameHost keeps a RenderViewHost in its SiteInstance alive. + // Eventually, RenderViewHost will be replaced with a page context. + RenderFrameHostImpl* render_frame_host_; scoped_ptr<WebUIImpl> web_ui_; - // A RenderViewHost used to load a cross-site page. This remains hidden + // A RenderFrameHost used to load a cross-site page. This remains hidden // while a cross-site request is pending until it calls DidNavigate. It may // have an associated Web UI, in which case the Web UI pointer will be non- // NULL. // // The |pending_web_ui_| may be non-NULL even when the - // |pending_render_view_host_| is NULL. This will happen when we're - // transitioning between two Web UI pages: the RVH won't be swapped, so the + // |pending_render_frame_host_| is NULL. This will happen when we're + // transitioning between two Web UI pages: the RFH won't be swapped, so the // pending pointer will be unused, but there will be a pending Web UI // associated with the navigation. - RenderViewHostImpl* pending_render_view_host_; + RenderFrameHostImpl* pending_render_frame_host_; // Tracks information about any current pending cross-process navigation. scoped_ptr<PendingNavigationParams> pending_nav_params_; @@ -390,10 +428,10 @@ class CONTENT_EXPORT RenderFrameHostManager scoped_ptr<WebUIImpl> pending_web_ui_; base::WeakPtr<WebUIImpl> pending_and_current_web_ui_; - // A map of site instance ID to swapped out RenderViewHosts. This may include - // pending_render_view_host_ for navigations to existing entries. - typedef base::hash_map<int32, RenderViewHostImpl*> RenderViewHostMap; - RenderViewHostMap swapped_out_hosts_; + // A map of site instance ID to swapped out RenderFrameHosts. This may + // include pending_render_frame_host_ for navigations to existing entries. + typedef base::hash_map<int32, RenderFrameHostImpl*> RenderFrameHostMap; + RenderFrameHostMap swapped_out_hosts_; // The intersitial page currently shown if any, not own by this class // (the InterstitialPage is self-owned, it deletes itself when hidden). diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc index 2ba5fb8..630262c 100644 --- a/content/browser/frame_host/render_frame_host_manager_unittest.cc +++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc @@ -567,17 +567,24 @@ TEST_F(RenderFrameHostManagerTest, Init) { scoped_ptr<TestWebContents> web_contents( TestWebContents::Create(browser_context(), instance)); - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); - - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); - - RenderViewHost* host = manager.current_host(); - ASSERT_TRUE(host); - EXPECT_EQ(instance, host->GetSiteInstance()); - EXPECT_EQ(web_contents.get(), host->GetDelegate()); - EXPECT_TRUE(manager.GetRenderWidgetHostView()); - EXPECT_FALSE(manager.pending_render_view_host()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); + + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); + + RenderViewHostImpl* rvh = manager->current_host(); + RenderFrameHostImpl* rfh = manager->current_frame_host(); + ASSERT_TRUE(rvh); + ASSERT_TRUE(rfh); + EXPECT_EQ(rvh, rfh->render_view_host()); + EXPECT_EQ(instance, rvh->GetSiteInstance()); + EXPECT_EQ(web_contents.get(), rvh->GetDelegate()); + EXPECT_EQ(web_contents.get(), rfh->delegate()); + EXPECT_TRUE(manager->GetRenderWidgetHostView()); + EXPECT_FALSE(manager->pending_render_view_host()); } // Tests the Navigate function. We navigate three sites consecutively and check @@ -593,10 +600,13 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { Source<WebContents>(web_contents.get())); // Create. - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); RenderViewHost* host; @@ -606,16 +616,16 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { NULL /* instance */, -1 /* page_id */, kUrl1, Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - host = manager.Navigate(entry1); + host = manager->Navigate(entry1); // The RenderViewHost created in Init will be reused. - EXPECT_TRUE(host == manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_TRUE(host == manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // Commit. - manager.DidNavigateMainFrame(host); + manager->DidNavigateMainFrame(host); // Commit to SiteInstance should be delayed until RenderView commit. - EXPECT_TRUE(host == manager.current_host()); + EXPECT_TRUE(host == manager->current_host()); ASSERT_TRUE(host); EXPECT_FALSE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); @@ -628,15 +638,15 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { Referrer(kUrl1, blink::WebReferrerPolicyDefault), base::string16() /* title */, PAGE_TRANSITION_LINK, true /* is_renderer_init */); - host = manager.Navigate(entry2); + host = manager->Navigate(entry2); // The RenderViewHost created in Init will be reused. - EXPECT_TRUE(host == manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_TRUE(host == manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // Commit. - manager.DidNavigateMainFrame(host); - EXPECT_TRUE(host == manager.current_host()); + manager->DidNavigateMainFrame(host); + EXPECT_TRUE(host == manager->current_host()); ASSERT_TRUE(host); EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); @@ -648,22 +658,22 @@ TEST_F(RenderFrameHostManagerTest, Navigate) { Referrer(kUrl2, blink::WebReferrerPolicyDefault), base::string16() /* title */, PAGE_TRANSITION_LINK, false /* is_renderer_init */); - host = manager.Navigate(entry3); + host = manager->Navigate(entry3); // A new RenderViewHost should be created. - EXPECT_TRUE(manager.pending_render_view_host()); - ASSERT_EQ(host, manager.pending_render_view_host()); + EXPECT_TRUE(manager->pending_render_view_host()); + ASSERT_EQ(host, manager->pending_render_view_host()); notifications.Reset(); // Commit. - manager.DidNavigateMainFrame(manager.pending_render_view_host()); - EXPECT_TRUE(host == manager.current_host()); + manager->DidNavigateMainFrame(manager->pending_render_view_host()); + EXPECT_TRUE(host == manager->current_host()); ASSERT_TRUE(host); EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); // Check the pending RenderViewHost has been committed. - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // We should observe a notification. EXPECT_TRUE( @@ -685,10 +695,13 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { Source<WebContents>(web_contents.get())); // Create. - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); // 1) The first navigation. -------------------------- const GURL kUrl1("http://www.google.com/"); @@ -696,11 +709,11 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - RenderViewHost* host = manager.Navigate(entry1); + RenderViewHost* host = manager->Navigate(entry1); // The RenderViewHost created in Init will be reused. - EXPECT_TRUE(host == manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_TRUE(host == manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // We should observe a notification. EXPECT_TRUE( @@ -708,10 +721,10 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { notifications.Reset(); // Commit. - manager.DidNavigateMainFrame(host); + manager->DidNavigateMainFrame(host); // Commit to SiteInstance should be delayed until RenderView commit. - EXPECT_TRUE(host == manager.current_host()); + EXPECT_TRUE(host == manager->current_host()); ASSERT_TRUE(host); EXPECT_FALSE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); @@ -724,12 +737,12 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); RenderViewHostImpl* host2 = static_cast<RenderViewHostImpl*>( - manager.Navigate(entry2)); + manager->Navigate(entry2)); int host2_process_id = host2->GetProcess()->GetID(); // A new RenderViewHost should be created. - EXPECT_TRUE(manager.pending_render_view_host()); - ASSERT_EQ(host2, manager.pending_render_view_host()); + EXPECT_TRUE(manager->pending_render_view_host()); + ASSERT_EQ(host2, manager->pending_render_view_host()); EXPECT_NE(host2, host); // Check that the navigation is still suspended because the old RVH @@ -756,15 +769,15 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { // call of RenderFrameHostManager::SwapOutOldPage before // RenderFrameHostManager::DidNavigateMainFrame is called. // The RVH is not swapped out until the commit. - manager.SwapOutOldPage(); + manager->SwapOutOldPage(); EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching( ViewMsg_SwapOut::ID)); test_host->OnSwappedOut(false); - EXPECT_EQ(host, manager.current_host()); + EXPECT_EQ(host, manager->current_host()); EXPECT_FALSE(static_cast<RenderViewHostImpl*>( - manager.current_host())->is_swapped_out()); - EXPECT_EQ(host2, manager.pending_render_view_host()); + manager->current_host())->is_swapped_out()); + EXPECT_EQ(host2, manager->pending_render_view_host()); // There should be still no navigation messages being sent. EXPECT_FALSE(test_process_host2->sink().GetUniqueMessageMatching( ViewMsg_Navigate::ID)); @@ -776,11 +789,11 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { PAGE_TRANSITION_TYPED, false /* is_renderer_init */); test_process_host->sink().ClearMessages(); - RenderViewHost* host3 = manager.Navigate(entry3); + RenderViewHost* host3 = manager->Navigate(entry3); // A new RenderViewHost should be created. host2 is now deleted. - EXPECT_TRUE(manager.pending_render_view_host()); - ASSERT_EQ(host3, manager.pending_render_view_host()); + EXPECT_TRUE(manager->pending_render_view_host()); + ASSERT_EQ(host3, manager->pending_render_view_host()); EXPECT_NE(host3, host); EXPECT_NE(host3->GetProcess()->GetID(), host2_process_id); @@ -789,9 +802,9 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { // request. EXPECT_TRUE(static_cast<RenderViewHostImpl*>( host3)->are_navigations_suspended()); - EXPECT_EQ(host, manager.current_host()); + EXPECT_EQ(host, manager->current_host()); EXPECT_FALSE(static_cast<RenderViewHostImpl*>( - manager.current_host())->is_swapped_out()); + manager->current_host())->is_swapped_out()); // Simulate a response to the second beforeunload request. EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching( @@ -802,19 +815,19 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) { // call of RenderFrameHostManager::SwapOutOldPage before // RenderFrameHostManager::DidNavigateMainFrame is called. // The RVH is not swapped out until the commit. - manager.SwapOutOldPage(); + manager->SwapOutOldPage(); EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching( ViewMsg_SwapOut::ID)); test_host->OnSwappedOut(false); // Commit. - manager.DidNavigateMainFrame(host3); - EXPECT_TRUE(host3 == manager.current_host()); + manager->DidNavigateMainFrame(host3); + EXPECT_TRUE(host3 == manager->current_host()); ASSERT_TRUE(host3); EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host3->GetSiteInstance())-> HasSite()); // Check the pending RenderViewHost has been committed. - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // We should observe a notification. EXPECT_TRUE( @@ -828,25 +841,28 @@ TEST_F(RenderFrameHostManagerTest, WebUI) { scoped_ptr<TestWebContents> web_contents( TestWebContents::Create(browser_context(), instance)); - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); - EXPECT_FALSE(manager.current_host()->IsRenderViewLive()); + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); + EXPECT_FALSE(manager->current_host()->IsRenderViewLive()); const GURL kUrl("chrome://foo"); NavigationEntryImpl entry(NULL /* instance */, -1 /* page_id */, kUrl, Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - RenderViewHost* host = manager.Navigate(entry); + RenderViewHost* host = manager->Navigate(entry); // We commit the pending RenderViewHost immediately because the previous // RenderViewHost was not live. We test a case where it is live in // WebUIInNewTab. EXPECT_TRUE(host); - EXPECT_EQ(host, manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_EQ(host, manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // It's important that the site instance get set on the Web UI page as soon // as the navigation starts, rather than lazily after it commits, so we don't @@ -858,11 +874,11 @@ TEST_F(RenderFrameHostManagerTest, WebUI) { // The Web UI is committed immediately because the RenderViewHost has not been // used yet. UpdateRendererStateForNavigate() took the short cut path. - EXPECT_FALSE(manager.pending_web_ui()); - EXPECT_TRUE(manager.web_ui()); + EXPECT_FALSE(manager->pending_web_ui()); + EXPECT_TRUE(manager->web_ui()); // Commit. - manager.DidNavigateMainFrame(host); + manager->DidNavigateMainFrame(host); EXPECT_TRUE(host->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI); } @@ -875,12 +891,14 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) { // Create a blank tab. scoped_ptr<TestWebContents> web_contents1( TestWebContents::Create(browser_context(), blank_instance)); - RenderFrameHostManager manager1(web_contents1.get(), web_contents1.get(), - web_contents1.get(), web_contents1.get()); - manager1.Init( + FrameTree tree1(web_contents1->GetFrameTree()->root()->navigator(), + web_contents1.get(), web_contents1.get(), + web_contents1.get(), web_contents1.get()); + RenderFrameHostManager* manager1 = tree1.root()->render_manager(); + manager1->Init( browser_context(), blank_instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); // Test the case that new RVH is considered live. - manager1.current_host()->CreateRenderView(base::string16(), -1, -1); + manager1->current_host()->CreateRenderView(base::string16(), -1, -1); // Navigate to a WebUI page. const GURL kUrl1("chrome://foo"); @@ -888,44 +906,46 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) { Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - RenderViewHost* host1 = manager1.Navigate(entry1); + RenderViewHost* host1 = manager1->Navigate(entry1); // We should have a pending navigation to the WebUI RenderViewHost. // It should already have bindings. - EXPECT_EQ(host1, manager1.pending_render_view_host()); - EXPECT_NE(host1, manager1.current_host()); + EXPECT_EQ(host1, manager1->pending_render_view_host()); + EXPECT_NE(host1, manager1->current_host()); EXPECT_TRUE(host1->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI); // Commit and ensure we still have bindings. - manager1.DidNavigateMainFrame(host1); + manager1->DidNavigateMainFrame(host1); SiteInstance* webui_instance = host1->GetSiteInstance(); - EXPECT_EQ(host1, manager1.current_host()); + EXPECT_EQ(host1, manager1->current_host()); EXPECT_TRUE(host1->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI); // Now simulate clicking a link that opens in a new tab. scoped_ptr<TestWebContents> web_contents2( TestWebContents::Create(browser_context(), webui_instance)); - RenderFrameHostManager manager2(web_contents2.get(), web_contents2.get(), - web_contents2.get(), web_contents2.get()); - manager2.Init( + FrameTree tree2(web_contents2->GetFrameTree()->root()->navigator(), + web_contents2.get(), web_contents2.get(), + web_contents2.get(), web_contents2.get()); + RenderFrameHostManager* manager2 = tree2.root()->render_manager(); + manager2->Init( browser_context(), webui_instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); // Make sure the new RVH is considered live. This is usually done in // RenderWidgetHost::Init when opening a new tab from a link. - manager2.current_host()->CreateRenderView(base::string16(), -1, -1); + manager2->current_host()->CreateRenderView(base::string16(), -1, -1); const GURL kUrl2("chrome://foo/bar"); NavigationEntryImpl entry2(NULL /* instance */, -1 /* page_id */, kUrl2, Referrer(), base::string16() /* title */, PAGE_TRANSITION_LINK, true /* is_renderer_init */); - RenderViewHost* host2 = manager2.Navigate(entry2); + RenderViewHost* host2 = manager2->Navigate(entry2); // No cross-process transition happens because we are already in the right // SiteInstance. We should grant bindings immediately. - EXPECT_EQ(host2, manager2.current_host()); + EXPECT_EQ(host2, manager2->current_host()); EXPECT_TRUE(host2->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI); - manager2.DidNavigateMainFrame(host2); + manager2->DidNavigateMainFrame(host2); } // Tests that we don't end up in an inconsistent state if a page does a back and @@ -1058,20 +1078,20 @@ TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) { rvh2->GetSiteInstance())); // Ensure rvh1 is placed on swapped out list of the current tab. - EXPECT_TRUE(manager->IsOnSwappedOutList(rvh1)); + EXPECT_TRUE(manager->IsRVHOnSwappedOutList(rvh1)); EXPECT_EQ(rvh1, manager->GetSwappedOutRenderViewHost(rvh1->GetSiteInstance())); // Ensure a swapped out RVH is created in the first opener tab. TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>( opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); - EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rvh)); + EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh)); EXPECT_TRUE(opener1_rvh->is_swapped_out()); // Ensure a swapped out RVH is created in the second opener tab. TestRenderViewHost* opener2_rvh = static_cast<TestRenderViewHost*>( opener2_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); - EXPECT_TRUE(opener2_manager->IsOnSwappedOutList(opener2_rvh)); + EXPECT_TRUE(opener2_manager->IsRVHOnSwappedOutList(opener2_rvh)); EXPECT_TRUE(opener2_rvh->is_swapped_out()); // Navigate to a cross-BrowsingInstance URL. @@ -1173,7 +1193,7 @@ TEST_F(RenderFrameHostManagerTest, EnableWebUIWithSwappedOutOpener) { // Ensure a swapped out RVH is created in the first opener tab. TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>( opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance())); - EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rvh)); + EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh)); EXPECT_TRUE(opener1_rvh->is_swapped_out()); // Ensure the new RVH has WebUI bindings. @@ -1191,10 +1211,13 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { TestWebContents::Create(browser_context(), instance)); // Create. - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); RenderViewHost* host; @@ -1204,17 +1227,17 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { NULL /* instance */, -1 /* page_id */, kUrl1, Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - host = manager.Navigate(entry1); + host = manager->Navigate(entry1); // The RenderViewHost created in Init will be reused. - EXPECT_TRUE(host == manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); - EXPECT_EQ(manager.current_host()->GetSiteInstance(), instance); + EXPECT_TRUE(host == manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); + EXPECT_EQ(manager->current_host()->GetSiteInstance(), instance); // Commit. - manager.DidNavigateMainFrame(host); + manager->DidNavigateMainFrame(host); // Commit to SiteInstance should be delayed until RenderView commit. - EXPECT_EQ(host, manager.current_host()); + EXPECT_EQ(host, manager->current_host()); ASSERT_TRUE(host); EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); @@ -1227,15 +1250,15 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) { Referrer(kUrl1, blink::WebReferrerPolicyDefault), base::string16() /* title */, PAGE_TRANSITION_LINK, true /* is_renderer_init */); - host = manager.Navigate(entry2); + host = manager->Navigate(entry2); // The RenderViewHost created in Init will be reused. - EXPECT_EQ(host, manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_EQ(host, manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // Commit. - manager.DidNavigateMainFrame(host); - EXPECT_EQ(host, manager.current_host()); + manager->DidNavigateMainFrame(host); + EXPECT_EQ(host, manager->current_host()); ASSERT_TRUE(host); EXPECT_EQ(static_cast<SiteInstanceImpl*>(host->GetSiteInstance()), instance); @@ -1256,10 +1279,13 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) { Source<WebContents>(web_contents.get())); // Create. - RenderFrameHostManager manager(web_contents.get(), web_contents.get(), - web_contents.get(), web_contents.get()); + FrameTree tree(web_contents->GetFrameTree()->root()->navigator(), + web_contents.get(), web_contents.get(), + web_contents.get(), web_contents.get()); + RenderFrameHostManager* manager = tree.root()->render_manager(); - manager.Init(browser_context(), instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE); + manager->Init(browser_context(), instance, MSG_ROUTING_NONE, + MSG_ROUTING_NONE); // 1) The first navigation. -------------------------- const GURL kUrl1("http://www.google.com/"); @@ -1267,11 +1293,11 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) { Referrer(), base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); - RenderViewHost* host = manager.Navigate(entry1); + RenderViewHost* host = manager->Navigate(entry1); // The RenderViewHost created in Init will be reused. - EXPECT_EQ(host, manager.current_host()); - EXPECT_FALSE(manager.pending_render_view_host()); + EXPECT_EQ(host, manager->current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); // We should observe a notification. EXPECT_TRUE( @@ -1279,10 +1305,10 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) { notifications.Reset(); // Commit. - manager.DidNavigateMainFrame(host); + manager->DidNavigateMainFrame(host); // Commit to SiteInstance should be delayed until RenderView commit. - EXPECT_EQ(host, manager.current_host()); + EXPECT_EQ(host, manager->current_host()); EXPECT_FALSE(static_cast<SiteInstanceImpl*>(host->GetSiteInstance())-> HasSite()); static_cast<SiteInstanceImpl*>(host->GetSiteInstance())->SetSite(kUrl1); @@ -1294,26 +1320,26 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) { base::string16() /* title */, PAGE_TRANSITION_TYPED, false /* is_renderer_init */); RenderViewHostImpl* host2 = static_cast<RenderViewHostImpl*>( - manager.Navigate(entry2)); + manager->Navigate(entry2)); // A new RenderViewHost should be created. - ASSERT_EQ(host2, manager.pending_render_view_host()); + ASSERT_EQ(host2, manager->pending_render_view_host()); EXPECT_NE(host2, host); - EXPECT_EQ(host, manager.current_host()); + EXPECT_EQ(host, manager->current_host()); EXPECT_FALSE(static_cast<RenderViewHostImpl*>( - manager.current_host())->is_swapped_out()); - EXPECT_EQ(host2, manager.pending_render_view_host()); + manager->current_host())->is_swapped_out()); + EXPECT_EQ(host2, manager->pending_render_view_host()); // 3) Close the tab. ------------------------- notifications.ListenFor(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, Source<RenderWidgetHost>(host2)); - manager.ShouldClosePage(false, true, base::TimeTicks()); + manager->ShouldClosePage(false, true, base::TimeTicks()); EXPECT_TRUE( notifications.Check1AndReset(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED)); - EXPECT_FALSE(manager.pending_render_view_host()); - EXPECT_EQ(host, manager.current_host()); + EXPECT_FALSE(manager->pending_render_view_host()); + EXPECT_EQ(host, manager->current_host()); } } // namespace content diff --git a/content/browser/renderer_host/DEPS b/content/browser/renderer_host/DEPS index 1d612c4..2696801 100644 --- a/content/browser/renderer_host/DEPS +++ b/content/browser/renderer_host/DEPS @@ -33,7 +33,5 @@ specific_include_rules = { # of RenderViewHost on the FrameTree. "render_view_host_impl\.(cc|h)": [ "+content/browser/frame_host/frame_tree.h", - "+content/browser/frame_host/render_frame_host_factory.h", - "+content/browser/frame_host/render_frame_host_impl.h", ], } diff --git a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc index db766cd..6abb183 100644 --- a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc +++ b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc @@ -228,14 +228,13 @@ class CaptureTestRenderViewHost : public TestRenderViewHost { public: CaptureTestRenderViewHost(SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, bool swapped_out, CaptureTestSourceController* controller) - : TestRenderViewHost(instance, delegate, frame_delegate, widget_delegate, - routing_id, main_frame_routing_id, swapped_out), + : TestRenderViewHost(instance, delegate, widget_delegate, routing_id, + main_frame_routing_id, swapped_out), controller_(controller) { // Override the default view installed by TestRenderViewHost; we need // our special subclass which has mocked-out tab capture support. @@ -290,15 +289,13 @@ class CaptureTestRenderViewHostFactory : public RenderViewHostFactory { virtual RenderViewHost* CreateRenderViewHost( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, bool swapped_out) OVERRIDE { - return new CaptureTestRenderViewHost(instance, delegate, frame_delegate, - widget_delegate, routing_id, - main_frame_routing_id, swapped_out, - controller_); + return new CaptureTestRenderViewHost(instance, delegate, widget_delegate, + routing_id, main_frame_routing_id, + swapped_out, controller_); } private: CaptureTestSourceController* controller_; diff --git a/content/browser/renderer_host/render_view_host_browsertest.cc b/content/browser/renderer_host/render_view_host_browsertest.cc index 386ff52..555852c 100644 --- a/content/browser/renderer_host/render_view_host_browsertest.cc +++ b/content/browser/renderer_host/render_view_host_browsertest.cc @@ -6,6 +6,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "base/values.h" +#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_view_host_impl.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/common/view_messages.h" @@ -100,19 +101,19 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostTest, BasicRenderFrameHost) { GURL test_url = embedded_test_server()->GetURL("/simple_page.html"); NavigateToURL(shell(), test_url); - RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>( - shell()->web_contents()->GetRenderViewHost()); - EXPECT_TRUE(rvh->main_render_frame_host()); + FrameTreeNode* old_root = static_cast<WebContentsImpl*>( + shell()->web_contents())->GetFrameTree()->root(); + EXPECT_TRUE(old_root->current_frame_host()); ShellAddedObserver new_shell_observer; EXPECT_TRUE(ExecuteScript(shell()->web_contents(), "window.open();")); Shell* new_shell = new_shell_observer.GetShell(); - RenderViewHostImpl* new_rvh = static_cast<RenderViewHostImpl*>( - new_shell->web_contents()->GetRenderViewHost()); + FrameTreeNode* new_root = static_cast<WebContentsImpl*>( + new_shell->web_contents())->GetFrameTree()->root(); - EXPECT_TRUE(new_rvh->main_render_frame_host()); - EXPECT_NE(rvh->main_render_frame_host()->routing_id(), - new_rvh->main_render_frame_host()->routing_id()); + EXPECT_TRUE(new_root->current_frame_host()); + EXPECT_NE(old_root->current_frame_host()->routing_id(), + new_root->current_frame_host()->routing_id()); } } // namespace content diff --git a/content/browser/renderer_host/render_view_host_factory.cc b/content/browser/renderer_host/render_view_host_factory.cc index fe51c7e..5cffd13 100644 --- a/content/browser/renderer_host/render_view_host_factory.cc +++ b/content/browser/renderer_host/render_view_host_factory.cc @@ -16,19 +16,17 @@ RenderViewHostFactory* RenderViewHostFactory::factory_ = NULL; RenderViewHost* RenderViewHostFactory::Create( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, bool swapped_out, bool hidden) { if (factory_) { - return factory_->CreateRenderViewHost(instance, delegate, frame_delegate, - widget_delegate, routing_id, - main_frame_routing_id, swapped_out); + return factory_->CreateRenderViewHost(instance, delegate, widget_delegate, + routing_id, main_frame_routing_id, + swapped_out); } - return new RenderViewHostImpl(instance, delegate, frame_delegate, - widget_delegate, routing_id, + return new RenderViewHostImpl(instance, delegate, widget_delegate, routing_id, main_frame_routing_id, swapped_out, hidden); } diff --git a/content/browser/renderer_host/render_view_host_factory.h b/content/browser/renderer_host/render_view_host_factory.h index cf6a0f1..734f61f 100644 --- a/content/browser/renderer_host/render_view_host_factory.h +++ b/content/browser/renderer_host/render_view_host_factory.h @@ -27,7 +27,6 @@ class RenderViewHostFactory { static RenderViewHost* Create( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, @@ -48,7 +47,6 @@ class RenderViewHostFactory { virtual RenderViewHost* CreateRenderViewHost( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc index b19331f..7577b59 100644 --- a/content/browser/renderer_host/render_view_host_impl.cc +++ b/content/browser/renderer_host/render_view_host_impl.cc @@ -27,7 +27,6 @@ #include "content/browser/cross_site_request_manager.h" #include "content/browser/dom_storage/session_storage_namespace_impl.h" #include "content/browser/frame_host/frame_tree.h" -#include "content/browser/frame_host/render_frame_host_factory.h" #include "content/browser/gpu/compositor_util.h" #include "content/browser/gpu/gpu_data_manager_impl.h" #include "content/browser/gpu/gpu_process_host.h" @@ -182,7 +181,6 @@ RenderViewHostImpl* RenderViewHostImpl::FromID(int render_process_id, RenderViewHostImpl::RenderViewHostImpl( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, @@ -201,6 +199,7 @@ RenderViewHostImpl::RenderViewHostImpl( is_swapped_out_(swapped_out), is_subframe_(false), main_frame_id_(-1), + main_frame_routing_id_(main_frame_routing_id), run_modal_reply_msg_(NULL), run_modal_opener_id_(MSG_ROUTING_NONE), is_waiting_for_beforeunload_ack_(false), @@ -214,16 +213,6 @@ RenderViewHostImpl::RenderViewHostImpl( DCHECK(instance_.get()); CHECK(delegate_); // http://crbug.com/82827 - if (main_frame_routing_id == MSG_ROUTING_NONE) - main_frame_routing_id = GetProcess()->GetNextRoutingID(); - - main_render_frame_host_ = RenderFrameHostFactory::Create( - this, frame_delegate, delegate_->GetFrameTree(), - delegate_->GetFrameTree()->root(), - main_frame_routing_id, is_swapped_out_); - delegate_->GetFrameTree()->root()->set_render_frame_host( - main_render_frame_host_.get(), false); - GetProcess()->EnableSendQueue(); if (!swapped_out) @@ -303,7 +292,7 @@ bool RenderViewHostImpl::CreateRenderView( delegate_->GetRendererPrefs(GetProcess()->GetBrowserContext()); params.web_preferences = delegate_->GetWebkitPrefs(); params.view_id = GetRoutingID(); - params.main_frame_routing_id = main_render_frame_host()->routing_id(); + params.main_frame_routing_id = main_frame_routing_id_; params.surface_id = surface_id(); params.session_storage_namespace_id = delegate_->GetSessionStorageNamespace(instance_)->id(); @@ -1314,7 +1303,6 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) { void RenderViewHostImpl::Init() { RenderWidgetHostImpl::Init(); - main_render_frame_host()->Init(); } void RenderViewHostImpl::Shutdown() { @@ -1428,7 +1416,7 @@ void RenderViewHostImpl::OnRenderProcessGone(int status, int exit_code) { // 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()->SwapMainFrame(main_render_frame_host_.get()); + delegate_->GetFrameTree()->ResetForMainFrameSwap(); // Our base class RenderWidgetHost needs to reset some stuff. RendererExited(render_view_termination_status_, exit_code); @@ -2333,7 +2321,7 @@ bool RenderViewHostImpl::CanAccessFilesOfPageState( void RenderViewHostImpl::AttachToFrameTree() { FrameTree* frame_tree = delegate_->GetFrameTree(); - frame_tree->SwapMainFrame(main_render_frame_host_.get()); + frame_tree->ResetForMainFrameSwap(); if (main_frame_id() != FrameTreeNode::kInvalidFrameId) { frame_tree->OnFirstNavigationAfterSwap(main_frame_id()); } diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h index 13ff167..18c643f 100644 --- a/content/browser/renderer_host/render_view_host_impl.h +++ b/content/browser/renderer_host/render_view_host_impl.h @@ -13,7 +13,6 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/process/kill.h" -#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_widget_host_impl.h" #include "content/browser/site_instance_impl.h" #include "content/common/accessibility_node_data.h" @@ -61,8 +60,6 @@ namespace content { class BrowserMediaPlayerManager; class ChildProcessSecurityPolicyImpl; class PageState; -class RenderFrameHostDelegate; -class RenderFrameHostImpl; class RenderWidgetHostDelegate; class SessionStorageNamespace; class SessionStorageNamespaceImpl; @@ -119,7 +116,6 @@ class CONTENT_EXPORT RenderViewHostImpl RenderViewHostImpl( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, @@ -617,12 +613,6 @@ class CONTENT_EXPORT RenderViewHostImpl void OnShowPopup(const ViewHostMsg_ShowPopup_Params& params); #endif - // TODO(nasko): Remove this accessor once RenderFrameHost moves into the frame - // tree. - RenderFrameHostImpl* main_render_frame_host() const { - return main_render_frame_host_.get(); - } - private: friend class TestRenderViewHost; FRIEND_TEST_ALL_PREFIXES(RenderViewHostTest, BasicRenderFrameHost); @@ -634,16 +624,6 @@ class CONTENT_EXPORT RenderViewHostImpl bool CanAccessFilesOfPageState(const PageState& state) const; - // All RenderViewHosts must have a RenderFrameHost for its main frame. - // Currently the RenderFrameHost is created in lock step on construction - // and a pointer to the main frame is given to the FrameTreeNode - // when the RenderViewHost commits (see AttachToFrameTree()). - // - // TODO(ajwong): Make this reference non-owning. The root FrameTreeNode of - // the FrameTree should be responsible for owning the main frame's - // RenderFrameHost. - scoped_ptr<RenderFrameHostImpl> main_render_frame_host_; - // Our delegate, which wants to know about changes in the RenderView. RenderViewHostDelegate* delegate_; @@ -689,8 +669,12 @@ class CONTENT_EXPORT RenderViewHostImpl // The frame id of the main (top level) frame. This value is set on the // initial navigation of a RenderView and reset when the RenderView's // process is terminated (in RenderProcessGone). + // TODO(creis): Remove this when we switch to routing IDs for frames. int64 main_frame_id_; + // Routing ID for the main frame's RenderFrameHost. + int main_frame_routing_id_; + // If we were asked to RunModal, then this will hold the reply_msg that we // must return to the renderer to unblock it. IPC::Message* run_modal_reply_msg_; diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc index e81e67f..0b56d16 100644 --- a/content/browser/renderer_host/render_view_host_unittest.cc +++ b/content/browser/renderer_host/render_view_host_unittest.cc @@ -5,6 +5,7 @@ #include "base/path_service.h" #include "base/strings/utf_string_conversions.h" #include "content/browser/child_process_security_policy_impl.h" +#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/common/input_messages.h" #include "content/common/view_messages.h" #include "content/port/browser/render_view_host_delegate_view.h" @@ -288,10 +289,10 @@ TEST_F(RenderViewHostTest, NavigationWithBadHistoryItemFiles) { } TEST_F(RenderViewHostTest, RoutingIdSane) { - EXPECT_EQ(test_rvh()->GetProcess(), - test_rvh()->main_render_frame_host()->GetProcess()); - EXPECT_NE(test_rvh()->GetRoutingID(), - test_rvh()->main_render_frame_host()->routing_id()); + RenderFrameHostImpl* root_rfh = + contents()->GetFrameTree()->root()->current_frame_host(); + EXPECT_EQ(test_rvh()->GetProcess(), root_rfh->GetProcess()); + EXPECT_NE(test_rvh()->GetRoutingID(), root_rfh->routing_id()); } } // namespace content diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 474dac8..4d2b3b2 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -282,7 +282,7 @@ bool CollectSites(BrowserContext* context, bool ForEachFrameInternal( const base::Callback<void(RenderFrameHost*)>& on_frame, FrameTreeNode* node) { - on_frame.Run(node->render_frame_host()); + on_frame.Run(node->current_frame_host()); return true; } @@ -651,7 +651,7 @@ RenderProcessHost* WebContentsImpl::GetRenderProcessHost() const { } RenderFrameHost* WebContentsImpl::GetMainFrame() { - return frame_tree_.root()->render_frame_host(); + return frame_tree_.root()->current_frame_host(); } void WebContentsImpl::ForEachFrame( @@ -3062,8 +3062,13 @@ void WebContentsImpl::UpdateState(RenderViewHost* rvh, const PageState& page_state) { // Ensure that this state update comes from either the active RVH or one of // the swapped out RVHs. We don't expect to hear from any other RVHs. - DCHECK(rvh == GetRenderViewHost() || - GetRenderManager()->IsOnSwappedOutList(rvh)); + // TODO(nasko): This should go through RenderFrameHost. + // TODO(creis): We can't update state for cross-process subframes until we + // have FrameNavigationEntries. Once we do, this should be a DCHECK. + if (rvh != GetRenderViewHost() && + !GetRenderManager()->IsRVHOnSwappedOutList( + static_cast<RenderViewHostImpl*>(rvh))) + return; // We must be prepared to handle state updates for any page, these occur // when the user is scrolling and entering form data, as well as when we're @@ -3490,8 +3495,8 @@ WebPreferences WebContentsImpl::GetWebkitPrefs() { int WebContentsImpl::CreateSwappedOutRenderView( SiteInstance* instance) { - return GetRenderManager()->CreateRenderView(instance, MSG_ROUTING_NONE, - true, true); + return GetRenderManager()->CreateRenderFrame(instance, MSG_ROUTING_NONE, + true, true); } void WebContentsImpl::OnUserGesture() { @@ -3660,8 +3665,8 @@ int WebContentsImpl::CreateOpenerRenderViews(SiteInstance* instance) { // Create a swapped out RenderView in the given SiteInstance if none exists, // setting its opener to the given route_id. Return the new view's route_id. - return GetRenderManager()->CreateRenderView(instance, opener_route_id, - true, true); + return GetRenderManager()->CreateRenderFrame(instance, opener_route_id, + true, true); } NavigationControllerImpl& WebContentsImpl::GetControllerForRenderManager() { diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index 72aa6a0..c9654e2 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -423,6 +423,8 @@ TEST_F(WebContentsImplTest, NavigateToExcessivelyLongURL) { TEST_F(WebContentsImplTest, CrossSiteBoundaries) { contents()->transition_cross_site = true; TestRenderViewHost* orig_rvh = test_rvh(); + RenderFrameHostImpl* orig_rfh = + contents()->GetFrameTree()->root()->current_frame_host(); int orig_rvh_delete_count = 0; orig_rvh->set_delete_counter(&orig_rvh_delete_count); SiteInstance* instance1 = contents()->GetSiteInstance(); @@ -455,6 +457,8 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost()); int pending_rvh_delete_count = 0; pending_rvh->set_delete_counter(&pending_rvh_delete_count); + RenderFrameHostImpl* pending_rfh = contents()->GetFrameTree()->root()-> + render_manager()->pending_frame_host(); // Navigations should be suspended in pending_rvh until ShouldCloseACK. EXPECT_TRUE(pending_rvh->are_navigations_suspended()); @@ -478,9 +482,9 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { EXPECT_EQ(url2, contents()->GetVisibleURL()); EXPECT_NE(instance1, instance2); EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL); - // We keep the original RVH around, swapped out. + // We keep the original RFH around, swapped out. EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList( - orig_rvh)); + orig_rfh)); EXPECT_EQ(orig_rvh_delete_count, 0); // Going back should switch SiteInstances again. The first SiteInstance is @@ -503,9 +507,9 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) { EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(goback_rvh, contents()->GetRenderViewHost()); EXPECT_EQ(instance1, contents()->GetSiteInstance()); - // The pending RVH should now be swapped out, not deleted. + // The pending RFH should now be swapped out, not deleted. EXPECT_TRUE(contents()->GetRenderManagerForTesting()-> - IsOnSwappedOutList(pending_rvh)); + IsOnSwappedOutList(pending_rfh)); EXPECT_EQ(pending_rvh_delete_count, 0); // Close contents and ensure RVHs are deleted. @@ -628,6 +632,8 @@ TEST_F(WebContentsImplTest, NavigateDoesNotUseUpSiteInstance) { contents()->transition_cross_site = true; TestRenderViewHost* orig_rvh = test_rvh(); + RenderFrameHostImpl* orig_rfh = + contents()->GetFrameTree()->root()->current_frame_host(); int orig_rvh_delete_count = 0; orig_rvh->set_delete_counter(&orig_rvh_delete_count); SiteInstanceImpl* orig_instance = @@ -698,9 +704,9 @@ TEST_F(WebContentsImplTest, NavigateDoesNotUseUpSiteInstance) { EXPECT_EQ(url2, contents()->GetVisibleURL()); EXPECT_NE(new_instance, orig_instance); EXPECT_FALSE(contents()->GetPendingRenderViewHost()); - // We keep the original RVH around, swapped out. + // We keep the original RFH around, swapped out. EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList( - orig_rvh)); + orig_rfh)); EXPECT_EQ(orig_rvh_delete_count, 0); // Close contents and ensure RVHs are deleted. diff --git a/content/renderer/dom_serializer_browsertest.cc b/content/renderer/dom_serializer_browsertest.cc index 16bd30f..738bcd9 100644 --- a/content/renderer/dom_serializer_browsertest.cc +++ b/content/renderer/dom_serializer_browsertest.cc @@ -51,6 +51,13 @@ using blink::WebURL; using blink::WebView; using blink::WebVector; +namespace { + +// The first RenderFrame is routing ID 1, and the first RenderView is 2. +const int kRenderViewRoutingId = 2; + +} + namespace content { // Iterate recursively over sub-frames to find one with with a given url. @@ -223,7 +230,7 @@ class DomSerializerTests : public ContentBrowserTest, RenderView* GetRenderView() { // We could have the test on the UI thread get the WebContent's routing ID, // but we know this will be the first RV so skip that and just hardcode it. - return RenderView::FromRoutingID(1); + return RenderView::FromRoutingID(kRenderViewRoutingId); } WebView* GetWebView() { diff --git a/content/renderer/resource_fetcher_browsertest.cc b/content/renderer/resource_fetcher_browsertest.cc index a9498cb..a1f1593 100644 --- a/content/renderer/resource_fetcher_browsertest.cc +++ b/content/renderer/resource_fetcher_browsertest.cc @@ -25,6 +25,13 @@ using blink::WebFrame; using blink::WebURLRequest; using blink::WebURLResponse; +namespace { + +// The first RenderFrame is routing ID 1, and the first RenderView is 2. +const int kRenderViewRoutingId = 2; + +} + namespace content { static const int kMaxWaitTimeMs = 5000; @@ -135,7 +142,7 @@ class ResourceFetcherTests : public ContentBrowserTest { RenderView* GetRenderView() { // We could have the test on the UI thread get the WebContent's routing ID, // but we know this will be the first RV so skip that and just hardcode it. - return RenderView::FromRoutingID(1); + return RenderView::FromRoutingID(kRenderViewRoutingId); } void ResourceFetcherDownloadOnRenderer(const GURL& url) { diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc index 57f8c0f..e9a3bc8 100644 --- a/content/test/test_render_frame_host.cc +++ b/content/test/test_render_frame_host.cc @@ -4,6 +4,9 @@ #include "content/test/test_render_frame_host.h" +#include "content/browser/frame_host/frame_tree.h" +#include "content/test/test_render_view_host.h" + namespace content { TestRenderFrameHost::TestRenderFrameHost(RenderViewHostImpl* render_view_host, @@ -17,7 +20,13 @@ TestRenderFrameHost::TestRenderFrameHost(RenderViewHostImpl* render_view_host, frame_tree, frame_tree_node, routing_id, - is_swapped_out) {} + is_swapped_out) { + // Allow TestRenderViewHosts to easily access their main frame RFH. + if (frame_tree_node == frame_tree->root()) { + static_cast<TestRenderViewHost*>(render_view_host)-> + set_main_render_frame_host(this); + } +} TestRenderFrameHost::~TestRenderFrameHost() {} diff --git a/content/test/test_render_view_host.cc b/content/test/test_render_view_host.cc index 5c13673..0754316 100644 --- a/content/test/test_render_view_host.cc +++ b/content/test/test_render_view_host.cc @@ -235,14 +235,12 @@ gfx::NativeViewId TestRenderWidgetHostView::GetParentForWindowlessPlugin() TestRenderViewHost::TestRenderViewHost( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, bool swapped_out) : RenderViewHostImpl(instance, delegate, - frame_delegate, widget_delegate, routing_id, main_frame_routing_id, @@ -253,7 +251,8 @@ TestRenderViewHost::TestRenderViewHost( simulate_fetch_via_proxy_(false), simulate_history_list_was_cleared_(false), contents_mime_type_("text/html"), - opener_route_id_(MSG_ROUTING_NONE) { + opener_route_id_(MSG_ROUTING_NONE), + main_render_frame_host_(NULL) { // TestRenderWidgetHostView installs itself into this->view_ in its // constructor, and deletes itself when TestRenderWidgetHostView::Destroy() is // called. diff --git a/content/test/test_render_view_host.h b/content/test/test_render_view_host.h index 9eca24f..afe67ba 100644 --- a/content/test/test_render_view_host.h +++ b/content/test/test_render_view_host.h @@ -35,6 +35,7 @@ class Rect; namespace content { class SiteInstance; +class TestRenderFrameHost; class TestWebContents; // Utility function to initialize ViewHostMsg_NavigateParams_Params @@ -233,7 +234,6 @@ class TestRenderViewHost public: TestRenderViewHost(SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, @@ -313,6 +313,14 @@ class TestRenderViewHost // The opener route id passed to CreateRenderView(). int opener_route_id() const { return opener_route_id_; } + // TODO(creis): Remove the need for these methods. + TestRenderFrameHost* main_render_frame_host() const { + return main_render_frame_host_; + } + void set_main_render_frame_host(TestRenderFrameHost* rfh) { + main_render_frame_host_ = rfh; + } + // RenderViewHost overrides -------------------------------------------------- virtual bool CreateRenderView(const base::string16& frame_name, @@ -358,6 +366,8 @@ class TestRenderViewHost // See opener_route_id() above. int opener_route_id_; + TestRenderFrameHost* main_render_frame_host_; + DISALLOW_COPY_AND_ASSIGN(TestRenderViewHost); }; diff --git a/content/test/test_render_view_host_factory.cc b/content/test/test_render_view_host_factory.cc index 6cd4e8a..8bfc697 100644 --- a/content/test/test_render_view_host_factory.cc +++ b/content/test/test_render_view_host_factory.cc @@ -29,14 +29,13 @@ void TestRenderViewHostFactory::set_render_process_host_factory( RenderViewHost* TestRenderViewHostFactory::CreateRenderViewHost( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, bool swapped_out) { return new TestRenderViewHost( - instance, delegate, frame_delegate, widget_delegate, routing_id, - main_frame_routing_id, swapped_out); + instance, delegate, widget_delegate, routing_id, main_frame_routing_id, + swapped_out); } } // namespace content diff --git a/content/test/test_render_view_host_factory.h b/content/test/test_render_view_host_factory.h index df6a2d0..e93762f 100644 --- a/content/test/test_render_view_host_factory.h +++ b/content/test/test_render_view_host_factory.h @@ -30,7 +30,6 @@ class TestRenderViewHostFactory : public RenderViewHostFactory { virtual RenderViewHost* CreateRenderViewHost( SiteInstance* instance, RenderViewHostDelegate* delegate, - RenderFrameHostDelegate* frame_delegate, RenderWidgetHostDelegate* widget_delegate, int routing_id, int main_frame_routing_id, diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc index a2a42e6..a94a2bc 100644 --- a/content/test/test_web_contents.cc +++ b/content/test/test_web_contents.cc @@ -43,7 +43,7 @@ TestWebContents::~TestWebContents() { } RenderViewHost* TestWebContents::GetPendingRenderViewHost() const { - return GetRenderManager()->pending_render_view_host_; + return GetRenderManager()->pending_render_view_host(); } TestRenderViewHost* TestWebContents::pending_test_rvh() const { |