diff options
Diffstat (limited to 'content')
| -rw-r--r-- | content/browser/frame_host/navigation_controller_impl.cc | 46 | ||||
| -rw-r--r-- | content/browser/frame_host/navigation_controller_impl.h | 6 | ||||
| -rw-r--r-- | content/browser/frame_host/navigation_controller_impl_browsertest.cc | 266 | ||||
| -rw-r--r-- | content/browser/frame_host/navigation_entry_impl.cc | 74 | ||||
| -rw-r--r-- | content/browser/frame_host/navigation_entry_impl.h | 14 | ||||
| -rw-r--r-- | content/browser/frame_host/navigation_request.cc | 3 | ||||
| -rw-r--r-- | content/browser/frame_host/navigator_impl.cc | 1 | ||||
| -rw-r--r-- | content/common/frame_messages.h | 1 | ||||
| -rw-r--r-- | content/common/navigation_params.cc | 3 | ||||
| -rw-r--r-- | content/common/navigation_params.h | 7 | ||||
| -rw-r--r-- | content/renderer/render_frame_impl.cc | 7 |
11 files changed, 364 insertions, 64 deletions
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc index 5ee44552..b03a66c 100644 --- a/content/browser/frame_host/navigation_controller_impl.cc +++ b/content/browser/frame_host/navigation_controller_impl.cc @@ -421,6 +421,12 @@ NavigationEntryImpl* NavigationControllerImpl::GetEntryWithPageID( return (index != -1) ? entries_[index].get() : NULL; } +bool NavigationControllerImpl::HasCommittedRealLoad( + FrameTreeNode* frame_tree_node) const { + NavigationEntryImpl* last_committed = GetLastCommittedEntry(); + return last_committed && last_committed->HasFrameEntry(frame_tree_node); +} + void NavigationControllerImpl::LoadEntry(NavigationEntryImpl* entry) { // When navigating to a new page, we don't know for sure if we will actually // end up leaving the current page. The new page load could for example @@ -833,13 +839,10 @@ bool NavigationControllerImpl::RendererDidNavigate( new_type == NAVIGATION_TYPE_EXISTING_PAGE; if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kSitePerProcess)) { - // In site-per-process mode, the new classifier sometimes correctly - // classifies things as auto-subframe where the old classifier incorrectly - // ignored them or called them NEW_SUBFRAME. - ignore_mismatch |= details->type == NAVIGATION_TYPE_NAV_IGNORE && - new_type == NAVIGATION_TYPE_AUTO_SUBFRAME; - ignore_mismatch |= details->type == NAVIGATION_TYPE_NEW_SUBFRAME && - new_type == NAVIGATION_TYPE_AUTO_SUBFRAME; + // We know that the old classifier is wrong for OOPIFs, so use the new one + // in --site-per-process mode. + details->type = new_type; + ignore_mismatch = true; } if (!ignore_mismatch) { DCHECK_EQ(details->type, new_type); @@ -943,10 +946,6 @@ bool NavigationControllerImpl::RendererDidNavigate( NavigationType NavigationControllerImpl::ClassifyNavigation( RenderFrameHostImpl* rfh, const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { - // Hack; remove once http://crbug.com/464014 is fixed. - if (rfh->IsCrossProcessSubframe()) - return NAVIGATION_TYPE_NEW_SUBFRAME; - if (params.page_id == -1) { // The renderer generates the page IDs, and so if it gives us the invalid // page ID (-1) we know it didn't actually navigate. This happens in a few @@ -1096,10 +1095,6 @@ NavigationType NavigationControllerImpl::ClassifyNavigation( NavigationType NavigationControllerImpl::ClassifyNavigationWithoutPageID( RenderFrameHostImpl* rfh, const FrameHostMsg_DidCommitProvisionalLoad_Params& params) const { - // Hack; remove once http://crbug.com/464014 is fixed. - if (rfh->IsCrossProcessSubframe()) - return NAVIGATION_TYPE_NEW_SUBFRAME; - if (params.did_create_new_entry) { // A new entry. We may or may not have a pending entry for the page, and // this may or may not be the main frame. @@ -1450,18 +1445,13 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe( DCHECK(GetLastCommittedEntry()); if (params.nav_entry_id) { - // If the |nav_entry_id| is non-zero, this is a browser-initiated navigation - // and is thus a "history auto" navigation. TODO(creis): Implement - // back/forward this way for site-per-process. - int entry_index = GetEntryIndexWithUniqueID(params.nav_entry_id); - if (entry_index == -1) { - NOTREACHED(); - return false; - } - // Update the current navigation entry in case we're going back/forward. - if (entry_index != last_committed_entry_index_) { + // If the |nav_entry_id| is non-zero and matches an existing entry, this is + // a history auto" navigation. Update the last committed index accordingly. + // If we don't recognize the |nav_entry_id|, it might be either a pending + // entry for a transfer or a recently pruned entry. We'll handle it below. + if (entry_index != -1 && entry_index != last_committed_entry_index_) { // Make sure that a subframe commit isn't changing the main frame's // origin. Otherwise the renderer process may be confused, leading to a // URL spoof. We can't check the path since that may change @@ -1474,6 +1464,8 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe( // kill the renderer process with bad_message::NC_AUTO_SUBFRAME. NOTREACHED() << "Unexpected main frame origin change on AUTO_SUBFRAME."; } + + // TODO(creis): Update the FrameNavigationEntry in --site-per-process. last_committed_entry_index_ = entry_index; DiscardNonCommittedEntriesInternal(); return true; @@ -1488,6 +1480,10 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe( last_committed->AddOrUpdateFrameEntry(rfh->frame_tree_node(), rfh->GetSiteInstance(), params.url, params.referrer); + + // Cross-process subframe navigations may leave a pending entry around. + // TODO(creis): Don't use pending entries for subframe navigations. + DiscardNonCommittedEntriesInternal(); } // We do not need to discard the pending entry in this case, since we will diff --git a/content/browser/frame_host/navigation_controller_impl.h b/content/browser/frame_host/navigation_controller_impl.h index ee00ca4..3a38bd0 100644 --- a/content/browser/frame_host/navigation_controller_impl.h +++ b/content/browser/frame_host/navigation_controller_impl.h @@ -20,6 +20,7 @@ struct FrameHostMsg_DidCommitProvisionalLoad_Params; namespace content { +class FrameTreeNode; class RenderFrameHostImpl; class NavigationEntryScreenshotManager; class SiteInstance; @@ -116,6 +117,11 @@ class CONTENT_EXPORT NavigationControllerImpl SiteInstance* instance, int32 page_id) const; + // Whether the given frame has committed any navigations yet. + // This currently only returns true in --site-per-process mode. + // TODO(creis): Create FrameNavigationEntries by default so this always works. + bool HasCommittedRealLoad(FrameTreeNode* frame_tree_node) const; + NavigationControllerDelegate* delegate() const { return delegate_; } diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc index 3f371bf..a1bfd03 100644 --- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc +++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc @@ -374,12 +374,25 @@ class LoadCommittedCapturer : public WebContentsObserver { private: void RenderFrameCreated(RenderFrameHost* render_frame_host) override { - // If this object was created with a specified tree frame node, there - // shouldn't be any frames being created. - DCHECK_EQ(0, frame_tree_node_id_); RenderFrameHostImpl* rfh = static_cast<RenderFrameHostImpl*>(render_frame_host); - frame_tree_node_id_ = rfh->frame_tree_node()->frame_tree_node_id(); + + // Don't pay attention to swapped out RenderFrameHosts in the main frame. + // TODO(nasko): Remove once swappedout:// is gone. + // See https://crbug.com/357747. + if (!RenderFrameHostImpl::IsRFHStateActive(rfh->rfh_state())) { + DLOG(INFO) << "Skipping swapped out RFH: " + << rfh->GetSiteInstance()->GetSiteURL(); + return; + } + + // If this object was not created with a specified frame tree node, then use + // the first created active RenderFrameHost. Once a node is selected, there + // shouldn't be any other frames being created. + int frame_tree_node_id = rfh->frame_tree_node()->frame_tree_node_id(); + DCHECK(frame_tree_node_id_ == 0 || + frame_tree_node_id_ == frame_tree_node_id); + frame_tree_node_id_ = frame_tree_node_id; } void DidCommitProvisionalLoadForFrame( @@ -969,9 +982,137 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, } } +// Verify the tree of FrameNavigationEntries after initial about:blank commits +// in subframes, which should not count as real committed loads. +IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, + FrameNavigationEntry_BlankAutoSubframe) { + GURL main_url(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_1.html")); + NavigateToURL(shell(), main_url); + const NavigationControllerImpl& controller = + static_cast<const NavigationControllerImpl&>( + shell()->web_contents()->GetController()); + FrameTreeNode* root = + static_cast<WebContentsImpl*>(shell()->web_contents())-> + GetFrameTree()->root(); + + // 1. Create a iframe with no URL. + { + LoadCommittedCapturer capturer(shell()->web_contents()); + std::string script = "var iframe = document.createElement('iframe');" + "document.body.appendChild(iframe);"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type()); + } + + // Check last committed NavigationEntry. + EXPECT_EQ(1, controller.GetEntryCount()); + NavigationEntryImpl* entry = controller.GetLastCommittedEntry(); + EXPECT_EQ(main_url, entry->GetURL()); + FrameNavigationEntry* root_entry = entry->root_node()->frame_entry.get(); + EXPECT_EQ(main_url, root_entry->url()); + + // Verify no subframe entries are created. + EXPECT_EQ(0U, entry->root_node()->children.size()); + EXPECT_FALSE(controller.HasCommittedRealLoad(root->child_at(0))); + + // 2. Create another iframe with an about:blank URL. + { + LoadCommittedCapturer capturer(shell()->web_contents()); + std::string script = "var iframe = document.createElement('iframe');" + "iframe.src = 'about:blank';" + "document.body.appendChild(iframe);"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type()); + } + + // Check last committed NavigationEntry. + EXPECT_EQ(1, controller.GetEntryCount()); + EXPECT_EQ(entry, controller.GetLastCommittedEntry()); + + // Verify no subframe entries are created. + EXPECT_EQ(0U, entry->root_node()->children.size()); + EXPECT_FALSE(controller.HasCommittedRealLoad(root->child_at(1))); + + // 3. A real same-site navigation in the first iframe should be AUTO. + GURL frame_url(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_1.html")); + { + LoadCommittedCapturer capturer(root->child_at(0)); + std::string script = "var frames = document.getElementsByTagName('iframe');" + "frames[0].src = '" + frame_url.spec() + "';"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type()); + } + + // Check last committed NavigationEntry. + EXPECT_EQ(1, controller.GetEntryCount()); + EXPECT_EQ(entry, controller.GetLastCommittedEntry()); + + // Verify subframe entries if we're in --site-per-process mode. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)) { + // The entry should now have one subframe FrameNavigationEntry. + ASSERT_EQ(1U, entry->root_node()->children.size()); + FrameNavigationEntry* frame_entry = + entry->root_node()->children[0]->frame_entry.get(); + EXPECT_EQ(frame_url, frame_entry->url()); + EXPECT_TRUE(controller.HasCommittedRealLoad(root->child_at(0))); + EXPECT_FALSE(controller.HasCommittedRealLoad(root->child_at(1))); + } else { + // There are no subframe FrameNavigationEntries by default. + EXPECT_EQ(0U, entry->root_node()->children.size()); + } + + // 4. A real cross-site navigation in the second iframe should be AUTO. + GURL foo_url(embedded_test_server()->GetURL( + "foo.com", "/navigation_controller/simple_page_2.html")); + { + LoadCommittedCapturer capturer(root->child_at(1)); + std::string script = "var frames = document.getElementsByTagName('iframe');" + "frames[1].src = '" + foo_url.spec() + "';"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type()); + } + + // Check last committed NavigationEntry. + EXPECT_EQ(1, controller.GetEntryCount()); + EXPECT_EQ(entry, controller.GetLastCommittedEntry()); + + // Verify subframe entries if we're in --site-per-process mode. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)) { + // The entry should now have two subframe FrameNavigationEntries. + ASSERT_EQ(2U, entry->root_node()->children.size()); + FrameNavigationEntry* frame_entry = + entry->root_node()->children[1]->frame_entry.get(); + EXPECT_EQ(foo_url, frame_entry->url()); + EXPECT_TRUE(controller.HasCommittedRealLoad(root->child_at(1))); + } else { + // There are no subframe FrameNavigationEntries by default. + EXPECT_EQ(0U, entry->root_node()->children.size()); + } + + // Check the end result of the frame tree. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)) { + FrameTreeVisualizer visualizer; + EXPECT_EQ( + " Site A ------------ proxies for B\n" + " |--Site A ------- proxies for B\n" + " +--Site B ------- proxies for A\n" + "Where A = http://127.0.0.1/\n" + " B = http://foo.com/", + visualizer.DepictFrameTree(root)); + } +} + // Verify the tree of FrameNavigationEntries after NAVIGATION_TYPE_AUTO_SUBFRAME // commits. -// TODO(creis): Test cross-site iframes. // TODO(creis): Test updating entries for history auto subframe navigations. IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry_AutoSubframe) { @@ -985,7 +1126,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, static_cast<WebContentsImpl*>(shell()->web_contents())-> GetFrameTree()->root(); - // Create an iframe. + // 1. Create a same-site iframe. GURL frame_url(embedded_test_server()->GetURL( "/navigation_controller/simple_page_2.html")); { @@ -1004,6 +1145,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(main_url, entry->GetURL()); FrameNavigationEntry* root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); + EXPECT_FALSE(controller.GetPendingEntry()); // Verify subframe entries if we're in --site-per-process mode. if (base::CommandLine::ForCurrentProcess()->HasSwitch( @@ -1013,16 +1155,19 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry* frame_entry = entry->root_node()->children[0]->frame_entry.get(); EXPECT_EQ(frame_url, frame_entry->url()); + EXPECT_TRUE(controller.HasCommittedRealLoad(root->child_at(0))); } else { // There are no subframe FrameNavigationEntries by default. EXPECT_EQ(0U, entry->root_node()->children.size()); } - // Create a second iframe. + // 2. Create a second, initially cross-site iframe. + GURL foo_url(embedded_test_server()->GetURL( + "foo.com", "/navigation_controller/simple_page_1.html")); { LoadCommittedCapturer capturer(shell()->web_contents()); std::string script = "var iframe = document.createElement('iframe');" - "iframe.src = '" + frame_url.spec() + "';" + "iframe.src = '" + foo_url.spec() + "';" "document.body.appendChild(iframe);"; EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); capturer.Wait(); @@ -1035,6 +1180,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(main_url, entry->GetURL()); root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); + EXPECT_FALSE(controller.GetPendingEntry()); // Verify subframe entries if we're in --site-per-process mode. if (base::CommandLine::ForCurrentProcess()->HasSwitch( @@ -1043,17 +1189,18 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, ASSERT_EQ(2U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = entry->root_node()->children[1]->frame_entry.get(); - EXPECT_EQ(frame_url, frame_entry->url()); + EXPECT_EQ(foo_url, frame_entry->url()); + EXPECT_TRUE(controller.HasCommittedRealLoad(root->child_at(1))); } else { // There are no subframe FrameNavigationEntries by default. EXPECT_EQ(0U, entry->root_node()->children.size()); } - // Create a nested iframe in the second subframe. + // 3. Create a nested iframe in the second subframe. { LoadCommittedCapturer capturer(shell()->web_contents()); std::string script = "var iframe = document.createElement('iframe');" - "iframe.src = '" + frame_url.spec() + "';" + "iframe.src = '" + foo_url.spec() + "';" "document.body.appendChild(iframe);"; EXPECT_TRUE(content::ExecuteScript(root->child_at(1)->current_frame_host(), script)); @@ -1076,11 +1223,106 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, ASSERT_EQ(1U, entry->root_node()->children[1]->children.size()); FrameNavigationEntry* frame_entry = entry->root_node()->children[1]->children[0]->frame_entry.get(); - EXPECT_EQ(frame_url, frame_entry->url()); + EXPECT_EQ(foo_url, frame_entry->url()); } else { // There are no subframe FrameNavigationEntries by default. EXPECT_EQ(0U, entry->root_node()->children.size()); } + + // TODO(creis): Add tests for another subframe on B, and for a subframe on A + // within it. Both are currently broken. + + // Check the end result of the frame tree. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)) { + FrameTreeVisualizer visualizer; + EXPECT_EQ( + " Site A ------------ proxies for B\n" + " |--Site A ------- proxies for B\n" + " +--Site B ------- proxies for A\n" + " +--Site B -- proxies for A\n" + "Where A = http://127.0.0.1/\n" + " B = http://foo.com/", + visualizer.DepictFrameTree(root)); + } +} + +// Verify the tree of FrameNavigationEntries after NAVIGATION_TYPE_NEW_SUBFRAME +// commits. +IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, + FrameNavigationEntry_NewSubframe) { + GURL main_url(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_1.html")); + NavigateToURL(shell(), main_url); + FrameTreeNode* root = + static_cast<WebContentsImpl*>(shell()->web_contents())-> + GetFrameTree()->root(); + + // 1. Create a same-site iframe. + GURL frame_url(embedded_test_server()->GetURL( + "/navigation_controller/simple_page_2.html")); + { + LoadCommittedCapturer capturer(shell()->web_contents()); + std::string script = "var iframe = document.createElement('iframe');" + "iframe.src = '" + frame_url.spec() + "';" + "document.body.appendChild(iframe);"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + } + + // 2. Navigate in the first subframe same-site. + GURL frame_url2(embedded_test_server()->GetURL( + "/navigation_controller/page_with_links.html")); + { + FrameNavigateParamsCapturer capturer(root->child_at(0)); + NavigateFrameToURL(root->child_at(0), frame_url2); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, + capturer.params().transition); + EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type); + } + + // 3. Create a second, initially cross-site iframe. + GURL foo_url(embedded_test_server()->GetURL( + "foo.com", "/navigation_controller/simple_page_1.html")); + { + LoadCommittedCapturer capturer(shell()->web_contents()); + std::string script = "var iframe = document.createElement('iframe');" + "iframe.src = '" + foo_url.spec() + "';" + "document.body.appendChild(iframe);"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + } + + // 4. Navigate in the second subframe cross-site. + GURL bar_url(embedded_test_server()->GetURL( + "bar.com", "/navigation_controller/simple_page_1.html")); + { + FrameNavigateParamsCapturer capturer(root->child_at(1)); + std::string script = "var frames = document.getElementsByTagName('iframe');" + "frames[1].src = '" + bar_url.spec() + "';"; + EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script)); + capturer.Wait(); + EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, + capturer.params().transition); + EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type); + } + + // TODO(creis): Expand this test once we clone FrameNavigationEntries for + // NEW_SUBFRAME navigations. + + // Check the end result of the frame tree. + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)) { + FrameTreeVisualizer visualizer; + EXPECT_EQ( + " Site A ------------ proxies for B\n" + " |--Site A ------- proxies for B\n" + " +--Site B ------- proxies for A\n" + "Where A = http://127.0.0.1/\n" + " B = http://bar.com/", + visualizer.DepictFrameTree(root)); + } } namespace { diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc index 2723606..7ba5cf3 100644 --- a/content/browser/frame_host/navigation_entry_impl.cc +++ b/content/browser/frame_host/navigation_entry_impl.cc @@ -33,6 +33,17 @@ NavigationEntryImpl::TreeNode::TreeNode(FrameNavigationEntry* frame_entry) NavigationEntryImpl::TreeNode::~TreeNode() { } +bool NavigationEntryImpl::TreeNode::MatchesFrame( + FrameTreeNode* frame_tree_node) const { + if (frame_tree_node->frame_tree_node_id() == + frame_entry->frame_tree_node_id()) + return true; + + // For now, we set the root FNE's FrameTreeNode ID to -1. + return frame_tree_node->IsMainFrame() && + frame_entry->frame_tree_node_id() == -1; +} + NavigationEntryImpl::TreeNode* NavigationEntryImpl::TreeNode::Clone() const { // Clone the tree using a copy of the FrameNavigationEntry, without sharing. NavigationEntryImpl::TreeNode* copy = @@ -419,6 +430,7 @@ StartNavigationParams NavigationEntryImpl::ConstructStartNavigationParams() RequestNavigationParams NavigationEntryImpl::ConstructRequestNavigationParams( base::TimeTicks navigation_start, + bool has_committed_real_load, bool intended_as_new_entry, int pending_history_list_offset, int current_history_list_offset, @@ -444,9 +456,9 @@ RequestNavigationParams NavigationEntryImpl::ConstructRequestNavigationParams( return RequestNavigationParams( GetIsOverridingUserAgent(), navigation_start, redirects, GetCanLoadLocalResources(), base::Time::Now(), GetPageState(), - GetPageID(), GetUniqueID(), intended_as_new_entry, pending_offset_to_send, - current_offset_to_send, current_length_to_send, - should_clear_history_list()); + GetPageID(), GetUniqueID(), has_committed_real_load, + intended_as_new_entry, pending_offset_to_send, current_offset_to_send, + current_length_to_send, should_clear_history_list()); } void NavigationEntryImpl::ResetForCommit() { @@ -477,27 +489,9 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node, // We should already have a TreeNode for the parent node by the time this node // commits. Find it first. DCHECK(frame_tree_node->parent()); - int parent_ftn_id = frame_tree_node->parent()->frame_tree_node_id(); - bool found = false; - NavigationEntryImpl::TreeNode* parent_node = nullptr; - std::queue<NavigationEntryImpl::TreeNode*> work_queue; - work_queue.push(root_node()); - while (!found && !work_queue.empty()) { - parent_node = work_queue.front(); - work_queue.pop(); - // The root FNE will have an ID of -1, so check for that as well. - if (parent_node->frame_entry->frame_tree_node_id() == parent_ftn_id || - (parent_node->frame_entry->frame_tree_node_id() == -1 && - parent_node == root_node() && - frame_tree_node->parent()->IsMainFrame())) { - found = true; - break; - } - // Enqueue any children and keep looking. - for (auto& child : parent_node->children) - work_queue.push(child); - } - if (!found) { + NavigationEntryImpl::TreeNode* parent_node = + FindFrameEntry(frame_tree_node->parent()); + if (!parent_node) { // The renderer should not send a commit for a subframe before its parent. // TODO(creis): This can currently happen because we don't yet clone the // FrameNavigationEntry tree on manual subframe navigations. Once that's @@ -515,14 +509,21 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node, } } - // No entry exists yet, so create a new one. Unordered list, since we expect - // to look up entries by frame sequence number or unique name. + // No entry exists yet, so create a new one unless it's for about:blank. + // Unordered list, since we expect to look up entries by frame sequence number + // or unique name. + if (url == GURL(url::kAboutBlankURL)) + return; FrameNavigationEntry* frame_entry = new FrameNavigationEntry( frame_tree_node_id, site_instance, url, referrer); parent_node->children.push_back( new NavigationEntryImpl::TreeNode(frame_entry)); } +bool NavigationEntryImpl::HasFrameEntry(FrameTreeNode* frame_tree_node) const { + return FindFrameEntry(frame_tree_node) != nullptr; +} + void NavigationEntryImpl::SetScreenshotPNGData( scoped_refptr<base::RefCountedBytes> png_data) { screenshot_ = png_data; @@ -534,4 +535,25 @@ GURL NavigationEntryImpl::GetHistoryURLForDataURL() const { return GetBaseURLForDataURL().is_empty() ? GURL() : GetVirtualURL(); } +NavigationEntryImpl::TreeNode* NavigationEntryImpl::FindFrameEntry( + FrameTreeNode* frame_tree_node) const { + NavigationEntryImpl::TreeNode* node = nullptr; + std::queue<NavigationEntryImpl::TreeNode*> work_queue; + work_queue.push(root_node()); + while (!work_queue.empty()) { + node = work_queue.front(); + work_queue.pop(); + if (node->MatchesFrame(frame_tree_node)) { + // Only the root TreeNode should have a FTN ID of -1. + DCHECK_IMPLIES(node->frame_entry->frame_tree_node_id() == -1, + node == root_node()); + return node; + } + // Enqueue any children and keep looking. + for (auto& child : node->children) + work_queue.push(child); + } + return nullptr; +} + } // namespace content diff --git a/content/browser/frame_host/navigation_entry_impl.h b/content/browser/frame_host/navigation_entry_impl.h index d5f43ea..8fb41c3 100644 --- a/content/browser/frame_host/navigation_entry_impl.h +++ b/content/browser/frame_host/navigation_entry_impl.h @@ -34,6 +34,9 @@ class CONTENT_EXPORT NavigationEntryImpl TreeNode(FrameNavigationEntry* frame_entry); ~TreeNode(); + // Returns whether this TreeNode corresponds to |frame_tree_node|. + bool MatchesFrame(FrameTreeNode* frame_tree_node) const; + // Returns a deep copy of the tree with copies of each node's // FrameNavigationEntries. We do not yet share FrameNavigationEntries // across trees. @@ -134,6 +137,7 @@ class CONTENT_EXPORT NavigationEntryImpl StartNavigationParams ConstructStartNavigationParams() const; RequestNavigationParams ConstructRequestNavigationParams( base::TimeTicks navigation_start, + bool has_committed_real_load, bool intended_as_new_entry, int pending_offset_to_send, int current_offset_to_send, @@ -156,11 +160,17 @@ class CONTENT_EXPORT NavigationEntryImpl // its FrameNavigationEntry. A new FrameNavigationEntry is added if none // exists, or else the existing one (which might be shared with other // NavigationEntries) is updated with the given parameters. + // Does nothing if there is no entry already and |url| is about:blank, since + // that does not count as a real commit. void AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node, SiteInstanceImpl* site_instance, const GURL& url, const Referrer& referrer); + // Returns whether this entry has a FrameNavigationEntry for the given + // |frame_tree_node|. + bool HasFrameEntry(FrameTreeNode* frame_tree_node) const; + void set_unique_id(int unique_id) { unique_id_ = unique_id; } @@ -307,6 +317,10 @@ class CONTENT_EXPORT NavigationEntryImpl #endif private: + // Finds the TreeNode associated with |frame_tree_node|, if any. + NavigationEntryImpl::TreeNode* FindFrameEntry( + FrameTreeNode* frame_tree_node) const; + // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING // Session/Tab restore save portions of this class so that it can be recreated // later. If you add a new field that needs to be persisted you'll have to diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc index ce8b2ae..b5f959b 100644 --- a/content/browser/frame_host/navigation_request.cc +++ b/content/browser/frame_host/navigation_request.cc @@ -90,7 +90,7 @@ scoped_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated( BeginNavigationParams(method, headers.ToString(), LoadFlagFromNavigationType(navigation_type), false), entry.ConstructRequestNavigationParams( - navigation_start, + navigation_start, controller->HasCommittedRealLoad(frame_tree_node), controller->GetPendingEntryIndex() == -1, controller->GetIndexOfEntry(&entry), controller->GetLastCommittedEntryIndex(), @@ -113,6 +113,7 @@ scoped_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated( // TODO(clamy): See if the navigation start time should be measured in the // renderer and sent to the browser instead of being measured here. // TODO(clamy): The pending history list offset should be properly set. + // TODO(clamy): Set has_committed_real_load. RequestNavigationParams request_params; request_params.current_history_list_offset = current_history_list_offset; request_params.current_history_list_length = current_history_list_length; diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc index b9dc7c0..8219a3d 100644 --- a/content/browser/frame_host/navigator_impl.cc +++ b/content/browser/frame_host/navigator_impl.cc @@ -325,6 +325,7 @@ bool NavigatorImpl::NavigateToEntry( entry.ConstructStartNavigationParams(), entry.ConstructRequestNavigationParams( navigation_start, + controller_->HasCommittedRealLoad(frame_tree_node), controller_->GetPendingEntryIndex() == -1, controller_->GetIndexOfEntry(&entry), controller_->GetLastCommittedEntryIndex(), diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index 4187c13..3ad3deb 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h @@ -285,6 +285,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::RequestNavigationParams) IPC_STRUCT_TRAITS_MEMBER(page_state) IPC_STRUCT_TRAITS_MEMBER(page_id) IPC_STRUCT_TRAITS_MEMBER(nav_entry_id) + IPC_STRUCT_TRAITS_MEMBER(has_committed_real_load) IPC_STRUCT_TRAITS_MEMBER(intended_as_new_entry) IPC_STRUCT_TRAITS_MEMBER(pending_history_list_offset) IPC_STRUCT_TRAITS_MEMBER(current_history_list_offset) diff --git a/content/common/navigation_params.cc b/content/common/navigation_params.cc index 57149de..33d1c8e 100644 --- a/content/common/navigation_params.cc +++ b/content/common/navigation_params.cc @@ -85,6 +85,7 @@ RequestNavigationParams::RequestNavigationParams() request_time(base::Time::Now()), page_id(-1), nav_entry_id(0), + has_committed_real_load(false), intended_as_new_entry(false), pending_history_list_offset(-1), current_history_list_offset(-1), @@ -101,6 +102,7 @@ RequestNavigationParams::RequestNavigationParams( const PageState& page_state, int32 page_id, int nav_entry_id, + bool has_committed_real_load, bool intended_as_new_entry, int pending_history_list_offset, int current_history_list_offset, @@ -114,6 +116,7 @@ RequestNavigationParams::RequestNavigationParams( page_state(page_state), page_id(page_id), nav_entry_id(nav_entry_id), + has_committed_real_load(has_committed_real_load), intended_as_new_entry(intended_as_new_entry), pending_history_list_offset(pending_history_list_offset), current_history_list_offset(current_history_list_offset), diff --git a/content/common/navigation_params.h b/content/common/navigation_params.h index 90eb70e..8059471 100644 --- a/content/common/navigation_params.h +++ b/content/common/navigation_params.h @@ -170,6 +170,7 @@ struct CONTENT_EXPORT RequestNavigationParams { const PageState& page_state, int32 page_id, int nav_entry_id, + bool has_committed_real_load, bool intended_as_new_entry, int pending_history_list_offset, int current_history_list_offset, @@ -211,6 +212,12 @@ struct CONTENT_EXPORT RequestNavigationParams { // the resulting FrameHostMsg_DidCommitProvisionalLoad message. int nav_entry_id; + // Whether the frame being navigated has already committed a real page, which + // affects how new navigations are classified in the renderer process. + // This currently is only ever set to true in --site-per-process mode. + // TODO(creis): Create FrameNavigationEntries by default so this always works. + bool has_committed_real_load; + // For browser-initiated navigations, this is true if this is a new entry // being navigated to. This is false otherwise. TODO(avi): Remove this when // the pending entry situation is made sane and the browser keeps them around diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index f9c71e7..2bfd535 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -4406,6 +4406,13 @@ void RenderFrameImpl::NavigateInternal( GetContentClient()->SetActiveURL(common_params.url); + // If this frame isn't in the same process as the main frame, it may naively + // assume that this is the first navigation in the iframe, but this may not + // actually be the case. Inform the frame's state machine if this frame has + // already committed other loads. + if (request_params.has_committed_real_load && frame_->parent()) + frame_->setCommittedFirstRealLoad(); + if (is_reload && !render_view_->history_controller()->GetCurrentEntry()) { // We cannot reload if we do not have any history state. This happens, for // example, when recovering from a crash. |