PlzNavigate: move ownership of the NavigationRequest to the FrameTreeNode

content/browser/frame_host/navigator_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigator_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 349 matching lines @@
 }
 
 void NavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& input_params) {
   // PlzNavigate
   // The navigation request has been committed so the browser process doesn't
   // need to care about it anymore.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
-    navigation_request_map_.erase(
-        render_frame_host->frame_tree_node()->frame_tree_node_id());
+    render_frame_host->frame_tree_node()->ResetNavigationRequest(true);
   }
 
   FrameHostMsg_DidCommitProvisionalLoad_Params params(input_params);
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool use_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kSitePerProcess);
 
   if (ui::PageTransitionIsMainFrame(params.transition)) {
     if (delegate_) {
       // When overscroll navigation gesture is enabled, a screenshot of the page
@@ skipping 226 matching lines @@
     delegate_->RequestOpenURL(render_frame_host, params);
 }
 
 // PlzNavigate
 void NavigatorImpl::OnBeforeUnloadACK(FrameTreeNode* frame_tree_node,
                                       bool proceed) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   DCHECK(frame_tree_node);
 
-  NavigationRequest* navigation_request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+  NavigationRequest* navigation_request = frame_tree_node->navigation_request();
 
   // The NavigationRequest may have been canceled while the renderer was
   // executing the BeforeUnload event.
   if (!navigation_request)
     return;
 
   DCHECK_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
             navigation_request->state());
 
   if (proceed)
     BeginNavigation(frame_tree_node);
   else
     CancelNavigation(frame_tree_node);
 }
 
 // PlzNavigate
 void NavigatorImpl::OnBeginNavigation(
     FrameTreeNode* frame_tree_node,
     const CommonNavigationParams& common_params,
     const BeginNavigationParams& begin_params,
     scoped_refptr<ResourceRequestBody> body) {
   // This is a renderer-initiated navigation.
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   DCHECK(frame_tree_node);
 
   NavigationRequest* ongoing_navigation_request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+      frame_tree_node->navigation_request();
 
   // The renderer-initiated navigation request is ignored iff a) there is an
   // ongoing request b) which is browser or user-initiated and c) the renderer
   // request is not user-initiated.
   if (ongoing_navigation_request &&
       (ongoing_navigation_request->browser_initiated() ||
        ongoing_navigation_request->begin_params().has_user_gesture) &&
       !begin_params.has_user_gesture) {
     return;
   }
 
   // In all other cases the current navigation, if any, is canceled and a new
-  // NavigationRequest is created and stored in the map.
-  if (ongoing_navigation_request)
-    CancelNavigation(frame_tree_node);
-
+  // NavigationRequest is created for the node.
   scoped_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateRendererInitiated(
           frame_tree_node, common_params, begin_params, body,
           controller_->GetLastCommittedEntryIndex(),
           controller_->GetEntryCount());
-  navigation_request_map_.set(
-      frame_tree_node->frame_tree_node_id(), navigation_request.Pass());
+  frame_tree_node->SetNavigationRequest(navigation_request.Pass());
 
   if (frame_tree_node->IsMainFrame())
     navigation_data_.reset();
 
   BeginNavigation(frame_tree_node);
 }
 
 // PlzNavigate
 void NavigatorImpl::CommitNavigation(FrameTreeNode* frame_tree_node,
                                      ResourceResponse* response,
                                      scoped_ptr<StreamHandle> body) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
 
-  NavigationRequest* navigation_request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+  NavigationRequest* navigation_request = frame_tree_node->navigation_request();
   DCHECK(navigation_request);
   DCHECK(response ||
          !NavigationRequest::ShouldMakeNetworkRequest(
              navigation_request->common_params().url));
 
   // HTTP 204 (No Content) and HTTP 205 (Reset Content) responses should not
   // commit; they leave the frame showing the previous page.
   if (response && response->head.headers.get() &&
       (response->head.headers->response_code() == 204 ||
        response->head.headers->response_code() == 205)) {
@@ skipping 22 matching lines @@
 
   render_frame_host->CommitNavigation(response, body.Pass(),
                                       navigation_request->common_params(),
                                       navigation_request->request_params());
 }
 
 // PlzNavigate
 void NavigatorImpl::CancelNavigation(FrameTreeNode* frame_tree_node) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
-  navigation_request_map_.erase(frame_tree_node->frame_tree_node_id());
+  frame_tree_node->ResetNavigationRequest(false);
   if (frame_tree_node->IsMainFrame())
     navigation_data_.reset();
   // TODO(carlosk): move this cleanup into the NavigationRequest destructor once
   // we properly cancel ongoing navigations.

[Fabrice (no longer in Chrome), 2015/04/15 12:44:50]

Is this comment still relevant? If yes, I think it needs to be rewritten.

[clamy, 2015/04/16 12:12:30]

Done.

-  frame_tree_node->render_manager()->CleanUpNavigation();
-}
-
-// PlzNavigate
-NavigationRequest* NavigatorImpl::GetNavigationRequestForNodeForTesting(
-    FrameTreeNode* frame_tree_node) {
-  return navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
 }
 
 bool NavigatorImpl::IsWaitingForBeforeUnloadACK(
     FrameTreeNode* frame_tree_node) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
-  NavigationRequest* request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+  NavigationRequest* request = frame_tree_node->navigation_request();
   if (!request)
     return false;
   return request->state() == NavigationRequest::WAITING_FOR_RENDERER_RESPONSE;
 }
 
 void NavigatorImpl::LogResourceRequestTime(
     base::TimeTicks timestamp, const GURL& url) {
   if (navigation_data_ && navigation_data_->url_ == url) {
     navigation_data_->url_job_start_time_ = timestamp;
     UMA_HISTOGRAM_TIMES(
@@ skipping 32 matching lines @@
 
 // PlzNavigate
 void NavigatorImpl::RequestNavigation(
     FrameTreeNode* frame_tree_node,
     const NavigationEntryImpl& entry,
     NavigationController::ReloadType reload_type,
     base::TimeTicks navigation_start) {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   DCHECK(frame_tree_node);
-  int64 frame_tree_node_id = frame_tree_node->frame_tree_node_id();
   FrameMsg_Navigate_Type::Value navigation_type =
       GetNavigationType(controller_->GetBrowserContext(), entry, reload_type);
   scoped_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateBrowserInitiated(frame_tree_node, entry,
                                                 navigation_type,
                                                 navigation_start, controller_);
   // TODO(clamy): Check if navigations are blocked and if so store the
   // parameters.
 
-  // If there is an ongoing request, cancel and replace it.
-  NavigationRequest* ongoing_request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
-  if (ongoing_request)
-    CancelNavigation(frame_tree_node);
-
-  navigation_request_map_.set(frame_tree_node_id, navigation_request.Pass());
+  frame_tree_node->SetNavigationRequest(navigation_request.Pass());

[Fabrice (no longer in Chrome), 2015/04/15 12:44:50]

With the TODO line above, this is slightly confusing.
Maybe re-add a comment to make it clear it is not related?

[clamy, 2015/04/16 12:12:30]

Done.

 
   // Have the current renderer execute its beforeUnload event if needed. If it
   // is not needed (eg. the renderer is not live), BeginNavigation should get
   // called.
-  NavigationRequest* request_to_send =
-      navigation_request_map_.get(frame_tree_node_id);
-  request_to_send->SetWaitingForRendererResponse();
+  frame_tree_node->navigation_request()->SetWaitingForRendererResponse();
   frame_tree_node->current_frame_host()->DispatchBeforeUnload(true);
 }
 
 void NavigatorImpl::BeginNavigation(FrameTreeNode* frame_tree_node) {
-  NavigationRequest* navigation_request =
-      navigation_request_map_.get(frame_tree_node->frame_tree_node_id());
+  NavigationRequest* navigation_request = frame_tree_node->navigation_request();
 
   // A browser-initiated navigation could have been cancelled while it was
   // waiting for the BeforeUnload event to execute.
   if (!navigation_request)
     return;
 
   // Start the request.
    if (navigation_request->BeginNavigation()) {
     // If the request was sent to the IO thread, notify the
     // RenderFrameHostManager so it can speculatively create a RenderFrameHost
@@ skipping 50 matching lines @@
         "Navigation.TimeToCommit_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_commit);
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToURLJobStart_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_network);
   }
   navigation_data_.reset();
 }
 
 }  // namespace content