Navigation start to commit trace

BUG=part of 585134
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

This is a part of the design for 585134 which includes a larger set of traces.
As part of that design we also noticed overlapping traces that existed earlier
for navigation start to commit. As discussed with carlosk@, this change also
reverts the earlier traces which only covered PlzNavigate and adds the new
traces that cover the following scenarios:
- plzNavigate and traditional flows
- committed and aborted/failed/canceled loads

This patch adds the async begin and end traces in the constructor and
destructor of NavigationHandleImpl respectively. That would cover all of
the scenarios as detailed in the design doc
(https://docs.google.com/a/chromium.org/document/d/1yM2khCxLC5AV3X4y7cyuCNCFHCmq9CpqQzDwK6MROaw/edit?usp=sharing).
The start time of Navigation is passed in the
constructor and that would be used as the trace start timestamp. For
failed/aborted provisional loads, the argument net error code will give the error
code.

In this patch I am adding them in the existing "navigation" category only and
later they might be extended for new categories like "loading".

Review URL: https://codereview.chromium.org/1825533002

Cr-Commit-Position: refs/heads/master@{#389881}

content/browser/frame_host/navigation_handle_impl.cc

@@ -67,6 +67,12 @@ NavigationHandleImpl::NavigationHandleImpl(
       pending_nav_entry_id_(pending_nav_entry_id) {
   DCHECK(!navigation_start.is_null());
   GetDelegate()->DidStartNavigation(this);
+
+  if (IsInMainFrame()) {
+    TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
+        "navigation", "Navigation StartToCommit", this,
+        navigation_start.ToInternalValue(), "Initial URL", url_.spec());
+  }
 }
 
 NavigationHandleImpl::~NavigationHandleImpl() {
@@ -76,6 +82,12 @@ NavigationHandleImpl::~NavigationHandleImpl() {
   // destroyed in the middle of the NavigationThrottles checks.
   if (!IsBrowserSideNavigationEnabled() && !complete_callback_.is_null())
     RunCompleteCallback(NavigationThrottle::CANCEL_AND_IGNORE);
+
+  if (IsInMainFrame()) {
+    TRACE_EVENT_ASYNC_END2("navigation", "Navigation StartToCommit", this,
+                           "URL", url_.spec(), "Net Error Code",
+                           net_error_code_);
+  }
 }
 
 NavigatorDelegate* NavigationHandleImpl::GetDelegate() const {

content/browser/frame_host/navigator_impl.cc

@@ -322,7 +322,7 @@ bool NavigatorImpl::NavigateToEntry(
         frame_tree_node->navigation_request()) {
       // TODO(carlosk): extend these traces to support subframes and
       // non-PlzNavigate navigations.
-      // For these traces below we're using the navigation handle as the async
+      // For the trace below we're using the navigation handle as the async
       // trace id, |navigation_start| as the timestamp and reporting the
       // FrameTreeNode id as a parameter. For navigations where no network
       // request is made (data URLs, JavaScript URLs, etc) there is no handle
@@ -332,11 +332,6 @@ bool NavigatorImpl::NavigateToEntry(
           frame_tree_node->navigation_request()->navigation_handle(),
           navigation_start.ToInternalValue(),
           "FrameTreeNode id", frame_tree_node->frame_tree_node_id());
-      TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
-          "navigation", "Navigation timeToCommit",
-          frame_tree_node->navigation_request()->navigation_handle(),
-          navigation_start.ToInternalValue(),
-          "FrameTreeNode id", frame_tree_node->frame_tree_node_id());
     }
 
   } else {
@@ -574,12 +569,6 @@ void NavigatorImpl::DidNavigate(
   if (!params.url_is_unreachable)
     render_frame_host->set_last_successful_url(params.url);
 
-  if (did_navigate && render_frame_host->frame_tree_node()->IsMainFrame() &&
-      IsBrowserSideNavigationEnabled()) {
-    TRACE_EVENT_ASYNC_END0("navigation", "Navigation timeToCommit",
-                           render_frame_host->navigation_handle());
-  }
-
   // Send notification about committed provisional loads. This notification is
   // different from the NAV_ENTRY_COMMITTED notification which doesn't include
   // the actual URL navigated to and isn't sent for AUTO_SUBFRAME navigations.