Add time-to-network histogram considering browser side navigation

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index c49bfe9a4c3b6431a9fe5a5ae1210af8e5608e0f..080c3960f82272ec81136106e131a4526e66d624 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -32,6 +32,7 @@
 #include "content/browser/download/save_file_resource_handler.h"
 #include "content/browser/fileapi/chrome_blob_storage_context.h"
 #include "content/browser/frame_host/navigation_request_info.h"
+#include "content/browser/frame_host/navigator.h"
 #include "content/browser/loader/async_resource_handler.h"
 #include "content/browser/loader/buffered_resource_handler.h"
 #include "content/browser/loader/cross_site_resource_handler.h"
@@ -329,6 +330,24 @@ void AttachRequestBodyBlobDataHandles(
   }
 }
 
+// PlzNavigate
+// This method is called in the UI thread to send the timestamp of a resource
+// request to the respective Navigator (for an UMA histogram).
+void LogResourceRequestTimeOnUI(
+    base::TimeTicks timestamp,
+    int render_process_id,
+    int render_frame_id,
+    const GURL& url) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  RenderFrameHostImpl* host =
+      RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
+  if (host != NULL) {
+    DCHECK(host->frame_tree_node()->IsMainFrame());
+    host->frame_tree_node()->navigator()->LogResourceRequestTime(
+        timestamp, url);
+  }
+}
+
 }  // namespace
 
 // static
@@ -888,6 +907,19 @@ void ResourceDispatcherHostImpl::OnRequestResource(
     int routing_id,
     int request_id,
     const ResourceHostMsg_Request& request_data) {
+  // When logging time-to-network only cares about main frame and non-transfer

[nasko, 2014/09/26 00:10:35]

nit: s/cares/care/
Why do we not care about transfers?

[clamy, 2014/09/26 14:48:32]

I think we only want the first request for transfers, not the second one that is
made by the second renderer. I thought that checking we did not have a
transferred_request_request_id would eliminate that case.

[carlosk, 2014/09/26 15:50:25]

Done.

From my understanding of what we're tracking: because we want to measure only
the time it took from a navigation request being started until only the 1st
network request it caused, which is the delay we will be able to optimize with
PlzNavigate. Measuring later ones due to a possible transfer doesn't seem useful
in this context (we're not optimizing that). Does this sounds reasonable?

[nasko, 2014/09/26 17:34:18]

Acknowledged.

+  // navigations.
+  if (request_data.resource_type == RESOURCE_TYPE_MAIN_FRAME &&
+      request_data.transferred_request_request_id == -1) {

[davidben, 2014/09/26 15:42:45]

Aside: There's a slight weirdness that this'll actually hit for the future blob
URL fetch too, but the URL won't match, so it should be fine. We'll be able to
correlate them better in the PlzNavigate version, but having one codepath work
for both versions right now sounds like a good plan.

[carlosk, 2014/09/26 15:52:17]

Acknowledged.

+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&LogResourceRequestTimeOnUI,
+                   TimeTicks::Now(),
+                   filter_->child_id(),
+                   request_data.render_frame_id,
+                   request_data.url));
+  }
   BeginRequest(request_id, request_data, NULL, routing_id);
 }