Make sure we get at least one progress callback

media/blink/multibuffer_data_source.cc

 // Copyright 2013 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 "media/blink/multibuffer_data_source.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 159 matching lines @@
 
   init_cb_ = init_cb;
 
   CreateResourceLoader(0, kPositionNotSpecified);
 
   // We're not allowed to call Wait() if data is already available.
   if (reader_->Available()) {
     render_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&MultibufferDataSource::StartCallback, weak_ptr_));
+
+    // When the entire file is already in the cache, we won't get any more
+    // progress callbacks, which breaks some expectations. Post a task to
+    // make sure that the client gets at least one call each for the progress
+    // and loading callbacks.
+    render_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&MultibufferDataSource::UpdateProgress,
+                              weak_factory_.GetWeakPtr()));
   } else {
     reader_->Wait(1,
                   base::Bind(&MultibufferDataSource::StartCallback, weak_ptr_));
   }
 }
 
 void MultibufferDataSource::OnRedirect(
     const scoped_refptr<UrlData>& destination) {
   if (!destination) {
     // A failure occured.
@@ skipping 308 matching lines @@
     media_log_->SetBooleanProperty("single_origin", single_origin_);
     media_log_->SetBooleanProperty("passed_cors_access_check",
                                    DidPassCORSAccessCheck());
     media_log_->SetBooleanProperty("range_header_supported",
                                    url_data_->range_supported());
   }
 
   render_task_runner_->PostTask(
       FROM_HERE, base::Bind(base::ResetAndReturn(&init_cb_), success));
 
-  // When we tell our clients that we're loading below, it's possible that
-  // the entire file is already cashed. If that's true, then we won't get
-  // any progress callbacks later, so we'll never tell the client that we
-  // stopped loading. Post a task to make sure that we check the loading
-  // state at least once.
-  render_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&MultibufferDataSource::UpdateLoadingState,
-                            weak_factory_.GetWeakPtr()));
   // Even if data is cached, say that we're loading at this point for
   // compatibility.
   UpdateLoadingState_Locked(true);
 }
 
 void MultibufferDataSource::ProgressCallback(int64_t begin, int64_t end) {
   DVLOG(1) << __func__ << "(" << begin << ", " << end << ")";
   DCHECK(render_task_runner_->BelongsToCurrentThread());
 
   if (assume_fully_buffered())

[chcunningham, 2016/10/28 17:53:52]

The tests in the bug have both been http://, but I think the expectations around
progress/loading events are the same even for local files. I'm worried this
early return is bad for the local case.

[hubbe, 2016/10/31 17:25:17]

I'm not entirely sure why we do this, but it's been like this since before
multibuffers. It might be worth noticing that we never even
transition into the "loading" state for local resources. However, we might need
to add layout tests that runs on local files to make sure that WMPI behaves
properly when it gets no loading or progress callbacks.

[chcunningham, 2016/10/31 18:12:39]

Acknowledged.

     return;
 
   base::AutoLock auto_lock(lock_);
 
   if (end > begin) {
     // TODO(scherkus): we shouldn't have to lock to signal host(), see
     // http://crbug.com/113712 for details.
     if (stop_signal_received_)
       return;
 
@@ skipping 17 matching lines @@
 
     if (!loading_ && cancel_on_defer_) {
       reader_.reset(nullptr);
     }
 
     // Callback could kill us, be sure to call it last.
     downloading_cb_.Run(loading_);
   }
 }
 
-void MultibufferDataSource::UpdateLoadingState() {
+void MultibufferDataSource::UpdateProgress() {
   DCHECK(render_task_runner_->BelongsToCurrentThread());
-  base::AutoLock auto_lock(lock_);
-  UpdateLoadingState_Locked(false);
+  if (reader_) {
+    uint64_t available = reader_->Available();
+    uint64_t pos = reader_->Tell();
+    ProgressCallback(pos, pos + available);
+  }
 }
 
 void MultibufferDataSource::UpdateBufferSizes() {
   DVLOG(1) << __func__;
   if (!reader_)
     return;
 
   if (!assume_fully_buffered()) {
     // If the playback has started and the strategy is aggressive, then try to
     // load as much as possible, assuming that the file is cacheable. (If not,
@@ skipping 38 matching lines @@
   reader_->SetPinRange(back_buffer, kMaxBufferPreload + kPreloadHighExtra);
 
   if (preload_ == METADATA) {
     reader_->SetPreload(0, 0);
   } else {
     reader_->SetPreload(preload_high, preload);
   }
 }
 
 }  // namespace media