Split a portion of BufferedResourceLoader into a separate class ActiveLoader.

webkit/media/buffered_resource_loader.cc

 // Copyright (c) 2011 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 "webkit/media/buffered_resource_loader.h"
 
 #include "base/format_macros.h"
 #include "base/stringprintf.h"
 #include "base/string_util.h"
 #include "media/base/media_log.h"
@@ skipping 75 matching lines @@
   *out_backward_capacity = std::max(
       kTargetSecondsBufferedBehind * bytes_per_second, kMinBufferCapacity);
 
   *out_forward_capacity = std::min(*out_forward_capacity, kMaxBufferCapacity);
   *out_backward_capacity = std::min(*out_backward_capacity, kMaxBufferCapacity);
 
   if (backward_playback)
     std::swap(*out_forward_capacity, *out_backward_capacity);
 }
 
-
 BufferedResourceLoader::BufferedResourceLoader(
     const GURL& url,
     int64 first_byte_position,
     int64 last_byte_position,
     DeferStrategy strategy,
     int bitrate,
     float playback_rate,
     media::MediaLog* media_log)
-    : deferred_(false),
-      defer_strategy_(strategy),
-      completed_(false),
+    : defer_strategy_(strategy),
       range_requested_(false),
       range_supported_(false),
       saved_forward_capacity_(0),
       url_(url),
       first_byte_position_(first_byte_position),
       last_byte_position_(last_byte_position),
       single_origin_(true),
       start_callback_(NULL),
       offset_(0),
       content_length_(kPositionNotSpecified),
       instance_size_(kPositionNotSpecified),
       read_callback_(NULL),
       read_position_(0),
       read_size_(0),
       read_buffer_(NULL),
       first_offset_(0),
       last_offset_(0),
-      keep_test_loader_(false),
       bitrate_(bitrate),
       playback_rate_(playback_rate),
       media_log_(media_log) {
 
   size_t backward_capacity;
   size_t forward_capacity;
   ComputeTargetBufferWindow(
       playback_rate_, bitrate_, &backward_capacity, &forward_capacity);
   buffer_.reset(new media::SeekableBuffer(backward_capacity, forward_capacity));
 }
 
-BufferedResourceLoader::~BufferedResourceLoader() {
-  if (!completed_ && url_loader_.get())
-    url_loader_->cancel();
-}
+BufferedResourceLoader::~BufferedResourceLoader() {}
 
 void BufferedResourceLoader::Start(net::OldCompletionCallback* start_callback,
                                    const base::Closure& event_callback,
                                    WebFrame* frame) {
   // Make sure we have not started.
   DCHECK(!start_callback_.get());
   DCHECK(event_callback_.is_null());
   DCHECK(start_callback);
   DCHECK(!event_callback.is_null());
   CHECK(frame);
 
   start_callback_.reset(start_callback);
   event_callback_ = event_callback;
 
   if (first_byte_position_ != kPositionNotSpecified) {
     // TODO(hclam): server may not support range request so |offset_| may not
     // equal to |first_byte_position_|.
     offset_ = first_byte_position_;
   }
 
-  // Increment the reference count right before we start the request. This
-  // reference will be release when this request has ended.
-  AddRef();
-
   // Prepare the request.
   WebURLRequest request(url_);
   request.setTargetType(WebURLRequest::TargetIsMedia);
 
   if (IsRangeRequest()) {
     range_requested_ = true;
     request.setHTTPHeaderField(
         WebString::fromUTF8(net::HttpRequestHeaders::kRange),
         WebString::fromUTF8(GenerateHeaders(first_byte_position_,
                                             last_byte_position_)));
   }
   frame->setReferrerForRequest(request, WebKit::WebURL());
 
   // Disable compression, compression for audio/video doesn't make sense...
   request.setHTTPHeaderField(
       WebString::fromUTF8(net::HttpRequestHeaders::kAcceptEncoding),
       WebString::fromUTF8("identity;q=1, *;q=0"));
 
-  // This flag is for unittests as we don't want to reset |url_loader|
-  if (!keep_test_loader_) {
+  // Check for our test WebURLLoader.
+  WebURLLoader* loader = NULL;
+  if (test_loader_.get()) {
+    loader = test_loader_.release();
+  } else {
     WebURLLoaderOptions options;
     options.allowCredentials = true;
     options.crossOriginRequestPolicy =
         WebURLLoaderOptions::CrossOriginRequestPolicyAllow;
-    url_loader_.reset(frame->createAssociatedURLLoader(options));
+    loader = frame->createAssociatedURLLoader(options);
   }
 
   // Start the resource loading.
-  url_loader_->loadAsynchronously(request, this);
+  loader->loadAsynchronously(request, this);
+  active_loader_.reset(new ActiveLoader(this, loader));
 }
 
 void BufferedResourceLoader::Stop() {
   // Reset callbacks.
   start_callback_.reset();
   event_callback_.Reset();
   read_callback_.reset();
 
   // Use the internal buffer to signal that we have been stopped.
   // TODO(hclam): Not so pretty to do this.
   if (!buffer_.get())
     return;
 
   // Destroy internal buffer.
   buffer_.reset();
 
-  if (url_loader_.get()) {
-    if (deferred_)
-      url_loader_->setDefersLoading(false);
-    deferred_ = false;
-
-    if (!completed_) {
-      url_loader_->cancel();
-      completed_ = true;
-    }
-  }
+  // Cancel and reset any active loaders.
+  active_loader_.reset();
 }
 
 void BufferedResourceLoader::Read(int64 position,
                                   int read_size,
                                   uint8* buffer,
                                   net::OldCompletionCallback* read_callback) {
   DCHECK(!read_callback_.get());
   DCHECK(buffer_.get());
   DCHECK(read_callback);
   DCHECK(buffer);
@@ skipping 55 matching lines @@
     // capacity.
     //
     // This can happen when reading in a large seek index or when the
     // first byte of a read request falls within kForwardWaitThreshold.
     if (last_offset_ > static_cast<int>(buffer_->forward_capacity())) {
       saved_forward_capacity_ = buffer_->forward_capacity();
       buffer_->set_forward_capacity(last_offset_);
     }
 
     // Make sure we stop deferring now that there's additional capacity.
-    //
-    // XXX: can we DCHECK(url_loader_.get()) at this point in time?
-    if (deferred_)
+    if (active_loader_->deferred())
       SetDeferred(false);
 
     DCHECK(!ShouldEnableDefer())
         << "Capacity was not adjusted properly to prevent deferring.";
 
     return;
   }
 
   // Make a callback to report failure.
   DoneRead(net::ERR_CACHE_MISS);
@@ skipping 11 matching lines @@
 
 int64 BufferedResourceLoader::instance_size() {
   return instance_size_;
 }
 
 bool BufferedResourceLoader::range_supported() {
   return range_supported_;
 }
 
 bool BufferedResourceLoader::is_downloading_data() {
-  return !completed_ && !deferred_;
+  return active_loader_.get() && !active_loader_->deferred();
 }
 
 const GURL& BufferedResourceLoader::url() {
   return url_;
 }
 
-void BufferedResourceLoader::SetURLLoaderForTest(WebURLLoader* mock_loader) {
-  url_loader_.reset(mock_loader);
-  keep_test_loader_ = true;
+void BufferedResourceLoader::SetURLLoaderForTest(WebURLLoader* test_loader) {
+  test_loader_.reset(test_loader);
 }
 
 /////////////////////////////////////////////////////////////////////////////
 // WebKit::WebURLLoaderClient implementation.
 void BufferedResourceLoader::willSendRequest(
     WebURLLoader* loader,
     WebURLRequest& newRequest,
     const WebURLResponse& redirectResponse) {
 
   // The load may have been stopped and |start_callback| is destroyed.
@@ skipping 15 matching lines @@
     WebURLLoader* loader,
     unsigned long long bytes_sent,
     unsigned long long total_bytes_to_be_sent) {
   NOTIMPLEMENTED();
 }
 
 void BufferedResourceLoader::didReceiveResponse(
     WebURLLoader* loader,
     const WebURLResponse& response) {
   VLOG(1) << "didReceiveResponse: " << response.httpStatusCode();
+  DCHECK(active_loader_.get());
 
   // The loader may have been stopped and |start_callback| is destroyed.
   // In this case we shouldn't do anything.
   if (!start_callback_.get())
     return;
 
   bool partial_response = false;
 
   // We make a strong assumption that when we reach here we have either
   // received a response from HTTP/HTTPS protocol or the request was
@@ skipping 17 matching lines @@
         range_supported_ = true;
       else
         error = net::ERR_INVALID_RESPONSE;
     } else if (response.httpStatusCode() != kHttpOK) {
       // We didn't request a range but server didn't reply with "200 OK".
       error = net::ERR_FAILED;
     }
 
     if (error != net::OK) {
       DoneStart(error);
-      Stop();
       return;
     }
   } else {
     // For any protocol other than HTTP and HTTPS, assume range request is
     // always fulfilled.
     partial_response = range_requested_;
   }
 
   // Expected content length can be |kPositionNotSpecified|, in that case
   // |content_length_| is not specified and this is a streaming response.
   content_length_ = response.expectedContentLength();
 
   // If we have not requested a range, then the size of the instance is equal
   // to the content length.
   if (!partial_response)
     instance_size_ = content_length_;
 
   // Calls with a successful response.
   DoneStart(net::OK);
 }
 
 void BufferedResourceLoader::didReceiveData(
     WebURLLoader* loader,
     const char* data,
     int data_length,
     int encoded_data_length) {
   VLOG(1) << "didReceiveData: " << data_length << " bytes";
-
-  DCHECK(!completed_);
+  DCHECK(active_loader_.get());
   DCHECK_GT(data_length, 0);
 
   // If this loader has been stopped, |buffer_| would be destroyed.
   // In this case we shouldn't do anything.
   if (!buffer_.get())
     return;
 
   // Writes more data to |buffer_|.
   buffer_->Append(reinterpret_cast<const uint8*>(data), data_length);
 
@@ skipping 27 matching lines @@
     WebURLLoader* loader,
     const char* data,
     int data_length) {
   NOTIMPLEMENTED();
 }
 
 void BufferedResourceLoader::didFinishLoading(
     WebURLLoader* loader,
     double finishTime) {
   VLOG(1) << "didFinishLoading";
-
-  DCHECK(!completed_);
-  completed_ = true;
+  DCHECK(active_loader_.get());
+  active_loader_.reset();
 
   // If we didn't know the |instance_size_| we do now.
   if (instance_size_ == kPositionNotSpecified) {
     instance_size_ = offset_ + buffer_->forward_bytes();
   }
 
   // If there is a start callback, calls it.
   if (start_callback_.get()) {
     DoneStart(net::OK);

[vrk (LEFT CHROMIUM), 2011/12/01 00:22:28]

When would there be a start_callback_ at this point? Can didFinishLoading get
called before didReceiveResponse?

Also was wondering if this is the same scenario as below, where you can have a
start callback or a read callback but never both.

[scherkus (not reviewing), 2011/12/02 18:24:24]

I *think* so -- I'll add in a DCHECK for  now

   }
 
   // If there is a pending read but the request has ended, returns with what
   // we have.
   if (HasPendingRead()) {
     // Make sure we have a valid buffer before we satisfy a read request.
     DCHECK(buffer_.get());
 
     // Try to fulfill with what is in the buffer.
     if (CanFulfillRead())
       ReadInternal();
     else
       DoneRead(net::ERR_CACHE_MISS);
   }
 
   // There must not be any outstanding read request.
   DCHECK(!HasPendingRead());
 
   // Notify that network response is completed.
   NotifyNetworkEvent();
-
-  url_loader_.reset();
-  Release();
 }
 
 void BufferedResourceLoader::didFail(
     WebURLLoader* loader,
     const WebURLError& error) {
   VLOG(1) << "didFail: " << error.reason;
+  DCHECK(active_loader_.get());
+  active_loader_.reset();
 
-  DCHECK(!completed_);
-  completed_ = true;
+  NotifyNetworkEvent();
 
-  // If there is a start callback, calls it.
+  // Don't leave start callbacks hanging around.
   if (start_callback_.get()) {
     DoneStart(error.reason);
+    return;

[vrk (LEFT CHROMIUM), 2011/12/01 00:22:28]

As mentioned offline: DCHECK that you don't have both a start callback and a
read callback.

[scherkus (not reviewing), 2011/12/02 18:24:24]

Done.

   }
 
-  // If there is a pending read but the request failed, return with the
-  // reason for the error.
+  // Don't leave read callbacks hanging around.
   if (HasPendingRead()) {
     DoneRead(error.reason);
   }
-
-  // Notify that network response is completed.
-  NotifyNetworkEvent();
-
-  url_loader_.reset();
-  Release();
 }
 
 bool BufferedResourceLoader::HasSingleOrigin() const {
   return single_origin_;
 }
 
 void BufferedResourceLoader::UpdateDeferStrategy(DeferStrategy strategy) {
   defer_strategy_ = strategy;
   UpdateDeferBehavior();
 }
@@ skipping 28 matching lines @@
       playback_rate_, bitrate_, &backward_capacity, &forward_capacity);
 
   // This does not evict data from the buffer if the new capacities are less
   // than the current capacities; the new limits will be enforced after the
   // existing excess buffered data is consumed.
   buffer_->set_backward_capacity(backward_capacity);
   buffer_->set_forward_capacity(forward_capacity);
 }
 
 void BufferedResourceLoader::UpdateDeferBehavior() {
-  if (!url_loader_.get() || !buffer_.get())
+  if (!active_loader_.get() || !buffer_.get())
     return;
 
   // If necessary, toggle defer state and continue/pause downloading data
   // accordingly.
   if (ShouldEnableDefer() || ShouldDisableDefer())
-    SetDeferred(!deferred_);
+    SetDeferred(!active_loader_->deferred());
 }
 
 void BufferedResourceLoader::SetDeferred(bool deferred) {
-  deferred_ = deferred;
-  if (url_loader_.get()) {
-    url_loader_->setDefersLoading(deferred);
-    NotifyNetworkEvent();
-  }
+  active_loader_->SetDeferred(deferred);
+  NotifyNetworkEvent();
 }
 
-bool BufferedResourceLoader::ShouldEnableDefer() {
+bool BufferedResourceLoader::ShouldEnableDefer() const {
   // If we're already deferring, then enabling makes no sense.
-  if (deferred_)
+  if (active_loader_->deferred())
     return false;
 
   switch(defer_strategy_) {
     // Never defer at all, so never enable defer.
     case kNeverDefer:
       return false;
 
     // Defer if nothing is being requested.
     case kReadThenDefer:
       return !read_callback_.get();
 
     // Defer if we've reached the max capacity of the threshold.
     case kThresholdDefer:
       return buffer_->forward_bytes() >= buffer_->forward_capacity();
   }
   // Otherwise don't enable defer.
   return false;
 }
 
-bool BufferedResourceLoader::ShouldDisableDefer() {
+bool BufferedResourceLoader::ShouldDisableDefer() const {
   // If we're not deferring, then disabling makes no sense.
-  if (!deferred_)
+  if (!active_loader_->deferred())
     return false;
 
   switch(defer_strategy_) {
     // Always disable deferring.
     case kNeverDefer:
       return true;
 
     // We have an outstanding read request, and we have not buffered enough
     // yet to fulfill the request; disable defer to get more data.
     case kReadThenDefer:
       return read_callback_.get() &&
           last_offset_ > static_cast<int>(buffer_->forward_bytes());
 
     // We have less than half the capacity of our threshold, so
     // disable defer to get more data.
     case kThresholdDefer: {
       size_t amount_buffered = buffer_->forward_bytes();
       size_t half_capacity = buffer_->forward_capacity() / 2;
       return amount_buffered < half_capacity;
     }
   }
 
   // Otherwise keep deferring.
   return false;
 }
 
-bool BufferedResourceLoader::CanFulfillRead() {
+bool BufferedResourceLoader::CanFulfillRead() const {
   // If we are reading too far in the backward direction.
   if (first_offset_ < 0 &&
       first_offset_ + static_cast<int>(buffer_->backward_bytes()) < 0)
     return false;
 
   // If the start offset is too far ahead.
   if (first_offset_ >= static_cast<int>(buffer_->forward_bytes()))
     return false;
 
   // At the point, we verified that first byte requested is within the buffer.
   // If the request has completed, then just returns with what we have now.
-  if (completed_)
+  if (!active_loader_.get())
     return true;
 
   // If the resource request is still active, make sure the whole requested
   // range is covered.
   if (last_offset_ > static_cast<int>(buffer_->forward_bytes()))
     return false;
 
   return true;
 }
 
-bool BufferedResourceLoader::WillFulfillRead() {
+bool BufferedResourceLoader::WillFulfillRead() const {
   // Trying to read too far behind.
   if (first_offset_ < 0 &&
       first_offset_ + static_cast<int>(buffer_->backward_bytes()) < 0)
     return false;
 
   // Trying to read too far ahead.
   if (first_offset_ - static_cast<int>(buffer_->forward_bytes()) >=
       kForwardWaitThreshold)
     return false;
 
   // The resource request has completed, there's no way we can fulfill the
   // read request.
-  if (completed_)
+  if (!active_loader_.get())
     return false;
 
   return true;
 }
 
 void BufferedResourceLoader::ReadInternal() {
   // Seek to the first byte requested.
   bool ret = buffer_->Seek(first_offset_);
   DCHECK(ret);
 
@@ skipping 45 matching lines @@
   } else if (first_byte_position > kPositionNotSpecified) {
     header = base::StringPrintf("bytes=%" PRId64 "-",
                                 first_byte_position);
   } else if (last_byte_position > kPositionNotSpecified) {
     NOTIMPLEMENTED() << "Suffix range not implemented";
   }
   return header;
 }
 
 void BufferedResourceLoader::DoneRead(int error) {
-  read_callback_->RunWithParams(Tuple1<int>(error));
-  read_callback_.reset();
   if (buffer_.get() && saved_forward_capacity_) {
     buffer_->set_forward_capacity(saved_forward_capacity_);
     saved_forward_capacity_ = 0;
   }
   read_position_ = 0;
   read_size_ = 0;
   read_buffer_ = NULL;
   first_offset_ = 0;
   last_offset_ = 0;
   Log();
+
+  scoped_ptr<net::OldCompletionCallback> read_callback;
+  read_callback.swap(read_callback_);
+  read_callback->RunWithParams(Tuple1<int>(error));
 }
 
 void BufferedResourceLoader::DoneStart(int error) {
-  start_callback_->RunWithParams(Tuple1<int>(error));
-  start_callback_.reset();
+  scoped_ptr<net::OldCompletionCallback> start_callback;
+  start_callback.swap(start_callback_);
+  start_callback->RunWithParams(Tuple1<int>(error));
 }
 
 void BufferedResourceLoader::NotifyNetworkEvent() {
   if (!event_callback_.is_null())
     event_callback_.Run();
 }
 
 bool BufferedResourceLoader::IsRangeRequest() const {
   return first_byte_position_ != kPositionNotSpecified;
 }
 
 void BufferedResourceLoader::Log() {
   if (buffer_.get()) {
     media_log_->AddEvent(
         media_log_->CreateBufferedExtentsChangedEvent(
             offset_ - buffer_->backward_bytes(),
             offset_,
             offset_ + buffer_->forward_bytes()));
   }
 }
 
 }  // namespace webkit_media