Navigation: Move out NavigationHandleImpl's constructor and destructor.

Remove the manual reset of navigation_handle in NavigationRequest's
destructor.

Bug: 916537
Change-Id: Iddab8818f172ae1d0679e8e68619895f4ed861d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776057Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Mohamed Abdelhalim <zetamoo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692020}

content/browser/frame_host/navigation_handle_impl.cc

@@ -33,36 +33,9 @@ namespace content {
 
 NavigationHandleImpl::NavigationHandleImpl(
     NavigationRequest* navigation_request)
-    : navigation_request_(navigation_request) {
-  const GURL& url = navigation_request_->common_params().url;
-  TRACE_EVENT_ASYNC_BEGIN2("navigation", "NavigationHandle", this,
-                           "frame_tree_node", GetFrameTreeNodeId(), "url",
-                           url.possibly_invalid_spec());
-  DCHECK(!navigation_request_->common_params().navigation_start.is_null());
-  DCHECK(!IsRendererDebugURL(url));
-
-  if (IsInMainFrame()) {
-    TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
-        "navigation", "Navigation StartToCommit", this,
-        navigation_request_->common_params().navigation_start, "Initial URL",
-        url.spec());
-  }
-
-  if (IsSameDocument()) {
-    TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationHandle", this,
-                                 "Same document");
-  }
-}
-
-NavigationHandleImpl::~NavigationHandleImpl() {
-  if (IsInMainFrame()) {
-    TRACE_EVENT_ASYNC_END2("navigation", "Navigation StartToCommit", this,
-                           "URL",
-                           navigation_request_->common_params().url.spec(),
-                           "Net Error Code", GetNetErrorCode());
-  }
-  TRACE_EVENT_ASYNC_END0("navigation", "NavigationHandle", this);
-}
+    : navigation_request_(navigation_request) {}
+
+NavigationHandleImpl::~NavigationHandleImpl() = default;
 
 int64_t NavigationHandleImpl::GetNavigationId() {
   return navigation_request_->navigation_handle_id();

content/browser/frame_host/navigation_request.cc

@@ -943,12 +943,10 @@ NavigationRequest::~NavigationRequest() {
     navigation_handle_proxy_->DidFinish();
 #endif
 
-  if (navigation_handle())
+  if (navigation_handle()) {
     GetDelegate()->DidFinishNavigation(navigation_handle());
-
-  // This is done manually here because the NavigationHandleImpl uses
-  // NavigationRequest in its destructor.
-  navigation_handle_.reset();
+    TraceNavigationHandleEnd();
+  }
 }
 
 void NavigationRequest::BeginNavigation() {
@@ -1166,7 +1164,9 @@ void NavigationRequest::CreateNavigationHandle(bool is_for_commit) {
     return;
   }
 
+  DCHECK(!navigation_handle_);
   navigation_handle_ = std::move(navigation_handle);
+  TraceNavigationHandleStart();
 
   throttle_runner_ = base::WrapUnique(
       new NavigationThrottleRunner(this, navigation_handle_.get()));
@@ -1196,6 +1196,7 @@ void NavigationRequest::ResetForCrossDocumentRestart() {
   // |navigation_handle_proxy_|. See https://crbug.com/958396.
   if (navigation_handle()) {
     GetDelegate()->DidFinishNavigation(navigation_handle());
+    TraceNavigationHandleEnd();
     navigation_handle_.reset();
   }
 
@@ -3402,4 +3403,37 @@ bool NavigationRequest::HasPrefetchedAlternativeSubresourceSignedExchange() {
   return !commit_params_->prefetched_signed_exchanges.empty();
 }
 
+void NavigationRequest::TraceNavigationHandleStart() {
+  DCHECK(navigation_handle_);
+  TRACE_EVENT_ASYNC_BEGIN2("navigation", "NavigationHandle",
+                           navigation_handle_.get(), "frame_tree_node",
+                           GetFrameTreeNodeId(), "url",
+                           common_params_->url.possibly_invalid_spec());
+  DCHECK(!common_params_->navigation_start.is_null());
+  DCHECK(!IsRendererDebugURL(common_params_->url));
+
+  if (IsInMainFrame()) {
+    TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
+        "navigation", "Navigation StartToCommit", navigation_handle_.get(),
+        common_params_->navigation_start, "Initial URL",
+        common_params_->url.spec());
+  }
+
+  if (IsSameDocument()) {
+    TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationHandle",
+                                 navigation_handle_.get(), "Same document");
+  }
+}
+
+void NavigationRequest::TraceNavigationHandleEnd() {
+  DCHECK(navigation_handle_);
+  if (IsInMainFrame()) {
+    TRACE_EVENT_ASYNC_END2(
+        "navigation", "Navigation StartToCommit", navigation_handle_.get(),
+        "URL", common_params_->url.spec(), "Net Error Code", net_error_);
+  }
+  TRACE_EVENT_ASYNC_END0("navigation", "NavigationHandle",
+                         navigation_handle_.get());
+}
+
 }  // namespace content

content/browser/frame_host/navigation_request.h

@@ -838,6 +838,10 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
     return std::move(modified_request_headers_);
   }
 
+  // Helper functions to trace the start and end of |navigation_handle_|.
+  void TraceNavigationHandleStart();
+  void TraceNavigationHandleEnd();
+
   FrameTreeNode* frame_tree_node_;
 
   // Invariant: At least one of |loader_| or |render_frame_host_| is null.