(DEPRECATED) Send navigation_start to browser process in DidStartProvisionalLoad

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 84891997f0ec8c558433ddf78ed728162c07047c..4c46aadf142066626a46c3bf2a3a761af0fe8b22 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -137,6 +137,7 @@
 #include "third_party/WebKit/public/web/WebMediaStreamRegistry.h"
 #include "third_party/WebKit/public/web/WebNavigationPolicy.h"
 #include "third_party/WebKit/public/web/WebPageSerializer.h"
+#include "third_party/WebKit/public/web/WebPerformance.h"
 #include "third_party/WebKit/public/web/WebPlugin.h"
 #include "third_party/WebKit/public/web/WebPluginParams.h"
 #include "third_party/WebKit/public/web/WebRange.h"
@@ -423,6 +424,7 @@ bool IsTopLevelNavigation(WebFrame* frame) {
 }
 
 WebURLRequest CreateURLRequestForNavigation(
+    const RequestNavigationParams& request_params,
     const CommonNavigationParams& common_params,
     scoped_ptr<StreamOverrideParameters> stream_override,
     bool is_view_source_mode_enabled) {
@@ -443,41 +445,34 @@ WebURLRequest CreateURLRequestForNavigation(
   extra_data->set_stream_override(stream_override.Pass());
   request.setExtraData(extra_data);
 
-  // Set the ui timestamp for this navigation. Currently the timestamp here is
-  // only non empty when the navigation was triggered by an Android intent. The
-  // timestamp is converted to a double version supported by blink. It will be
-  // passed back to the browser in the DidCommitProvisionalLoad and the
-  // DocumentLoadComplete IPCs.
-  base::TimeDelta ui_timestamp = common_params.ui_timestamp - base::TimeTicks();
-  request.setUiStartTime(ui_timestamp.InSecondsF());
-  request.setInputPerfMetricReportPolicy(
-      static_cast<WebURLRequest::InputToLoadPerfMetricReportPolicy>(
-          common_params.report_type));
-  return request;
-}
-
-void UpdateFrameNavigationTiming(WebFrame* frame,
-                                 base::TimeTicks browser_navigation_start,
-                                 base::TimeTicks renderer_navigation_start) {
   // The browser provides the navigation_start time to bootstrap the
   // Navigation Timing information for the browser-initiated navigations. In
   // case of cross-process navigations, this carries over the time of
   // finishing the onbeforeunload handler of the previous page.
-  DCHECK(!browser_navigation_start.is_null());
-  if (frame->provisionalDataSource()) {
+  // TODO(csharrison) should we only do this for WebFrameLoadType::Standard?
+  if (!request_params.browser_navigation_start.is_null()) {
     // |browser_navigation_start| is likely before this process existed, so we
     // can't use InterProcessTimeTicksConverter. We need at least to ensure
     // that the browser-side navigation start we set is not later than the one
     // on the renderer side.
     base::TimeTicks navigation_start = std::min(
-        browser_navigation_start, renderer_navigation_start);
+        request_params.browser_navigation_start, base::TimeTicks::Now());
     double navigation_start_seconds =
         (navigation_start - base::TimeTicks()).InSecondsF();
-    frame->provisionalDataSource()->setNavigationStartTime(
-        navigation_start_seconds);
-    // TODO(clamy): We need to provide additional timing values for the
-    // Navigation Timing API to work with browser-side navigations.
+    request.setBrowserNavigationStartTime(navigation_start_seconds);

[clamy, 2015/10/28 12:40:01]

Instead of adding the start time to the WebURLRequest, can we add it to the
RequestExtraData? This is a content/class, so we can keep the timing in
base::TimeTicks without having to convert it back and forth to a double.

[Charlie Harrison, 2015/10/28 14:45:00]

Good idea! Will do that.

   }
+
+  // Set the ui timestamp for this navigation. Currently the timestamp here is
+  // only non empty when the navigation was triggered by an Android intent. The
+  // timestamp is converted to a double version supported by blink. It will be
+  // passed back to the browser in the DidCommitProvisionalLoad and the
+  // DocumentLoadComplete IPCs.
+  base::TimeDelta ui_timestamp = common_params.ui_timestamp - base::TimeTicks();
+  request.setUiStartTime(ui_timestamp.InSecondsF());
+  request.setInputPerfMetricReportPolicy(
+      static_cast<WebURLRequest::InputToLoadPerfMetricReportPolicy>(
+          common_params.report_type));
+  return request;
 }
 
 // PlzNavigate
@@ -2703,12 +2698,20 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
         ->set_transition_type(ui::PAGE_TRANSITION_AUTO_SUBFRAME);
   }
 
+  // See if we have a better timestamp from a browser initiated navigation.
+  // Otherwise use the renderer-marked timestamp.
+  DCHECK(ds->getNavigationStartTime());
+  double browser_navigation_start = ds->request().browserNavigationStartTime();
+  double navigation_start =
+      browser_navigation_start ? browser_navigation_start
+      : ds->getNavigationStartTime();
+
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers(),
                     DidStartProvisionalLoad(frame));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, DidStartProvisionalLoad());
 
   Send(new FrameHostMsg_DidStartProvisionalLoadForFrame(
-       routing_id_, ds->request().url()));
+       routing_id_, ds->request().url(), navigation_start));
 }
 
 void RenderFrameImpl::didReceiveServerRedirectForProvisionalLoad(
@@ -4312,14 +4315,16 @@ void RenderFrameImpl::OnFailedNavigation(
 
   // Inform the browser of the start of the provisional load. This is needed so
   // that the load is properly tracked by the WebNavigation API.
+  base::TimeDelta navigation_start =
+      (request_params.browser_navigation_start - base::TimeTicks());
   Send(new FrameHostMsg_DidStartProvisionalLoadForFrame(
-      routing_id_, common_params.url));
+      routing_id_, common_params.url, navigation_start.InSecondsF()));

[clamy, 2015/10/28 12:40:01]

nit: in PlzNavigate, you don't really care about what the renderer sends you
since you already have that information in the browser. Just send a
base::TimeTicks()?

 
   // Send the provisional load failure.
   blink::WebURLError error =
       CreateWebURLError(common_params.url, has_stale_copy_in_cache, error_code);
   WebURLRequest failed_request = CreateURLRequestForNavigation(
-      common_params, scoped_ptr<StreamOverrideParameters>(),
+      request_params, common_params, scoped_ptr<StreamOverrideParameters>(),
       frame_->isViewSourceModeEnabled());
   SendFailedProvisionalLoad(failed_request, error, frame_);
 
@@ -4655,7 +4660,8 @@ void RenderFrameImpl::NavigateInternal(
   bool should_load_request = false;
   WebHistoryItem item_for_history_navigation;
   WebURLRequest request = CreateURLRequestForNavigation(
-      common_params, stream_params.Pass(), frame_->isViewSourceModeEnabled());
+      request_params, common_params, stream_params.Pass(),
+      frame_->isViewSourceModeEnabled());
 #if defined(OS_ANDROID)
   request.setHasUserGesture(start_params.has_user_gesture);
 #endif
@@ -4780,10 +4786,6 @@ void RenderFrameImpl::NavigateInternal(
   }
 
   if (should_load_request) {
-    // Record this before starting the load. We need a lower bound of this
-    // time to sanitize the navigationStart override set below.
-    base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
-
     // Perform a navigation to a data url if needed.
     if (!common_params.base_url_for_data_url.is_empty() ||
         (browser_side_navigation &&
@@ -4794,12 +4796,12 @@ void RenderFrameImpl::NavigateInternal(
       frame_->toWebLocalFrame()->load(request, load_type,
                                       item_for_history_navigation);
     }
-
-    if (load_type == blink::WebFrameLoadType::Standard) {
-      UpdateFrameNavigationTiming(frame_,
-                                  request_params.browser_navigation_start,
-                                  renderer_navigation_start);
+    if (load_type == blink::WebFrameLoadType::Standard &&
+        frame_->provisionalDataSource()) {
+      frame_->provisionalDataSource()->setNavigationStartTime(
+          request.browserNavigationStartTime());
     }
+
   }
 
   // In case LoadRequest failed before didCreateDataSource was called.
@@ -5022,6 +5024,8 @@ void RenderFrameImpl::BeginNavigation(blink::WebURLRequest* request) {
                  GetRequestContextFrameTypeForWebURLRequest(*request) ==
                  REQUEST_CONTEXT_FRAME_TYPE_NESTED);
 
+  double navigation_start_seconds =
+      (base::TimeTicks::Now() - base::TimeTicks()).InSecondsF();
   Send(new FrameHostMsg_BeginNavigation(
       routing_id_,
       MakeCommonNavigationParams(request, should_replace_current_entry),
@@ -5029,7 +5033,8 @@ void RenderFrameImpl::BeginNavigation(blink::WebURLRequest* request) {
           request->httpMethod().latin1(), GetWebURLRequestHeaders(*request),
           GetLoadFlagsForWebURLRequest(*request), request->hasUserGesture(),
           request->skipServiceWorker(),
-          GetRequestContextTypeForWebURLRequest(*request)),
+          GetRequestContextTypeForWebURLRequest(*request),
+          navigation_start_seconds),
       GetRequestBodyForWebURLRequest(*request)));
 }