media: Allow suspend on HTTP 200 videos

media/blink/webmediaplayer_impl.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/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1780 matching lines @@
   if (!success) {
     SetNetworkState(WebMediaPlayer::kNetworkStateFormatError);
 
     // Not really necessary, since the pipeline was never started, but it at
     // least this makes sure that the error handling code is in sync.
     UpdatePlayState();
 
     return;
   }
 
+  // No point in preloading data as we'll probably just throw it away anyways.
+  if (IsStreaming() && preload_ > MultibufferDataSource::METADATA) {
+    data_source_->SetPreload(MultibufferDataSource::METADATA);
+  }
+
   StartPipeline();
 }
 
 void WebMediaPlayerImpl::NotifyDownloading(bool is_downloading) {
   DVLOG(1) << __func__ << "(" << is_downloading << ")";
   if (!is_downloading && network_state_ == WebMediaPlayer::kNetworkStateLoading)
     SetNetworkState(WebMediaPlayer::kNetworkStateIdle);
   else if (is_downloading &&
            network_state_ == WebMediaPlayer::kNetworkStateIdle)
     SetNetworkState(WebMediaPlayer::kNetworkStateLoading);
@@ skipping 259 matching lines @@
   return video_frame;
 }
 
 void WebMediaPlayerImpl::UpdatePlayState() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
 #if defined(OS_ANDROID)  // WMPI_CAST
   bool is_remote = IsRemote();
   bool can_auto_suspend = true;
 #else
+  int64_t size;

[sandersd (OOO until July 31), 2017/07/12 21:19:20]

nit: move after comment (and possibly rename to |unused|).

[hubbe, 2017/07/12 21:28:47]

Done.

   bool is_remote = false;
-  bool can_auto_suspend = !disable_pipeline_auto_suspend_ && !IsStreaming();
+  bool can_auto_suspend = !disable_pipeline_auto_suspend_;
+  // For streaming videos, we only allow suspending at the very beginning of the
+  // video, and only if we know the length of the video. (If we don't know the

[sandersd (OOO until July 31), 2017/07/12 21:19:19]

length -> size

[hubbe, 2017/07/12 21:28:47]

size seems confusing
how about "file size" ?

[sandersd (OOO until July 31), 2017/07/12 21:37:05]

sgtm

+  // length, it might be a dynamically generated video, and suspending will
+  // not work at all.)
+  if (IsStreaming() && ready_state_ != WebMediaPlayer::kReadyStateHaveNothing &&
+      CurrentTime() != 0.0 && data_source_ && data_source_->GetSize(&size)) {

[sandersd (OOO until July 31), 2017/07/12 21:19:20]

What's the reasoning for kReadyStateHaveNothing? It seems like we should be fine
to do this whenever CurrentTime() is 0.0. Does the data source not create a new
connection in those cases?

[hubbe, 2017/07/12 21:28:47]

Not allowed to call CurrentTime() if ready_state_ is kReadyStateHaveNothing

[sandersd (OOO until July 31), 2017/07/12 21:37:05]

Isn't that the same as saying that we can skip the time check entirely before
then?

[hubbe, 2017/07/12 22:04:23]

I think I had some of this logic wrong.
Changed it so to be more obvious what's going on.
Better now?

+    can_auto_suspend = false;
+  }
 #endif
 
   bool is_suspended = pipeline_controller_.IsSuspended();
   bool is_backgrounded = IsBackgroundedSuspendEnabled() && IsHidden();
   PlayState state = UpdatePlayState_ComputePlayState(
       is_remote, can_auto_suspend, is_suspended, is_backgrounded);
   SetDelegateState(state.delegate_state, state.is_idle);
   SetMemoryReportingState(state.is_memory_reporting_enabled);
   SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_);
 }
@@ skipping 541 matching lines @@
 }
 
 #undef UMA_HISTOGRAM_VIDEO_HEIGHT
 
 void WebMediaPlayerImpl::SetTickClockForTest(base::TickClock* tick_clock) {
   tick_clock_.reset(tick_clock);
   buffered_data_source_host_.SetTickClockForTest(tick_clock);
 }
 
 }  // namespace media