media: Allow suspend on HTTP 200 videos

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index c558870dab049b6f1872137ee80caf5f9150e5bd..4e03a9be43ce412939a1c935cdd2a6ae9a7773dd 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -1798,6 +1798,11 @@ void WebMediaPlayerImpl::DataSourceInitialized(bool success) {
     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();
 }
 
@@ -2077,8 +2082,17 @@ void WebMediaPlayerImpl::UpdatePlayState() {
   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();