net: Return LoadTiming information in the case of a cache hit

net/url_request/url_request.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.
 
 #include "net/url_request/url_request.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
@@ skipping 1016 matching lines @@
     delegate_->OnReadCompleted(this, bytes_read);
 
   // Nothing below this line as OnReadCompleted may delete |this|.
 }
 
 void URLRequest::OnHeadersComplete() {
   // Cache load timing information now, as information will be lost once the
   // socket is closed and the ClientSocketHandle is Reset, which will happen
   // once the body is complete.  The start times should already be populated.
   if (job_) {
+    // Keep a copy of the two times the URLRequest sets.
+    base::TimeTicks request_start = load_timing_info_.request_start;
+    base::Time request_start_time = load_timing_info_.request_start_time;

[mmenke, 2013/05/10 18:32:26]

I'm not a big fan of this, but other solutions seem worse a little worse:

1)  Have HttpCache::Transaction::GetLoadTimingInfo back up the values it
overwrites (Seems more regression prone than this, though this isn't great in
that respect, either).

2)  Keep the times in separate variables, and use them in
URLRequest::GetLoadTimingInfo (Can't just use them here, since I believe some
request types skip this notification).  Seems a little weird to pluck them from
a different location in GetLoadTimingInfo().

+
+    // Clear load times.  Shouldn't be neded, but gives the GetLoadTimingInfo a
+    // consistent place to start from.
+    load_timing_info_ = LoadTimingInfo();
     job_->GetLoadTimingInfo(&load_timing_info_);
+
+    load_timing_info_.request_start = request_start;
+    load_timing_info_.request_start_time = request_start_time;
+
     ConvertRealLoadTimesToBlockingTimes(&load_timing_info_);
   }
 }
 
 void URLRequest::NotifyRequestCompleted() {
   // TODO(battre): Get rid of this check, according to willchan it should
   // not be needed.
   if (has_notified_completion_)
     return;
 
@@ skipping 27 matching lines @@
       new base::debug::StackTrace(NULL, 0);
   *stack_trace_copy = stack_trace;
   stack_trace_.reset(stack_trace_copy);
 }
 
 const base::debug::StackTrace* URLRequest::stack_trace() const {
   return stack_trace_.get();
 }
 
 }  // namespace net