Refactoring BufferedDataSource to work with WebURLLoader instead of a MediaResourceLoaderBridge.

webkit/glue/media/buffered_data_source.cc

Index: webkit/glue/media/buffered_data_source.cc
diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 29c86ac8f07323ecfa554f83968c099fcb59cad2..0e63b2b0bf197099bf79ac49176db23aff757d6b 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -4,19 +4,31 @@
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/format_macros.h"
 #include "base/message_loop.h"
 #include "base/process_util.h"
 #include "base/stl_util-inl.h"
+#include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "media/base/filter_host.h"
 #include "media/base/media_format.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
-#include "net/http/http_response_headers.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebKit.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebKitClient.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebString.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebURLError.h"
 #include "webkit/glue/media/buffered_data_source.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/glue/webmediaplayer_impl.h"
 
+using WebKit::WebFrame;
+using WebKit::WebString;
+using WebKit::WebURLError;
+using WebKit::WebURLLoader;
+using WebKit::WebURLRequest;
+using WebKit::WebURLResponse;
+
 namespace {
 
 const char kHttpScheme[] = "http";
@@ -70,10 +82,10 @@ bool IsDataProtocol(const GURL& url) {
 }  // namespace
 
 namespace webkit_glue {
+
 /////////////////////////////////////////////////////////////////////////////
 // BufferedResourceLoader
 BufferedResourceLoader::BufferedResourceLoader(
-    webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory,
     const GURL& url,
     int64 first_byte_position,
     int64 last_byte_position)
@@ -83,12 +95,10 @@ BufferedResourceLoader::BufferedResourceLoader(
       completed_(false),
       range_requested_(false),
       partial_response_(false),
-      bridge_factory_(bridge_factory),
       url_(url),
       first_byte_position_(first_byte_position),
       last_byte_position_(last_byte_position),
       start_callback_(NULL),
-      bridge_(NULL),
       offset_(0),
       content_length_(kPositionNotSpecified),
       instance_size_(kPositionNotSpecified),
@@ -97,21 +107,28 @@ BufferedResourceLoader::BufferedResourceLoader(
       read_size_(0),
       read_buffer_(NULL),
       first_offset_(0),
-      last_offset_(0) {
+      last_offset_(0),
+      keep_test_loader(false) {
 }
 
 BufferedResourceLoader::~BufferedResourceLoader() {
+  if (!completed_ && url_loader_.get())
+    url_loader_->cancel();
 }
 
 void BufferedResourceLoader::Start(net::CompletionCallback* start_callback,
-                                   NetworkEventCallback* event_callback) {
+                                   NetworkEventCallback* event_callback,
+                                   WebFrame* frame) {
   // Make sure we have not started.
-  DCHECK(!bridge_.get());
   DCHECK(!start_callback_.get());
   DCHECK(!event_callback_.get());
   DCHECK(start_callback);
   DCHECK(event_callback);
 
+  // TODO(annacc): Maybe throw up an error here if frame is null?
+  if (frame == NULL)
+    return;
+
   start_callback_.reset(start_callback);
   event_callback_.reset(event_callback);
 
@@ -122,21 +139,25 @@ void BufferedResourceLoader::Start(net::CompletionCallback* start_callback,
     offset_ = first_byte_position_;
   }
 
-  // Creates the bridge on render thread since we can only access
-  // ResourceDispatcher on this thread.
-  bridge_.reset(
-      bridge_factory_->CreateBridge(
-          url_,
-          net::LOAD_NORMAL,
-          first_byte_position_,
-          last_byte_position_));
-
   // Increment the reference count right before we start the request. This
   // reference will be release when this request has ended.
   AddRef();
 
-  // And start the resource loading.
-  bridge_->Start(this);
+  // Prepare the request.
+  WebURLRequest request(url_);
+  request.setHTTPHeaderField(WebString::fromUTF8("Range"),
+                               WebString::fromUTF8(GenerateHeaders(
+                                                   first_byte_position_,
+                                                   last_byte_position_)));
+  frame->setReferrerForRequest(request, WebKit::WebURL());
+  frame->dispatchWillSendRequest(request);
+
+  // This flag is for unittests as we don't want to reset |url_loader|
+  if (!keep_test_loader)
+    url_loader_.reset(WebKit::webKitClient()->createURLLoader());
+
+  // Start the resource loading.
+  url_loader_->loadAsynchronously(request, this);
 }
 
 void BufferedResourceLoader::Stop() {
@@ -153,14 +174,15 @@ void BufferedResourceLoader::Stop() {
   // Destroy internal buffer.
   buffer_.reset();
 
-  if (bridge_.get()) {
-    // Cancel the request. This method call will cancel the request
-    // asynchronously. We may still get data or messages until we receive
-    // a response completed message.
+  if (url_loader_.get()) {
     if (deferred_)
-      bridge_->SetDefersLoading(false);
+      url_loader_->setDefersLoading(false);
     deferred_ = false;
-    bridge_->Cancel();
+
+    if (!completed_) {
+      url_loader_->cancel();
+      completed_ = true;
+    }
   }
 }
 
@@ -231,38 +253,48 @@ void BufferedResourceLoader::SetAllowDefer(bool is_allowed) {
   DisableDeferIfNeeded();
 }
 
+void BufferedResourceLoader::SetURLLoaderForTest(WebURLLoader* mock_loader) {
+  url_loader_.reset(mock_loader);
+  keep_test_loader = true;
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // BufferedResourceLoader,
-//     webkit_glue::ResourceLoaderBridge::Peer implementations
-bool BufferedResourceLoader::OnReceivedRedirect(
-    const GURL& new_url,
-    const webkit_glue::ResourceResponseInfo& info,
-    bool* has_new_first_party_for_cookies,
-    GURL* new_first_party_for_cookies) {
-  DCHECK(bridge_.get());
-
-  // Save the new URL.
-  url_ = new_url;
-  // TODO(wtc): should we return a new first party for cookies URL?
-  *has_new_first_party_for_cookies = false;
+//       WebKit::WebURLLoaderClient implementations.

[Alpha Left Google, 2010/11/19 22:53:42]

move this back to the previous line.

+
+void BufferedResourceLoader::willSendRequest(
+    WebURLLoader* loader,
+    WebURLRequest& newRequest,
+    const WebURLResponse& redirectResponse) {
 
   // The load may have been stopped and |start_callback| is destroyed.
   // In this case we shouldn't do anything.
-  if (!start_callback_.get())
-    return true;
+  if (!start_callback_.get()) {
+    // Set the url in the request to an invalid value (empty url).
+    newRequest.setURL(WebKit::WebURL());
+    return;
+  }
 
-  if (!IsProtocolSupportedForMedia(new_url)) {
+  if (!IsProtocolSupportedForMedia(newRequest.url())) {
+    // Set the url in the request to an invalid value (empty url).
+    newRequest.setURL(WebKit::WebURL());
     DoneStart(net::ERR_ADDRESS_INVALID);
     Stop();
-    return false;
+    return;
   }
-  return true;
+
+  url_loader_.reset(loader);
+  url_ = newRequest.url();
+}
+
+void BufferedResourceLoader::didSendData(
+    WebURLLoader* loader,
+    unsigned long long bytes_sent,
+    unsigned long long total_bytes_to_be_sent) {
 }

[Alpha Left Google, 2010/11/19 22:53:42]

Add NOTIMPLEMENTED();

 
-void BufferedResourceLoader::OnReceivedResponse(
-    const webkit_glue::ResourceResponseInfo& info,
-    bool content_filtered) {
-  DCHECK(bridge_.get());
+void BufferedResourceLoader::didReceiveResponse(
+    WebURLLoader* loader, const WebURLResponse& response) {
 
   // The loader may have been stopped and |start_callback| is destroyed.
   // In this case we shouldn't do anything.
@@ -275,23 +307,18 @@ void BufferedResourceLoader::OnReceivedResponse(
   // response for HTTP and HTTPS protocol.
   if (IsHttpProtocol(url_)) {
     int error = net::OK;
-    if (!info.headers) {
-      // We expect to receive headers because this is a HTTP or HTTPS protocol,
-      // if not report failure.
-      error = net::ERR_INVALID_RESPONSE;
-    } else {
-      if (info.headers->response_code() == kHttpPartialContent)
-        partial_response_ = true;
-
-      if (range_requested_ && partial_response_) {
-        // If we have verified the partial response and it is correct, we will
-        // return net::OK.
-        if (!VerifyPartialResponse(info))
+
+    if (response.httpStatusCode() == kHttpPartialContent)
+      partial_response_ = true;
+
+    if (range_requested_ && partial_response_) {
+      // If we have verified the partial response and it is correct, we will
+      // return net::OK.
+      if (!VerifyPartialResponse(response))
           error = net::ERR_INVALID_RESPONSE;
-      } else if (info.headers->response_code() != kHttpOK) {
-        // We didn't request a range but server didn't reply with "200 OK".
-        error = net::ERR_FAILED;
-      }
+    } 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) {
@@ -305,9 +332,9 @@ void BufferedResourceLoader::OnReceivedResponse(
     partial_response_ = range_requested_;
   }
 
-  // |info.content_length| can be -1, in that case |content_length_| is
+  // Expected content length can be -1, in that case |content_length_| is
   // not specified and this is a streaming response.
-  content_length_ = info.content_length;
+  content_length_ = response.expectedContentLength();
 
   // If we have not requested a range, then the size of the instance is equal
   // to the content length.
@@ -318,8 +345,10 @@ void BufferedResourceLoader::OnReceivedResponse(
   DoneStart(net::OK);
 }
 
-void BufferedResourceLoader::OnReceivedData(const char* data, int len) {
-  DCHECK(bridge_.get());
+void BufferedResourceLoader::didReceiveData(
+    WebURLLoader* loader, const char* data, int data_length) {
+  DCHECK(!completed_);
+  DCHECK(data_length > 0);

[Alpha Left Google, 2010/11/19 22:53:42]

DCHECK_GT(data_length, 0);

 
   // If this loader has been stopped, |buffer_| would be destroyed.
   // In this case we shouldn't do anything.
@@ -327,7 +356,7 @@ void BufferedResourceLoader::OnReceivedData(const char* data, int len) {
     return;
 
   // Writes more data to |buffer_|.
-  buffer_->Append(reinterpret_cast<const uint8*>(data), len);
+  buffer_->Append(reinterpret_cast<const uint8*>(data), data_length);
 
   // If there is an active read request, try to fulfill the request.
   if (HasPendingRead() && CanFulfillRead()) {
@@ -350,18 +379,24 @@ void BufferedResourceLoader::OnReceivedData(const char* data, int len) {
   NotifyNetworkEvent();
 }
 
-void BufferedResourceLoader::OnCompletedRequest(
-    const URLRequestStatus& status,
-    const std::string& security_info,
-    const base::Time& completion_time) {
-  DCHECK(bridge_.get());
+void BufferedResourceLoader::didDownloadData(
+    WebKit::WebURLLoader* loader, int dataLength) {
+}

[Alpha Left Google, 2010/11/19 22:53:42]

Use NOTIMPLEMENTED(); instead.

+
+void BufferedResourceLoader::didReceiveCachedMetadata(
+    WebURLLoader* loader, const char* data, int data_length) {
+  DCHECK(!completed_);

[Alpha Left Google, 2010/11/19 22:53:42]

Use NOTIMPLEMENTED(); instead.

+  DCHECK(data_length > 0);
+}
 
-  // Saves the information that the request has completed.
+void BufferedResourceLoader::didFinishLoading(
+    WebURLLoader* loader, double finishTime) {
+  DCHECK(!completed_);
   completed_ = true;
 
   // If there is a start callback, calls it.
   if (start_callback_.get()) {
-    DoneStart(status.os_error());
+    DoneStart(net::OK);
   }
 
   // If there is a pending read but the request has ended, returns with what
@@ -370,16 +405,11 @@ void BufferedResourceLoader::OnCompletedRequest(
     // Make sure we have a valid buffer before we satisfy a read request.
     DCHECK(buffer_.get());
 
-    if (status.is_success()) {
-      // Try to fulfill with what is in the buffer.
-      if (CanFulfillRead())
-        ReadInternal();
-      else
-        DoneRead(net::ERR_CACHE_MISS);
-    } else {
-      // If the request has failed, then fail the read.
-      DoneRead(net::ERR_FAILED);
-    }
+    // 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.
@@ -388,10 +418,31 @@ void BufferedResourceLoader::OnCompletedRequest(
   // Notify that network response is completed.
   NotifyNetworkEvent();
 
-  // We incremented the reference count when the loader was started. We balance
-  // that reference here so that we get destroyed. This is also the only safe
-  // place to destroy the ResourceLoaderBridge.
-  bridge_.reset();
+  url_loader_.reset();
+  Release();
+}
+
+void BufferedResourceLoader::didFail(
+    WebURLLoader* loader,
+    const WebURLError& error) {
+  DCHECK(!completed_);
+  completed_ = true;
+
+  // If there is a start callback, calls it.
+  if (start_callback_.get()) {
+    DoneStart(error.reason);
+  }
+
+  // If there is a pending read but the request failed, return with the
+  // reason for the error.
+  if (HasPendingRead()) {
+    DoneRead(error.reason);
+  }
+
+  // Notify that network response is completed.
+  NotifyNetworkEvent();
+
+  url_loader_.reset();
   Release();
 }
 
@@ -405,8 +456,8 @@ void BufferedResourceLoader::EnableDeferIfNeeded() {
       buffer_->forward_bytes() >= buffer_->forward_capacity()) {
     deferred_ = true;
 
-    if (bridge_.get())
-      bridge_->SetDefersLoading(true);
+  if (url_loader_.get())
+    url_loader_->setDefersLoading(true);
 
     NotifyNetworkEvent();
   }
@@ -418,8 +469,8 @@ void BufferedResourceLoader::DisableDeferIfNeeded() {
        buffer_->forward_bytes() < buffer_->forward_capacity() / 2)) {
     deferred_ = false;
 
-    if (bridge_.get())
-      bridge_->SetDefersLoading(false);
+    if (url_loader_.get())
+      url_loader_->setDefersLoading(false);
 
     NotifyNetworkEvent();
   }
@@ -480,18 +531,21 @@ void BufferedResourceLoader::ReadInternal() {
 }
 
 bool BufferedResourceLoader::VerifyPartialResponse(
-    const ResourceResponseInfo& info) {
-  int64 first_byte_position, last_byte_position, instance_size;
-  if (!info.headers->GetContentRange(&first_byte_position,
-                                     &last_byte_position,
-                                     &instance_size)) {
+    const WebURLResponse& response) {
+  int first_byte_position, last_byte_position, instance_size;
+
+  if (!ReadContentRanges(response,
+                         &first_byte_position,
+                         &last_byte_position,
+                         &instance_size)) {
     return false;
   }
 
-  if (instance_size != kPositionNotSpecified)
+  if (instance_size != kPositionNotSpecified) {
     instance_size_ = instance_size;
+  }
 
-  if (first_byte_position_ != -1 &&
+  if (first_byte_position_ != kPositionNotSpecified &&
       first_byte_position_ != first_byte_position) {
     return false;
   }
@@ -501,6 +555,102 @@ bool BufferedResourceLoader::VerifyPartialResponse(
   return true;
 }
 
+bool BufferedResourceLoader::ReadContentRanges(

[Alpha Left Google, 2010/11/19 22:53:42]

Instead of adding this method what about use ReadContentRanges() in
MultipartResponseDelegate. You can add content_range_instance_size and test it.
It is best to avoid copying code.
 

+    const WebURLResponse& response,
+    int* content_range_lower_bound,
+    int* content_range_upper_bound,
+    int* content_range_instance_size) {
+
+  std::string content_range = response.httpHeaderField("Content-Range").utf8();
+  if (content_range.empty()) {
+    content_range = response.httpHeaderField("Range").utf8();
+  }
+
+  if (content_range.empty()) {
+    DLOG(WARNING) << "Failed to read content range from response.";
+    return false;
+  }
+
+  size_t byte_range_lower_bound_start_offset = content_range.find(" ");
+  if (byte_range_lower_bound_start_offset == std::string::npos) {
+    return false;
+  }
+
+  // Skip over the initial space.
+  byte_range_lower_bound_start_offset++;
+
+  // Find the lower bound.
+  size_t byte_range_lower_bound_end_offset =
+      content_range.find("-", byte_range_lower_bound_start_offset);
+  if (byte_range_lower_bound_end_offset == std::string::npos) {
+    return false;
+  }
+
+  size_t byte_range_lower_bound_characters =
+      byte_range_lower_bound_end_offset - byte_range_lower_bound_start_offset;
+  std::string byte_range_lower_bound =
+      content_range.substr(byte_range_lower_bound_start_offset,
+                           byte_range_lower_bound_characters);
+
+  // Find the upper bound.
+  size_t byte_range_upper_bound_start_offset =
+      byte_range_lower_bound_end_offset + 1;
+
+  size_t byte_range_upper_bound_end_offset =
+      content_range.find("/", byte_range_upper_bound_start_offset);
+  if (byte_range_upper_bound_end_offset == std::string::npos) {
+    return false;
+  }
+
+  size_t byte_range_upper_bound_characters =
+      byte_range_upper_bound_end_offset - byte_range_upper_bound_start_offset;
+  std::string byte_range_upper_bound =
+      content_range.substr(byte_range_upper_bound_start_offset,
+                           byte_range_upper_bound_characters);
+
+  // Find the instance size.
+  size_t byte_range_instance_size_start_offset =
+      byte_range_upper_bound_end_offset + 1;
+
+  size_t byte_range_instance_size_end_offset =
+      content_range.length();
+
+  size_t byte_range_instance_size_characters =
+      byte_range_instance_size_end_offset -
+      byte_range_instance_size_start_offset;
+  std::string byte_range_instance_size =
+      content_range.substr(byte_range_instance_size_start_offset,
+                           byte_range_instance_size_characters);
+
+  if (!base::StringToInt(byte_range_lower_bound, content_range_lower_bound))
+    return false;
+  if (!base::StringToInt(byte_range_upper_bound, content_range_upper_bound))
+    return false;
+  if (!base::StringToInt(byte_range_instance_size, content_range_instance_size))
+    return false;
+  return true;
+}
+
+std::string BufferedResourceLoader::GenerateHeaders(
+    int64 first_byte_position, int64 last_byte_position) {
+  // Construct the value for the range header.
+  std::string header;
+  if (first_byte_position > kPositionNotSpecified &&
+      last_byte_position > kPositionNotSpecified) {
+    if (first_byte_position <= last_byte_position) {
+      header = base::StringPrintf("bytes=%" PRId64 "-%" PRId64,
+                                  first_byte_position,
+                                  last_byte_position);
+    }
+  } 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();
@@ -525,12 +675,12 @@ void BufferedResourceLoader::NotifyNetworkEvent() {
 // BufferedDataSource
 BufferedDataSource::BufferedDataSource(
     MessageLoop* render_loop,
-    webkit_glue::MediaResourceLoaderBridgeFactory* bridge_factory)
+    WebFrame* frame)
     : total_bytes_(kPositionNotSpecified),
       loaded_(false),
       streaming_(false),
+      frame_(frame),
       single_origin_(true),
-      bridge_factory_(bridge_factory),
       loader_(NULL),
       network_activity_(false),
       initialize_callback_(NULL),
@@ -558,7 +708,7 @@ BufferedResourceLoader* BufferedDataSource::CreateResourceLoader(
     int64 first_byte_position, int64 last_byte_position) {
   DCHECK(MessageLoop::current() == render_loop_);
 
-  return new BufferedResourceLoader(bridge_factory_.get(), url_,
+  return new BufferedResourceLoader(url_,
                                     first_byte_position,
                                     last_byte_position);
 }
@@ -658,12 +808,12 @@ void BufferedDataSource::Abort() {
   // If we are told to abort, immediately return from any pending read
   // with an error.
   if (read_callback_.get()) {
-    {
       AutoLock auto_lock(lock_);
       DoneRead_Locked(net::ERR_FAILED);
-    }
-    CleanupTask();
   }
+
+  CleanupTask();
+  frame_ = NULL;
 }
 
 /////////////////////////////////////////////////////////////////////////////
@@ -690,7 +840,8 @@ void BufferedDataSource::InitializeTask() {
     loader_ = CreateResourceLoader(0, 1024);
     loader_->Start(
         NewCallback(this, &BufferedDataSource::HttpInitialStartCallback),
-        NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+        NewCallback(this, &BufferedDataSource::NetworkEventCallback),
+        frame_);
   } else {
     // For all other protocols, assume they support range request. We fetch
     // the full range of the resource to obtain the instance size because
@@ -698,7 +849,8 @@ void BufferedDataSource::InitializeTask() {
     loader_ = CreateResourceLoader(-1, -1);
     loader_->Start(
         NewCallback(this, &BufferedDataSource::NonHttpInitialStartCallback),
-        NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+        NewCallback(this, &BufferedDataSource::NetworkEventCallback),
+        frame_);
   }
 }
 
@@ -775,7 +927,8 @@ void BufferedDataSource::RestartLoadingTask() {
   loader_->SetAllowDefer(!media_is_paused_);
   loader_->Start(
       NewCallback(this, &BufferedDataSource::PartialReadStartCallback),
-      NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+      NewCallback(this, &BufferedDataSource::NetworkEventCallback),
+      frame_);
 }
 
 void BufferedDataSource::WatchDogTask() {
@@ -806,7 +959,8 @@ void BufferedDataSource::WatchDogTask() {
   loader_->SetAllowDefer(!media_is_paused_);
   loader_->Start(
       NewCallback(this, &BufferedDataSource::PartialReadStartCallback),
-      NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+      NewCallback(this, &BufferedDataSource::NetworkEventCallback),
+      frame_);
 }
 
 void BufferedDataSource::SetPlaybackRateTask(float playback_rate) {
@@ -829,7 +983,7 @@ void BufferedDataSource::SetPlaybackRateTask(float playback_rate) {
 // prior to make this method call.
 void BufferedDataSource::ReadInternal() {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(loader_.get());
+  DCHECK(loader_);
 
   // First we prepare the intermediate read buffer for BufferedResourceLoader
   // to write to.
@@ -904,7 +1058,8 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
     loader_ = CreateResourceLoader(-1, -1);
     loader_->Start(
         NewCallback(this, &BufferedDataSource::HttpInitialStartCallback),
-        NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+        NewCallback(this, &BufferedDataSource::NetworkEventCallback),
+        frame_);
     return;
   }