New TimeToFirstURLJob* navigation metrics now work with PlzNavigate.

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 754 matching lines @@
     // navigation may start in a renderer and commit in another one.
     // TODO(clamy): See if the navigation start time should be measured in the
     // renderer and sent to the browser instead of being measured here.
     scoped_ptr<NavigationRequest> scoped_request(new NavigationRequest(
         frame_tree_node, common_params,
         CommitNavigationParams(PageState(), false, base::TimeTicks::Now()),
         nullptr));
     navigation_request = scoped_request.get();
     navigation_request_map_.set(
         frame_tree_node->frame_tree_node_id(), scoped_request.Pass());
+    navigation_data_.reset();

[davidben, 2015/02/02 22:33:57]

Does this also need an IsMainFrame guard?

[carlosk, 2015/02/03 11:00:04]

No because here we actually want it to be reset. This if-block handles
renderer-initiated navigations which we don't want to track.

[clamy, 2015/02/03 12:13:54]

I think we should have one for the case where the user starts a browser intiated
navigation in the main frame and a renderer initiated navigation starts in a
subframe. We don't want to discard the data coming from the main frame
navigation that will eventually commit.

[carlosk, 2015/02/03 12:48:34]

You are both right indeed, sorry. Done.

   }
   DCHECK(navigation_request);
 
   // Update the referrer with the one received from the renderer.
   navigation_request->common_params().referrer = common_params.referrer;
 
   scoped_ptr<NavigationRequestInfo> info(new NavigationRequestInfo(params));
 
   info->first_party_for_cookies =
       frame_tree_node->IsMainFrame()
@@ skipping 41 matching lines @@
   render_frame_host->CommitNavigation(response, body.Pass(),
                                       navigation_request->common_params(),
                                       navigation_request->commit_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());
+  if (frame_tree_node->IsMainFrame())
+    navigation_data_.reset();
   // TODO(carlosk): move this cleanup into the NavigationRequest destructor once
   // we properly cancel ongoing navigations.
   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());
 }
@@ skipping 91 matching lines @@
 void NavigatorImpl::RecordNavigationMetrics(
     const LoadCommittedDetails& details,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     SiteInstance* site_instance) {
   DCHECK(site_instance->HasProcess());
 
   if (!details.is_in_page)
     RecordAction(base::UserMetricsAction("FrameLoad"));
 
   if (!details.is_main_frame || !navigation_data_ ||
+      navigation_data_->url_job_start_time_.is_null() ||

[carlosk, 2015/01/30 14:01:47]

I realized that if for some reason NavigatorImpl::LogResourceRequestTime was not
called before commit then |navigation_data_->url_job_start_time_| would not be
set and we would record invalid values. Just in case, I'm updating
NavigatorImpl::RecordNavigationMetrics so that it doesn't try to record anything
if it was not set.

       navigation_data_->url_ != params.original_request_url) {
     return;
   }
 
   base::TimeDelta time_to_commit =
       base::TimeTicks::Now() - navigation_data_->start_time_;
   UMA_HISTOGRAM_TIMES("Navigation.TimeToCommit", time_to_commit);
 
   time_to_commit -= navigation_data_->before_unload_delay_;
   base::TimeDelta time_to_network = navigation_data_->url_job_start_time_ -
@@ skipping 24 matching lines @@
         "Navigation.TimeToCommit_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_commit);
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToURLJobStart_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_network);
   }
   navigation_data_.reset();
 }
 
 }  // namespace content