Add the actual data being read to the OnBytesRead callback, take two....

net/url_request/url_request_job.cc

 // Copyright (c) 2010 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_job.h"
 
 #include "base/histogram.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
@@ skipping 15 matching lines @@
 const int URLRequestJob::kFilterBufSize = 32 * 1024;
 
 URLRequestJob::URLRequestJob(URLRequest* request)
     : request_(request),
       prefilter_bytes_read_(0),
       postfilter_bytes_read_(0),
       is_compressible_content_(false),
       is_compressed_(false),
       done_(false),
       filter_needs_more_output_space_(false),
-      read_buffer_len_(0),
+      filtered_read_buffer_len_(0),
       has_handled_response_(false),
       expected_content_size_(-1),
       deferred_redirect_status_code_(-1),
       packet_timing_enabled_(false),
       filter_input_byte_count_(0),
       bytes_observed_in_packets_(0),
       max_packets_timed_(0),
       observed_packet_count_(0) {
   load_flags_ = request_->load_flags();
   is_profiling_ = request->enable_profiling();
@@ skipping 144 matching lines @@
 
 // This function calls ReadData to get stream data. If a filter exists, passes
 // the data to the attached filter. Then returns the output from filter back to
 // the caller.
 bool URLRequestJob::Read(net::IOBuffer* buf, int buf_size, int *bytes_read) {
   bool rv = false;
 
   DCHECK_LT(buf_size, 1000000);  // sanity check
   DCHECK(buf);
   DCHECK(bytes_read);
+  DCHECK(filtered_read_buffer_ == NULL);
+  DCHECK_EQ(0, filtered_read_buffer_len_);
 
   *bytes_read = 0;
 
   // Skip Filter if not present
   if (!filter_.get()) {
-    rv = ReadRawData(buf, buf_size, bytes_read);
-    if (rv && *bytes_read > 0)
-      RecordBytesRead(*bytes_read);
+    rv = ReadRawDataHelper(buf, buf_size, bytes_read);
   } else {
     // Save the caller's buffers while we do IO
     // in the filter's buffers.
-    read_buffer_ = buf;
-    read_buffer_len_ = buf_size;
+    filtered_read_buffer_ = buf;
+    filtered_read_buffer_len_ = buf_size;
 
     if (ReadFilteredData(bytes_read)) {
       rv = true;   // we have data to return
     } else {
       rv = false;  // error, or a new IO is pending
     }
   }
   if (rv && *bytes_read == 0)
     NotifyDone(URLRequestStatus());
   return rv;
@@ skipping 42 matching lines @@
   DCHECK(filter_.get());
 
   *bytes_read = 0;
 
   // Get more pre-filtered data if needed.
   // TODO(mbelshe): is it possible that the filter needs *MORE* data
   //    when there is some data already in the buffer?
   if (!filter_->stream_data_len() && !is_done()) {
     net::IOBuffer* stream_buffer = filter_->stream_buffer();
     int stream_buffer_size = filter_->stream_buffer_size();
-    rv = ReadRawData(stream_buffer, stream_buffer_size, bytes_read);
-    if (rv && *bytes_read > 0)
-      RecordBytesRead(*bytes_read);
+    rv = ReadRawDataHelper(stream_buffer, stream_buffer_size, bytes_read);
   }
   return rv;
 }
 
 void URLRequestJob::FollowRedirect(const GURL& location, int http_status_code) {
   g_url_request_job_tracker.OnJobRedirect(this, location, http_status_code);
 
   int rv = request_->Redirect(location, http_status_code);
   if (rv != net::OK)
     NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, rv));
 }
 
 void URLRequestJob::FilteredDataRead(int bytes_read) {
   DCHECK(filter_.get());  // don't add data if there is no filter
   filter_->FlushStreamBuffer(bytes_read);
 }
 
 bool URLRequestJob::ReadFilteredData(int* bytes_read) {
   DCHECK(filter_.get());  // don't add data if there is no filter
-  DCHECK(read_buffer_ != NULL);  // we need to have a buffer to fill
-  DCHECK_GT(read_buffer_len_, 0);  // sanity check
-  DCHECK_LT(read_buffer_len_, 1000000);  // sanity check
+  DCHECK(filtered_read_buffer_ != NULL);  // we need to have a buffer to fill
+  DCHECK_GT(filtered_read_buffer_len_, 0);  // sanity check
+  DCHECK_LT(filtered_read_buffer_len_, 1000000);  // sanity check
+  DCHECK(raw_read_buffer_ == NULL);  // there should be no raw read buffer yet
 
   bool rv = false;
   *bytes_read = 0;
 
   if (is_done())
     return true;
 
   if (!filter_needs_more_output_space_ && !filter_->stream_data_len()) {
     // We don't have any raw data to work with, so
     // read from the socket.
     int filtered_data_read;
     if (ReadRawDataForFilter(&filtered_data_read)) {
       if (filtered_data_read > 0) {
         filter_->FlushStreamBuffer(filtered_data_read);  // Give data to filter.
       } else {
         return true;  // EOF
       }
     } else {
       return false;  // IO Pending (or error)
     }
   }
 
   if ((filter_->stream_data_len() || filter_needs_more_output_space_)
       && !is_done()) {
     // Get filtered data.
-    int filtered_data_len = read_buffer_len_;
+    int filtered_data_len = filtered_read_buffer_len_;
     Filter::FilterStatus status;
     int output_buffer_size = filtered_data_len;
-    status = filter_->ReadData(read_buffer_->data(), &filtered_data_len);
+    status = filter_->ReadData(filtered_read_buffer_->data(),
+                               &filtered_data_len);
 
     if (filter_needs_more_output_space_ && 0 == filtered_data_len) {
       // filter_needs_more_output_space_ was mistaken... there are no more bytes
       // and we should have at least tried to fill up the filter's input buffer.
       // Correct the state, and try again.
       filter_needs_more_output_space_ = false;
       return ReadFilteredData(bytes_read);
     }
 
     switch (status) {
@@ skipping 44 matching lines @@
       }
     }
   } else {
     // we are done, or there is no data left.
     rv = true;
   }
 
   if (rv) {
     // When we successfully finished a read, we no longer need to
     // save the caller's buffers. Release our reference.
-    read_buffer_ = NULL;
-    read_buffer_len_ = 0;
+    filtered_read_buffer_ = NULL;
+    filtered_read_buffer_len_ = 0;
+  }
+  return rv;
+}
+
+bool URLRequestJob::ReadRawDataHelper(net::IOBuffer* buf, int buf_size,
+                                      int* bytes_read) {
+  DCHECK(!request_->status().is_io_pending());
+  DCHECK(raw_read_buffer_ == NULL);
+
+  // Keep a pointer to the read buffer, so we have access to it in the
+  // OnRawReadComplete() callback in the event that the read completes
+  // asynchronously.
+  raw_read_buffer_ = buf;
+  bool rv = ReadRawData(buf, buf_size, bytes_read);
+
+  if (!request_->status().is_io_pending()) {
+    // If the read completes synchronously, either success or failure,
+    // invoke the OnRawReadComplete callback so we can account for the
+    // completed read.
+    OnRawReadComplete(*bytes_read);
   }
   return rv;
 }
 
 bool URLRequestJob::ReadRawData(net::IOBuffer* buf, int buf_size,
                                 int *bytes_read) {
   DCHECK(bytes_read);
   *bytes_read = 0;
   NotifyDone(URLRequestStatus());
   return false;
@@ skipping 114 matching lines @@
   if (!request_ || !request_->delegate())
     return;  // The request was destroyed, so there is no more work to do.
 
   // TODO(darin): Bug 1004233. Re-enable this test once all of the chrome
   // unit_tests have been fixed to not trip this.
   //DCHECK(!request_->status().is_io_pending());
 
   // The headers should be complete before reads complete
   DCHECK(has_handled_response_);
 
-  if (bytes_read > 0)
-    RecordBytesRead(bytes_read);
+  OnRawReadComplete(bytes_read);
 
   // Don't notify if we had an error.
   if (!request_->status().is_success())
     return;
 
   // When notifying the delegate, the delegate can release the request
   // (and thus release 'this').  After calling to the delgate, we must
   // check the request pointer to see if it still exists, and return
   // immediately if it has been destroyed.  self_preservation ensures our
   // survival until we can get out of this method.
@@ skipping 92 matching lines @@
 void URLRequestJob::NotifyRestartRequired() {
   DCHECK(!has_handled_response_);
   if (GetStatus().status() != URLRequestStatus::CANCELED)
     request_->Restart();
 }
 
 bool URLRequestJob::FilterHasData() {
     return filter_.get() && filter_->stream_data_len();
 }
 
+void URLRequestJob::OnRawReadComplete(int bytes_read) {
+  DCHECK(raw_read_buffer_);
+  if (bytes_read > 0) {
+    RecordBytesRead(bytes_read);
+  }
+  raw_read_buffer_ = NULL;
+}
+
 void URLRequestJob::RecordBytesRead(int bytes_read) {
   if (is_profiling()) {
     ++(metrics_->number_of_read_IO_);
     metrics_->total_bytes_read_ += bytes_read;
   }
   filter_input_byte_count_ += bytes_read;
   UpdatePacketReadTimes();  // Facilitate stats recording if it is active.
-  g_url_request_job_tracker.OnBytesRead(this, bytes_read);
+  g_url_request_job_tracker.OnBytesRead(this, raw_read_buffer_->data(),
+                                        bytes_read);
 }
 
 const URLRequestStatus URLRequestJob::GetStatus() {
   if (request_)
     return request_->status();
   // If the request is gone, we must be cancelled.
   return URLRequestStatus(URLRequestStatus::CANCELED,
                           net::ERR_ABORTED);
 }
 
@@ skipping 233 matching lines @@
     return;
   }
 
   if (is_compressed_) {
     COMPRESSION_HISTOGRAM("NoProxy.BytesBeforeCompression", compressed_B);
     COMPRESSION_HISTOGRAM("NoProxy.BytesAfterCompression", decompressed_B);
   } else {
     COMPRESSION_HISTOGRAM("NoProxy.ShouldHaveBeenCompressed", decompressed_B);
   }
 }