diff options
3 files changed, 62 insertions, 49 deletions
diff --git a/chromium/content/browser/frame_host/navigation_controller_impl_unittest.cc b/chromium/content/browser/frame_host/navigation_controller_impl_unittest.cc index 190e5f060ba..c46e47d7dbf 100644 --- a/chromium/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/chromium/content/browser/frame_host/navigation_controller_impl_unittest.cc @@ -3511,11 +3511,35 @@ TEST_F(NavigationControllerTest, SubframeWhilePending) { TestRenderFrameHost* subframe = static_cast<TestRenderFrameHost*>( contents()->GetFrameTree()->root()->child_at(0)->current_frame_host()); const GURL url1_sub("http://foo/subframe"); - NavigationSimulator::NavigateAndCommitFromDocument(url1_sub, subframe); + + auto subframe_navigation = + NavigationSimulator::CreateRendererInitiated(url1_sub, subframe); + subframe_navigation->Start(); + + // Creating the subframe navigation should have no effect on the pending + // navigation entry and on the visible URL. + EXPECT_EQ(url1, controller.GetLastCommittedEntry()->GetURL()); + EXPECT_EQ(url2, controller.GetVisibleEntry()->GetURL()); + + auto nav_entry_id = [](NavigationSimulator* simulator) { + return NavigationRequest::From(simulator->GetNavigationHandle()) + ->nav_entry_id(); + }; + + // The main frame navigation is still associated with the pending entry, the + // subframe one isn't. + EXPECT_EQ(controller.GetPendingEntry()->GetUniqueID(), + nav_entry_id(main_frame_navigation.get())); + EXPECT_EQ(0, nav_entry_id(subframe_navigation.get())); + + subframe_navigation->Commit(); // The subframe navigation should have no effect on the displayed url. EXPECT_EQ(url1, controller.GetLastCommittedEntry()->GetURL()); EXPECT_EQ(url2, controller.GetVisibleEntry()->GetURL()); + + EXPECT_EQ(controller.GetPendingEntry()->GetUniqueID(), + nav_entry_id(main_frame_navigation.get())); } // Test CopyStateFrom with 2 urls, the first selected and nothing in the target. diff --git a/chromium/content/browser/frame_host/navigator_impl.cc b/chromium/content/browser/frame_host/navigator_impl.cc index e8043a64b3a..80901a82ea3 100644 --- a/chromium/content/browser/frame_host/navigator_impl.cc +++ b/chromium/content/browser/frame_host/navigator_impl.cc @@ -659,22 +659,9 @@ void NavigatorImpl::OnBeginNavigation( return; } - // In all other cases the current navigation, if any, is canceled and a new - // NavigationRequest is created for the node. - if (frame_tree_node->IsMainFrame()) { - // Renderer-initiated main-frame navigations that need to swap processes - // will go to the browser via a OpenURL call, and then be handled by the - // same code path as browser-initiated navigations. For renderer-initiated - // main frame navigation that start via a BeginNavigation IPC, the - // RenderFrameHost will not be swapped. Therefore it is safe to call - // DidStartMainFrameNavigation with the SiteInstance from the current - // RenderFrameHost. - DidStartMainFrameNavigation( - common_params, frame_tree_node->current_frame_host()->GetSiteInstance(), - nullptr); - navigation_data_.reset(); - } - NavigationEntryImpl* pending_entry = controller_->GetPendingEntry(); + NavigationEntryImpl* navigation_entry = + GetNavigationEntryForRendererInitiatedNavigation(common_params, + frame_tree_node); NavigationEntryImpl* current_entry = controller_->GetLastCommittedEntry(); // Only consult the delegate for override state if there is no current entry, // since that state should only apply to newly created tabs (and not cases @@ -685,7 +672,7 @@ void NavigatorImpl::OnBeginNavigation( : delegate_ && delegate_->ShouldOverrideUserAgentInNewTabs(); frame_tree_node->CreatedNavigationRequest( NavigationRequest::CreateRendererInitiated( - frame_tree_node, pending_entry, common_params, + frame_tree_node, navigation_entry, common_params, std::move(begin_params), controller_->GetLastCommittedEntryIndex(), controller_->GetEntryCount(), override_user_agent, std::move(blob_url_loader_factory), std::move(navigation_client), @@ -874,10 +861,14 @@ void NavigatorImpl::RecordNavigationMetrics( navigation_data_.reset(); } -void NavigatorImpl::DidStartMainFrameNavigation( +NavigationEntryImpl* +NavigatorImpl::GetNavigationEntryForRendererInitiatedNavigation( const CommonNavigationParams& common_params, - SiteInstanceImpl* site_instance, - NavigationHandleImpl* navigation_handle) { + FrameTreeNode* frame_tree_node) { + + if (!frame_tree_node->IsMainFrame()) + return nullptr; + // If there is no browser-initiated pending entry for this navigation and it // is not for the error URL, create a pending entry and ensure the address bar // updates accordingly. We don't know the referrer or extra headers at this @@ -887,6 +878,8 @@ void NavigatorImpl::DidStartMainFrameNavigation( NavigationEntryImpl* pending_entry = controller_->GetPendingEntry(); bool has_browser_initiated_pending_entry = pending_entry && !pending_entry->is_renderer_initiated(); + if (has_browser_initiated_pending_entry) + return nullptr; // A pending navigation entry is created in OnBeginNavigation(). The renderer // sends a provisional load notification after that. We don't want to create @@ -894,33 +887,29 @@ void NavigatorImpl::DidStartMainFrameNavigation( bool renderer_provisional_load_to_pending_url = pending_entry && pending_entry->is_renderer_initiated() && (pending_entry->GetURL() == common_params.url); + if (renderer_provisional_load_to_pending_url) + return nullptr; // If there is a transient entry, creating a new pending entry will result // in deleting it, which leads to inconsistent state. bool has_transient_entry = !!controller_->GetTransientEntry(); + if (has_transient_entry) + return nullptr; + + std::unique_ptr<NavigationEntryImpl> entry = + NavigationEntryImpl::FromNavigationEntry( + NavigationController::CreateNavigationEntry( + common_params.url, content::Referrer(), + common_params.initiator_origin, ui::PAGE_TRANSITION_LINK, + true /* is_renderer_initiated */, std::string(), + controller_->GetBrowserContext(), + nullptr /* blob_url_loader_factory */)); + + controller_->SetPendingEntry(std::move(entry)); + if (delegate_) + delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL); - if (!has_browser_initiated_pending_entry && !has_transient_entry && - !renderer_provisional_load_to_pending_url) { - std::unique_ptr<NavigationEntryImpl> entry = - NavigationEntryImpl::FromNavigationEntry( - NavigationController::CreateNavigationEntry( - common_params.url, content::Referrer(), - common_params.initiator_origin, ui::PAGE_TRANSITION_LINK, - true /* is_renderer_initiated */, std::string(), - controller_->GetBrowserContext(), - nullptr /* blob_url_loader_factory */)); - // TODO(creis): If there's a pending entry already, find a safe way to - // update it instead of replacing it and copying over things like this. - // That will allow us to skip the NavigationHandle update below as well. - if (pending_entry) { - entry->set_should_replace_entry(pending_entry->should_replace_entry()); - entry->SetRedirectChain(pending_entry->GetRedirectChain()); - } - - controller_->SetPendingEntry(std::move(entry)); - if (delegate_) - delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL); - } + return controller_->GetPendingEntry(); } } // namespace content diff --git a/chromium/content/browser/frame_host/navigator_impl.h b/chromium/content/browser/frame_host/navigator_impl.h index 5a7d89149d2..0f635c7a00b 100644 --- a/chromium/content/browser/frame_host/navigator_impl.h +++ b/chromium/content/browser/frame_host/navigator_impl.h @@ -124,12 +124,12 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator { const FrameHostMsg_DidCommitProvisionalLoad_Params& params, SiteInstance* site_instance); - // Called when a navigation has started in a main frame, to update the pending - // NavigationEntry if the controller does not currently have a - // browser-initiated one. - void DidStartMainFrameNavigation(const CommonNavigationParams& common_params, - SiteInstanceImpl* site_instance, - NavigationHandleImpl* navigation_handle); + // Called when a renderer initiated navigation has started. Returns the + // pending NavigationEntry to be used. Either null or a new one owned + // NavigationController. + NavigationEntryImpl* GetNavigationEntryForRendererInitiatedNavigation( + const CommonNavigationParams& common_params, + FrameTreeNode* frame_tree_node); // The NavigationController that will keep track of session history for all // RenderFrameHost objects using this NavigatorImpl. |