Add time-to-network histogram considering browser side navigation

content/browser/loader/resource_dispatcher_host_impl.cc

 // Copyright (c) 2012 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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <algorithm>
 #include <set>
@@ skipping 14 matching lines @@
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "content/browser/appcache/appcache_interceptor.h"
 #include "content/browser/appcache/chrome_appcache_service.h"
 #include "content/browser/cert_store_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/download/download_resource_handler.h"
 #include "content/browser/download/save_file_manager.h"
 #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"
 #include "content/browser/loader/detachable_resource_handler.h"
 #include "content/browser/loader/power_save_block_resource_throttle.h"
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 #include "content/browser/loader/resource_message_filter.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/loader/stream_resource_handler.h"
 #include "content/browser/loader/sync_resource_handler.h"
@@ skipping 277 matching lines @@
     DCHECK(handle);
     if (!handle)
       continue;
     // Ensure the blob and any attached shareable files survive until
     // upload completion. The |body| takes ownership of |handle|.
     const void* key = handle.get();
     body->SetUserData(key, handle.release());
   }
 }
 
+// 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
 ResourceDispatcherHost* ResourceDispatcherHost::Get() {
   return g_resource_dispatcher_host;
 }
 
 ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
     : save_file_manager_(new SaveFileManager()),
       request_id_(-1),
@@ skipping 539 matching lines @@
   }
 
   filter_ = NULL;
   return handled;
 }
 
 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);
 }
 
 // Begins a resource request with the given params on behalf of the specified
 // child process.  Responses will be dispatched through the given receiver. The
 // process ID is used to lookup WebContentsImpl from routing_id's in the case of
 // a request from a renderer.  request_context is the cookie/cache context to be
 // used for this request.
 //
 // If sync_result is non-null, then a SyncLoad reply will be generated, else
@@ skipping 1148 matching lines @@
 
   // Add a flag to selectively bypass the data reduction proxy if the resource
   // type is not an image.
   if (request_data.resource_type != RESOURCE_TYPE_IMAGE)
     load_flags |= net::LOAD_BYPASS_DATA_REDUCTION_PROXY;
 
   return load_flags;
 }
 
 }  // namespace content