Split BufferedResourceLoader's network callback into separate loading state and progress callbacks.

webkit/media/buffered_data_source.cc

 // Copyright (c) 2012 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_data_source.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "media/base/media_log.h"
 #include "net/base/net_errors.h"
 
 using WebKit::WebFrame;
 
 namespace {
 
 // BufferedDataSource has an intermediate buffer, this value governs the initial
 // size of that buffer. It is set to 32KB because this is a typical read size
 // of FFmpeg.
 const int kInitialReadBufferSize = 32768;
 
 // Number of cache misses we allow for a single Read() before signaling an
 // error.
 const int kNumCacheMissRetries = 3;
 
 }  // namespace
 
 namespace webkit_media {
 
+// Non-HTTP resources are assumed to be fully loaded so we ignore any
+// loading/progress related callbacks.
+static void NonHttpLoadingStateChangedCallback(
+    BufferedResourceLoader::LoadingState) {
+}
+static void NonHttpProgressCallback(int64) {}
+
 BufferedDataSource::BufferedDataSource(
     MessageLoop* render_loop,
     WebFrame* frame,
     media::MediaLog* media_log,
     const DownloadingCB& downloading_cb)
     : cors_mode_(BufferedResourceLoader::kUnspecified),
       total_bytes_(kPositionNotSpecified),
       buffered_bytes_(0),
       streaming_(false),
       frame_(frame),
-      is_downloading_data_(false),
       read_size_(0),
       read_buffer_(NULL),
       last_read_start_(0),
       intermediate_read_buffer_(new uint8[kInitialReadBufferSize]),
       intermediate_read_buffer_size_(kInitialReadBufferSize),
       render_loop_(render_loop),
       stop_signal_received_(false),
       stopped_on_render_loop_(false),
       media_has_played_(false),
       preload_(AUTO),
@@ skipping 48 matching lines @@
   cors_mode_ = cors_mode;
 
   initialize_cb_ = initialize_cb;
 
   if (url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme)) {
     // Do an unbounded range request starting at the beginning.  If the server
     // responds with 200 instead of 206 we'll fall back into a streaming mode.
     loader_.reset(CreateResourceLoader(0, kPositionNotSpecified));
     loader_->Start(
         base::Bind(&BufferedDataSource::HttpInitialStartCallback, this),
-        base::Bind(&BufferedDataSource::NetworkEventCallback, this),
+        base::Bind(&BufferedDataSource::HttpLoadingStateChangedCallback, this),
+        base::Bind(&BufferedDataSource::HttpProgressCallback, this),
         frame_);
     return;
   }
 
   // For all other protocols, assume they support range request. We fetch
   // the full range of the resource to obtain the instance size because
   // we won't be served HTTP headers.
   loader_.reset(CreateResourceLoader(kPositionNotSpecified,
                                      kPositionNotSpecified));
   loader_->Start(
       base::Bind(&BufferedDataSource::NonHttpInitialStartCallback, this),
-      base::Bind(&BufferedDataSource::NetworkEventCallback, this),
+      base::Bind(&NonHttpLoadingStateChangedCallback),
+      base::Bind(&NonHttpProgressCallback),
       frame_);
 }
 
 void BufferedDataSource::SetPreload(Preload preload) {
   DCHECK(MessageLoop::current() == render_loop_);
   preload_ = preload;
 }
 
 bool BufferedDataSource::HasSingleOrigin() {
   DCHECK(MessageLoop::current() == render_loop_);
@@ skipping 131 matching lines @@
   if (stopped_on_render_loop_)
     return;
 
   {
     // If there's no outstanding read then return early.
     base::AutoLock auto_lock(lock_);
     if (read_cb_.is_null())
       return;
   }
 
+  // Start reading from where we last left off until the end of the resource.
   loader_.reset(
       CreateResourceLoader(last_read_start_, kPositionNotSpecified));
-  loader_->Start(
-      base::Bind(&BufferedDataSource::PartialReadStartCallback, this),
-      base::Bind(&BufferedDataSource::NetworkEventCallback, this),
-      frame_);
+  if (url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme)) {
+    loader_->Start(
+        base::Bind(&BufferedDataSource::PartialReadStartCallback, this),
+        base::Bind(&BufferedDataSource::HttpLoadingStateChangedCallback, this),
+        base::Bind(&BufferedDataSource::HttpProgressCallback, this),
+        frame_);
+  } else {
+    loader_->Start(
+        base::Bind(&BufferedDataSource::NonHttpInitialStartCallback, this),
+        base::Bind(&NonHttpLoadingStateChangedCallback),
+        base::Bind(&NonHttpProgressCallback),
+        frame_);
+  }
 }
 
 void BufferedDataSource::SetPlaybackRateTask(float playback_rate) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
   if (playback_rate != 0)
     media_has_played_ = true;
 
   playback_rate_ = playback_rate;
@@ skipping 223 matching lines @@
       cache_miss_retries_left_--;
       render_loop_->PostTask(FROM_HERE,
           base::Bind(&BufferedDataSource::RestartLoadingTask, this));
       return;
     }
 
     // Fall through to signal a read error.
     bytes_read = kReadError;
   }
 
-  // We need to prevent calling to filter host and running the callback if
-  // we have received the stop signal. We need to lock down the whole callback
-  // method to prevent bad things from happening. The reason behind this is
-  // that we cannot guarantee tasks on render thread have completely stopped
-  // when we receive the Stop() method call. So only way to solve this is to
-  // let tasks on render thread to run but make sure they don't call outside
-  // this object when Stop() method is ever called. Locking this method is safe
-  // because |lock_| is only acquired in tasks on render thread.
+  // TODO(scherkus): we shouldn't have to lock to signal host(), see
+  // http://crbug.com/113712 for details.
   base::AutoLock auto_lock(lock_);
   if (stop_signal_received_)
     return;
 
   if (bytes_read > 0) {
     memcpy(read_buffer_, intermediate_read_buffer_.get(), bytes_read);
   } else if (bytes_read == 0 && total_bytes_ == kPositionNotSpecified) {
     // We've reached the end of the file and we didn't know the total size
     // before. Update the total size so Read()s past the end of the file will
     // fail like they would if we had known the file size at the beginning.
     total_bytes_ = loader_->instance_size();
 
     if (host() && total_bytes_ != kPositionNotSpecified) {
       host()->SetTotalBytes(total_bytes_);
       host()->AddBufferedByteRange(loader_->first_byte_position(),
                                    total_bytes_);
     }
   }
   DoneRead_Locked(bytes_read);
 }
 
-void BufferedDataSource::NetworkEventCallback() {
+void BufferedDataSource::HttpLoadingStateChangedCallback(
+    BufferedResourceLoader::LoadingState state) {
   DCHECK(MessageLoop::current() == render_loop_);
-  DCHECK(loader_.get());
+  DCHECK(url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme));
 
-  // In case of non-HTTP request we don't need to report network events,
-  // so return immediately.
-  if (!url_.SchemeIs(kHttpScheme) && !url_.SchemeIs(kHttpsScheme))
-    return;
-
-  downloading_cb_.Run(loader_->is_downloading_data());
-
-  int64 current_buffered_position = loader_->GetBufferedPosition();
-
-  // If we get an unspecified value, return immediately.
-  if (current_buffered_position == kPositionNotSpecified)
-    return;
-
-  // We need to prevent calling to filter host and running the callback if
-  // we have received the stop signal. We need to lock down the whole callback
-  // method to prevent bad things from happening. The reason behind this is
-  // that we cannot guarantee tasks on render thread have completely stopped
-  // when we receive the Stop() method call. So only way to solve this is to
-  // let tasks on render thread to run but make sure they don't call outside
-  // this object when Stop() method is ever called. Locking this method is safe
-  // because |lock_| is only acquired in tasks on render thread.
+  // TODO(scherkus): we shouldn't have to lock to signal host(), see
+  // http://crbug.com/113712 for details.
   base::AutoLock auto_lock(lock_);
   if (stop_signal_received_)
     return;
 
-  int64 start = loader_->first_byte_position();
-  if (host() && current_buffered_position > start)
-    host()->AddBufferedByteRange(start, current_buffered_position);
+  bool is_downloading_data;
+  switch (state) {
+    case BufferedResourceLoader::kLoading:
+      is_downloading_data = true;
+      break;
+    case BufferedResourceLoader::kLoadingDeferred:
+      is_downloading_data = false;
+      break;
+
+    // TODO(scherkus): we don't signal network activity changes when loads
+    // complete or fail to preserve existing behaviour when deferring is
+    // toggled, however we considering changing DownloadingCB to also
+    // propagate loading state. For example there isn't any signal today
+    // to notify the client that loading has failed/finished (we only get
+    // errors on subsequent reads).
+    case BufferedResourceLoader::kLoadingFailed:
+    case BufferedResourceLoader::kLoadingFinished:
+      return;
+  }
+
+  downloading_cb_.Run(is_downloading_data);

[Ami GONE FROM CHROMIUM, 2012/07/10 02:43:53]

This is now run under lock, where it didn't use to be. 
Problem?

[scherkus (not reviewing), 2012/07/10 04:10:53]

I don't *think* it should be an issue, but now that downloading_cb != host()
there's no reason for this block of code to be under the lock at all! (the lock
+ stop_signal_received_ is to avoid calling host() which is
Demuxer/Pipeline/etc)

+}
+
+void BufferedDataSource::HttpProgressCallback(int64 position) {
+  DCHECK(MessageLoop::current() == render_loop_);
+  DCHECK(url_.SchemeIs(kHttpScheme) || url_.SchemeIs(kHttpsScheme));
+
+  // TODO(scherkus): we shouldn't have to lock to signal host(), see
+  // http://crbug.com/113712 for details.
+  base::AutoLock auto_lock(lock_);
+  if (stop_signal_received_)
+    return;
+
+  if (host() && position > last_read_start_)
+    host()->AddBufferedByteRange(last_read_start_, position);
 }
 
 void BufferedDataSource::UpdateHostState_Locked() {
-  // Called from various threads, under lock.
   lock_.AssertAcquired();
 
   if (!host())
     return;
 
   if (total_bytes_ != kPositionNotSpecified)
     host()->SetTotalBytes(total_bytes_);
   int64 start = loader_->first_byte_position();
   if (buffered_bytes_ > start)
     host()->AddBufferedByteRange(start, buffered_bytes_);
 }
 
 }  // namespace webkit_media