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

net/http/http_cache_transaction.h

 // 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.
 
 // This file declares HttpCache::Transaction, a private class of HttpCache so
 // it should only be included by http_cache.cc
 
 #ifndef NET_HTTP_HTTP_CACHE_TRANSACTION_H_
 #define NET_HTTP_HTTP_CACHE_TRANSACTION_H_
 
 #include <string>
 
 #include "base/time.h"
 #include "net/base/completion_callback.h"
 #include "net/base/net_log.h"
 #include "net/base/request_priority.h"

[gavinp, 2013/05/10 20:23:03]

#include "net/base/load_timing_info.h"

[mmenke, 2013/05/10 20:44:00]

Since we just have a scoped_ptr, forward declared it instead.

 #include "net/http/http_cache.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_transaction.h"
 
 namespace net {
 
 class PartialData;
 struct HttpRequestInfo;
 class HttpTransactionDelegate;
@@ skipping 351 matching lines @@
 
   // Resets cache_io_start_ to the current time, if |return_value| is
   // ERR_IO_PENDING.
   // Returns |return_value|.
   int ResetCacheIOStart(int return_value);
 
   void ScheduleDelayedLoop(base::TimeDelta delay, int result);
   void RunDelayedLoop(base::TimeTicks delay_start_time,
                       base::TimeDelta intended_delay, int result);
 
+  // Updates |last_network_trans_load_timing_| from |network_trans_|, which must
+  // be non-NULL.  Only called before deleting the old transaction.

[gavinp, 2013/05/10 20:23:03]

I think "old network transaction" would make things clearer for me, as there's
three sorts of transactions I can think about here.

[mmenke, 2013/05/10 20:44:00]

Done.  I agree that "old" is clearer.

+  void CacheLoadTimingInformation();

[gavinp, 2013/05/10 20:23:03]

In that vein, using "Cache" here to mean "make a copy of and keep" instead of
"the http disk cache" is... Confusing.

I propose:

CopyLoadTimingInformationFromNetworkTransaction().

Or even:

ResetNetworkTransaction();

(and move network_trans_.reset() into the method).

[mmenke, 2013/05/10 20:44:00]

Switched to ResetNetworkTransaction, since we always call them together.  For
some reason, was thinking there was a cleanup function where updating the cached
copy wasn't needed.

+
   State next_state_;
   const HttpRequestInfo* request_;
   RequestPriority priority_;
   BoundNetLog net_log_;
   scoped_ptr<HttpRequestInfo> custom_request_;
   HttpRequestHeaders request_headers_copy_;
   // If extra_headers specified a "if-modified-since" or "if-none-match",
   // |external_validation_| contains the value of those headers.
   ValidationHeaders external_validation_;
   base::WeakPtr<HttpCache> cache_;
@@ skipping 42 matching lines @@
   // a delay on return, we must defer that delay until AddToEntry has been
   // called, to avoid a race condition on the address returned.
   base::TimeDelta deferred_cache_sensitivity_delay_;
   bool defer_cache_sensitivity_delay_;
 
   // For sensitivity analysis, the simulated increase in cache service times,
   // in percent.
   int sensitivity_analysis_percent_increase_;
 
   HttpTransactionDelegate* transaction_delegate_;
+
+  // Load timing information for the last network request, if any.  Set in the
+  // 304 and 206 response cases, as the network transaction may be destroyed
+  // before the caller requests load timing information.
+  scoped_ptr<LoadTimingInfo> last_network_trans_load_timing_;
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_CACHE_TRANSACTION_H_