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 webkit_media {
 
 // 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.
 static const int kInitialReadBufferSize = 32768;
 
 // Number of cache misses we allow for a single Read() before signalling an
 // error.
 static const int kNumCacheMissRetries = 3;
 
+// Non-HTTP resources are assumed to be fully loaded so we ignore any
+// loading/progress related callbacks.

[scherkus (not reviewing), 2012/07/06 16:28:35]

see http://codereview.chromium.org/10692106/ -- I ended up deleting this

[Ami GONE FROM CHROMIUM, 2012/07/09 03:25:10]

FTR, you mean https://chromiumcodereview.appspot.com/10735016/

+static void NonHttpLoadingCallback(BufferedResourceLoader::LoadingState) {}
+static void NonHttpProgressCallback(int64) {}
+
 BufferedDataSource::BufferedDataSource(
     MessageLoop* render_loop,
     WebFrame* frame,
     media::MediaLog* media_log)
     : total_bytes_(kPositionNotSpecified),
       buffered_bytes_(0),
       streaming_(false),
       frame_(frame),
       loader_(NULL),
       is_downloading_data_(false),
@@ skipping 53 matching lines @@
   url_ = url;
 
   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::HttpLoadingCallback, 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(&NonHttpLoadingCallback),
+      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 127 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::HttpLoadingCallback, this),
+        base::Bind(&BufferedDataSource::HttpProgressCallback, this),
+        frame_);
+  } else {
+    loader_->Start(
+        base::Bind(&BufferedDataSource::NonHttpInitialStartCallback, this),
+        base::Bind(&NonHttpLoadingCallback),
+        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(last_read_start_, total_bytes_);
     }
   }
   DoneRead_Locked(bytes_read);
 }
 
-void BufferedDataSource::NetworkEventCallback() {
+void BufferedDataSource::HttpLoadingCallback(
+    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;
-
-  bool is_downloading_data = 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;
 
+  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 SetNetworkActivity() should use a similar enum as 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;
+  }
+
   if (is_downloading_data != is_downloading_data_) {
     is_downloading_data_ = is_downloading_data;
     if (host())
       host()->SetNetworkActivity(is_downloading_data);
   }
+}
 
-  if (host() && current_buffered_position > last_read_start_)
-    host()->AddBufferedByteRange(last_read_start_, current_buffered_position);
+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_);
   if (buffered_bytes_ > last_read_start_)
     host()->AddBufferedByteRange(last_read_start_, buffered_bytes_);
 }
 
 }  // namespace webkit_media