Add UMA histograms to AsyncResourceHandler to track content size.

content/browser/loader/async_resource_handler.cc

Index: content/browser/loader/async_resource_handler.cc
diff --git a/content/browser/loader/async_resource_handler.cc b/content/browser/loader/async_resource_handler.cc
index d55bc0f9a72c14c08eeacd3c26729415dee696b6..bcfd5fddaff5da140b86ffabd7e1a3c96c3e7a41 100644
--- a/content/browser/loader/async_resource_handler.cc
+++ b/content/browser/loader/async_resource_handler.cc
@@ -72,6 +72,13 @@ void InitializeResourceBufferConstants() {
   GetNumericArg("resource-buffer-max-allocation-size", &kMaxAllocationSize);
 }
 
+enum ExpectedContentSize {

[mmenke, 2017/01/11 20:25:19]

"enum class" should generally be preferred to just enums

[maksims (do not use this acc), 2017/01/12 09:03:03]

invalid operands to binary expression ('content::(anonymous
namespace)::ExpectedContentSize' and 'int')

Macro includes arithmetic operations, which cannot be done on enums.

[mmenke, 2017/01/12 18:04:10]

Ah, right, good point.

+  CORRESPONDS_TO_RESPONSE_BODY = 0,

[mmenke, 2017/01/11 20:25:20]

EQ_RESPONSE_BODY?

[maksims (do not use this acc), 2017/01/13 12:05:18]

Done.

+  LT_RESPONSE_BODY = 1,
+  MT_RESPONSE_BODY = 2,
+  EXPECTED_CONTENT_SIZE,

[mmenke, 2017/01/11 20:25:19]

nit:  EXPECTED_CONTENT_MAX is more common

[maksims (do not use this acc), 2017/01/13 12:05:17]

Done.

+};
+
 }  // namespace
 
 // Used when kOptimizeLoadingIPCForSmallResources is enabled.
@@ -199,6 +206,7 @@ AsyncResourceHandler::AsyncResourceHandler(
       rdh_(rdh),
       pending_data_count_(0),
       allocation_size_(0),
+      total_read_bytes_(0),
       did_defer_(false),
       has_checked_for_sufficient_resources_(false),
       sent_received_response_msg_(false),
@@ -395,6 +403,8 @@ bool AsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
 
   buffer_->ShrinkLastAllocation(bytes_read);
 
+  total_read_bytes_ += bytes_read;
+
   if (!sent_data_buffer_msg_) {
     base::SharedMemoryHandle handle = base::SharedMemory::DuplicateHandle(
         buffer_->GetSharedMemory().handle());
@@ -551,6 +561,42 @@ void AsyncResourceHandler::RecordHistogram() {
   }
 
   inlining_helper_->RecordHistogram(elapsed_time);
+
+  // Record if content size is known in advance.

[mmenke, 2017/01/11 20:25:20]

nit:  is -> was (Since this is recorded at the end of a request).

[maksims (do not use this acc), 2017/01/13 12:05:18]

Done.

+  int64_t expected_content_size = request()->GetExpectedContentSize();
+  if (expected_content_size > 0) {

[mmenke, 2017/01/11 20:25:19]

nit:  >= 0 (We use -1 to mean unknown, 0 means the body is known to be empty)

[maksims (do not use this acc), 2017/01/13 12:05:17]

Done.

+    UMA_HISTOGRAM_BOOLEAN(
+        "Net.ResourceLoader.ExpectedContentSize.IsKnown",
+        true);
+    // Compare response body size to expected content size.
+    if (expected_content_size == total_read_bytes_) {

[mmenke, 2017/01/11 20:25:19]

Should we separate out those where expected_content_size is >= kBufferSize?

[maksims (do not use this acc), 2017/01/12 09:03:02]

Do you think I should add another enum entry, which will be something like
"EQ_RESPONSE_BODY_MT_EQ_BUFFER_SIZE"?

[mmenke, 2017/01/12 18:04:10]

I'm fine with that.  I'm not sure how much it gets us, but it can't hurt, and
better to have it and not need it than need it and not have it.

[maksims (do not use this acc), 2017/01/13 12:05:17]

Done.

+      UMA_HISTOGRAM_ENUMERATION(

[mmenke, 2017/01/11 20:25:19]

These UMA_HISTOGRAM_ENUMERATION are deceivingly expensive.  It would reduce
binary size to decide on the correct value, and then only have one call to add
an entry to the Net.ResourceLoader.ExpectedContentSize.CorrespondsBodySize
histogram.

[maksims (do not use this acc), 2017/01/13 12:05:18]

I see. Do you think I should use something else?

[mmenke, 2017/01/13 16:01:36]

No, but you should only have one instance of "UMA_HISTOGRAM_ENUMERATION".

+          "Net.ResourceLoader.ExpectedContentSize.CorrespondsBodySize",
+          ExpectedContentSize::CORRESPONDS_TO_RESPONSE_BODY,
+          ExpectedContentSize::EXPECTED_CONTENT_SIZE);
+    } else if (expected_content_size < total_read_bytes_) {
+      UMA_HISTOGRAM_ENUMERATION(
+          "Net.ResourceLoader.ExpectedContentSize.CorrespondsBodySize",
+          ExpectedContentSize::LT_RESPONSE_BODY,
+          ExpectedContentSize::EXPECTED_CONTENT_SIZE);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(
+          "Net.ResourceLoader.ExpectedContentSize.CorrespondsBodySize",
+          ExpectedContentSize::MT_RESPONSE_BODY,
+          ExpectedContentSize::EXPECTED_CONTENT_SIZE);
+    }
+  } else {
+    UMA_HISTOGRAM_BOOLEAN(
+        "Net.ResourceLoader.ExpectedContentSize.IsKnown",
+        false);

[mmenke, 2017/01/11 20:25:19]

Do we really need two histograms?  Seems like we could just make an
ExpectedContentSize::UNKNOWN and add it to the previous histogram.  This reducse
histogram upload size, and reduces data on the servers (Each recorded datapoint
is a database entry, I believe).  So unless we get something from additional
histograms we can't get from just using a calculator and a smaller number of
histograms, best to use fewer histograms.

[maksims (do not use this acc), 2017/01/12 09:03:03]

sounds reasonable.

[maksims (do not use this acc), 2017/01/13 12:05:17]

Done.

+  }
+
+  // Record how many times total size of read body is less or more/eq than
+  // buffer.
+  UMA_HISTOGRAM_BOOLEAN(
+      "Net.ResourceLoader.TotalBodySize.LT_AsyncHandlerBufferSize",
+      total_read_bytes_ < kBufferSize);

[mmenke, 2017/01/11 20:25:20]

I don't think we need this - can just add up the values of
ExpectedContentSize::CORRESPONDS_TO_RESPONSE_BODY and
ExpectedContentSize::LT_RESPONSE_BODY.

[maksims (do not use this acc), 2017/01/12 09:03:02]

But we'll loose data, which tells us how accurate expected content size is
comparing to total body size, then.

[mmenke, 2017/01/12 18:04:10]

Sorry, I wasn't clear.  I mean keep them as separate buckets in the histogram,
but if we ever want to know what this histogram would look like, we can just add
those two buckets in the
"Net.ResourceLoader.ExpectedContentSize.CorrespondsBodySize" histogram, so we
don't need a separate histogram for them.

+  total_read_bytes_ = 0;
 }
 
 void AsyncResourceHandler::SendUploadProgress(int64_t current_position,