Record Data Savings for Client-Side LoFi

components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

Index: components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
diff --git a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
index 9727090d5115ad2b6a9300d20e5ee3b9450ccffb..d24c18602456d42b6940cd291a27b581e0957128 100644
--- a/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
+++ b/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
@@ -4,6 +4,7 @@
 
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h"
 
+#include <algorithm>
 #include <limits>
 #include <utility>
 
@@ -117,18 +118,48 @@ void RecordContentLengthHistograms(bool lofi_low_header_added,
                           received_content_length);
 }
 
-// Given a |request| that went through the Data Reduction Proxy, this function
-// estimates how many bytes would have been received if the response had been
-// received directly from the origin using HTTP/1.1 with a content length of
-// |adjusted_original_content_length|.
-int64_t EstimateOriginalReceivedBytes(const net::URLRequest& request) {
+// Estimate the size of the original headers of |request|. If |used_drp| is
+// true, then it's assumed that the original request would have used HTTP/1.1,
+// otherwise it assumes that the original request would have used the same
+// protocol as |request| did. This is to account for stuff like HTTP/2 header
+// compression.
+int64_t EstimateOriginalHeaderBytes(const net::URLRequest& request,
+                                    bool used_drp) {
+  if (used_drp) {
+    // TODO(sclittle): Remove headers added by Data Reduction Proxy when
+    // computing original size. https://crbug.com/535701.
+    return request.response_headers()->raw_headers().size();
+  }
+  return std::max<int64_t>(0, request.GetTotalReceivedBytes() -

[RyanSturm, 2017/05/10 18:14:08]

As a note, I don't believe GetTotalReceivedBytes counts previous jobs (cache
revalidation, redirects) can you double check and add that information to
https://crbug.com/535701?

[sclittle, 2017/05/10 19:56:36]

From looking at URLRequest::GetTotalReceivedBytes(), it looks like you're right
that it doesn't include redirects, but it does seem to include revalidations
(running totals are saved from previous network transactions:
https://cs.chromium.org/chromium/src/net/http/http_cache_transaction.cc?l=2778).

Even if GetTotalReceivedBytes() is changed in the future to include the
redirect, it'll be fine since that way both the received size and the original
size used for data savings calculations will include the size of the redirect.
So that should just work.

Also, IIRC successful revalidations (i.e. 304 response) are marked as
was_cached, so they'll be handled by the if(request.was_cached()) check in
EstimateOriginalReceivedBytes below, so they don't get to this function.

+                                  request.received_response_content_length());
+}
+
+// Given a |request| that went through the Data Reduction Proxy if |used_drp| is
+// true, this function estimates how many bytes would have been received if the
+// response had been received directly from the origin without any data saver
+// optimizations.
+int64_t EstimateOriginalReceivedBytes(const net::URLRequest& request,
+                                      bool used_drp,
+                                      const LoFiDecider* lofi_decider) {
   if (request.was_cached() || !request.response_headers())
     return request.GetTotalReceivedBytes();
 
-  // TODO(sclittle): Remove headers added by Data Reduction Proxy when computing
-  // original size. http://crbug/535701.
-  return request.response_headers()->raw_headers().size() +
-         util::CalculateEffectiveOCL(request);
+  if (lofi_decider) {
+    if (lofi_decider->IsClientLoFiAutoReloadRequest(request))
+      return 0;
+
+    int64_t first, last, length;
+    if (lofi_decider->IsClientLoFiImageRequest(request) &&
+        request.response_headers()->GetContentRangeFor206(&first, &last,
+                                                          &length) &&
+        length > request.received_response_content_length()) {
+      return EstimateOriginalHeaderBytes(request, used_drp) + length;
+    }
+  }
+
+  return used_drp ? EstimateOriginalHeaderBytes(request, used_drp) +
+                        util::CalculateEffectiveOCL(request)
+                  : request.GetTotalReceivedBytes();
 }
 
 // Verifies that the chrome proxy related request headers are set correctly.
@@ -451,10 +482,11 @@ void DataReductionProxyNetworkDelegate::CalculateAndRecordDataUsage(
 
   // Estimate how many bytes would have been used if the DataReductionProxy was
   // not used, and record the data usage.
-  int64_t original_size = data_used;
-
-  if (request_type == VIA_DATA_REDUCTION_PROXY)
-    original_size = EstimateOriginalReceivedBytes(request);
+  int64_t original_size = EstimateOriginalReceivedBytes(
+      request, request_type == VIA_DATA_REDUCTION_PROXY,
+      data_reduction_proxy_io_data_
+          ? data_reduction_proxy_io_data_->lofi_decider()
+          : nullptr);
 
   std::string mime_type;
   if (request.response_headers())