DataReductionProxy.BypassedBytes.* counts all triggering request bytes

components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.cc

 // Copyright 2014 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 "components/data_reduction_proxy/browser/data_reduction_proxy_usage_stats.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/metrics/histogram.h"
@@ skipping 180 matching lines @@
 
   if (data_reduction_proxy_enabled.GetValue() &&
       data_reduction_proxy_params_->IsBypassedByDataReductionProxyLocalRules(
           request, data_reduction_proxy_config)) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::LOCAL_BYPASS_RULES,
                         content_length);
     return;
   }
 
-  if (triggering_request_) {
+  // We only record separate triggering request UMA for short, medium, and long

[bengr, 2014/09/19 17:23:33]

Don't use "We" in comments. Change to:

Triggering request UMA are recorded separately only for short, medium and long
bypass events. 

[megjablon, 2014/09/19 17:58:02]

Done.

+  // bypass events.
+  if (triggering_request_ &&
+     (last_bypass_type_ ==  BYPASS_EVENT_TYPE_SHORT ||
+      last_bypass_type_ ==  BYPASS_EVENT_TYPE_MEDIUM ||
+      last_bypass_type_ ==  BYPASS_EVENT_TYPE_LONG)) {
     // We only record when audio or video triggers a bypass. We don't care

[bengr, 2014/09/19 17:23:33]

Remove the "We"s in this comment too.

[megjablon, 2014/09/19 17:58:03]

Done.

     // about audio and video bypassed as collateral damage.
     std::string mime_type;
     request.GetMimeType(&mime_type);
     // MIME types are named by <media-type>/<subtype>. We check to see if the
     // media type is audio or video.

[bengr, 2014/09/19 17:23:33]

Add a comment that states that we only have a bucket for short bypasses due to
a/v.

[megjablon, 2014/09/19 17:58:02]

Done.

-    if (mime_type.compare(0, 6, "audio/") == 0  ||
-        mime_type.compare(0, 6, "video/") == 0) {
+    if (last_bypass_type_ ==  BYPASS_EVENT_TYPE_SHORT &&
+       (mime_type.compare(0, 6, "audio/") == 0  ||
+        mime_type.compare(0, 6, "video/") == 0)) {
       RecordBypassedBytes(last_bypass_type_,

[bengr, 2014/09/19 17:23:33]

This UMA is complicated enough that we should have unit tests for all of it.
Please file a bug with the label Cr-Internals-Network-DataProxy and assign
yourself.

[megjablon, 2014/09/19 17:58:02]

Done.

                           DataReductionProxyUsageStats::AUDIO_VIDEO,
                           content_length);
       return;
     }
 
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::TRIGGERING_REQUEST,
                         content_length);
     triggering_request_ = false;
     return;
   }
 
   if (last_bypass_type_ != BYPASS_EVENT_TYPE_MAX) {
     RecordBypassedBytes(last_bypass_type_,
                         DataReductionProxyUsageStats::BYPASSED_BYTES_TYPE_MAX,
                         content_length);
     return;
   }
 
   if (data_reduction_proxy_params_->
           AreDataReductionProxiesBypassed(request, NULL)) {

[bengr, 2014/09/19 17:23:33]

Can the line break be before "request" instead of after "->"?

[megjablon, 2014/09/19 17:58:03]

Done.

-      RecordBypassedBytes(last_bypass_type_,
-                          DataReductionProxyUsageStats::NETWORK_ERROR,
-                          content_length);
+    RecordBypassedBytes(last_bypass_type_,
+                        DataReductionProxyUsageStats::NETWORK_ERROR,
+                        content_length);
   }
 }
 
 void DataReductionProxyUsageStats::RecordBypassEventHistograms(
     const net::ProxyServer& bypassed_proxy,
     int net_error) const {
   DataReductionProxyTypeInfo data_reduction_proxy_info;
   if (bypassed_proxy.is_valid() && !bypassed_proxy.is_direct() &&
       data_reduction_proxy_params_->IsDataReductionProxy(
       bypassed_proxy.host_port_pair(), &data_reduction_proxy_info)) {
@@ skipping 124 matching lines @@
         default:
           break;
       }
       break;
   }
 }
 
 }  // namespace data_reduction_proxy