Commit e2cc88c0 authored by hubbe's avatar hubbe Committed by Commit Bot

media: Allow suspend on HTTP 200 videos

Currently, connections are kept open indefinitely, which
can block future loads from a server if there are 6 or more
<video> tags on the same page.  This should fix that in most
cases.

BUG=716748

Review-Url: https://codereview.chromium.org/2979893002
Cr-Commit-Position: refs/heads/master@{#486919}
parent 69c312ef
...@@ -601,14 +601,15 @@ TEST_F(MultibufferDataSourceTest, Range_WrongContentRange) { ...@@ -601,14 +601,15 @@ TEST_F(MultibufferDataSourceTest, Range_WrongContentRange) {
TEST_F(MultibufferDataSourceTest, Range_ServerLied) { TEST_F(MultibufferDataSourceTest, Range_ServerLied) {
InitializeWith206Response(); InitializeWith206Response();
// Read causing a new request to be made -- we'll expect it to error. // Read causing a new request to be made, we will discard the data that
// was already read in the first request.
ReadAt(kFarReadPosition); ReadAt(kFarReadPosition);
// Return a 200 in response to a range request. // Return a 200 in response to a range request.
EXPECT_CALL(*this, ReadCallback(media::DataSource::kReadError)); EXPECT_CALL(*this, ReadCallback(media::DataSource::kReadError));
Respond(response_generator_->Generate200()); Respond(response_generator_->Generate200());
EXPECT_FALSE(loading()); EXPECT_TRUE(loading());
Stop(); Stop();
} }
......
...@@ -287,6 +287,7 @@ void ResourceMultiBufferDataProvider::DidReceiveResponse( ...@@ -287,6 +287,7 @@ void ResourceMultiBufferDataProvider::DidReceiveResponse(
int64_t content_length = response.ExpectedContentLength(); int64_t content_length = response.ExpectedContentLength();
bool end_of_file = false; bool end_of_file = false;
bool do_fail = false; bool do_fail = false;
bytes_to_discard_ = 0;
// We make a strong assumption that when we reach here we have either // We make a strong assumption that when we reach here we have either
// received a response from HTTP/HTTPS protocol or the request was // received a response from HTTP/HTTPS protocol or the request was
...@@ -308,11 +309,12 @@ void ResourceMultiBufferDataProvider::DidReceiveResponse( ...@@ -308,11 +309,12 @@ void ResourceMultiBufferDataProvider::DidReceiveResponse(
if (partial_response && if (partial_response &&
VerifyPartialResponse(response, destination_url_data)) { VerifyPartialResponse(response, destination_url_data)) {
destination_url_data->set_range_supported(); destination_url_data->set_range_supported();
} else if (ok_response && pos_ == 0) { } else if (ok_response) {
// We accept a 200 response for a Range:0- request, trusting the // We accept a 200 response for a Range:0- request, trusting the
// Accept-Ranges header, because Apache thinks that's a reasonable thing // Accept-Ranges header, because Apache thinks that's a reasonable thing
// to return. // to return.
destination_url_data->set_length(content_length); destination_url_data->set_length(content_length);
bytes_to_discard_ = byte_pos();
} else if (response.HttpStatusCode() == kHttpRangeNotSatisfiable) { } else if (response.HttpStatusCode() == kHttpRangeNotSatisfiable) {
// Unsatisfiable range // Unsatisfiable range
// Really, we should never request a range that doesn't exist, but // Really, we should never request a range that doesn't exist, but
...@@ -386,6 +388,15 @@ void ResourceMultiBufferDataProvider::DidReceiveData(const char* data, ...@@ -386,6 +388,15 @@ void ResourceMultiBufferDataProvider::DidReceiveData(const char* data,
DCHECK(active_loader_); DCHECK(active_loader_);
DCHECK_GT(data_length, 0); DCHECK_GT(data_length, 0);
if (bytes_to_discard_) {
uint64_t tmp = std::min<uint64_t>(bytes_to_discard_, data_length);
data_length -= tmp;
data += tmp;
bytes_to_discard_ -= tmp;
if (data_length == 0)
return;
}
// When we receive data, we allow more retries. // When we receive data, we allow more retries.
retries_ = 0; retries_ = 0;
...@@ -561,9 +572,13 @@ bool ResourceMultiBufferDataProvider::VerifyPartialResponse( ...@@ -561,9 +572,13 @@ bool ResourceMultiBufferDataProvider::VerifyPartialResponse(
url_data->set_length(instance_size); url_data->set_length(instance_size);
} }
if (byte_pos() != first_byte_position) { if (first_byte_position > byte_pos()) {
return false;
}
if (last_byte_position + 1 < byte_pos()) {
return false; return false;
} }
bytes_to_discard_ = byte_pos() - first_byte_position;
return true; return true;
} }
......
...@@ -119,6 +119,10 @@ class MEDIA_BLINK_EXPORT ResourceMultiBufferDataProvider ...@@ -119,6 +119,10 @@ class MEDIA_BLINK_EXPORT ResourceMultiBufferDataProvider
// When we encounter a redirect, this is the source of the redirect. // When we encounter a redirect, this is the source of the redirect.
GURL redirects_to_; GURL redirects_to_;
// If the server tries to gives us more bytes than we want, this how
// many bytes we need to discard before we get to the right place.
uint64_t bytes_to_discard_ = 0;
base::WeakPtrFactory<ResourceMultiBufferDataProvider> weak_factory_; base::WeakPtrFactory<ResourceMultiBufferDataProvider> weak_factory_;
}; };
......
...@@ -1820,6 +1820,11 @@ void WebMediaPlayerImpl::DataSourceInitialized(bool success) { ...@@ -1820,6 +1820,11 @@ void WebMediaPlayerImpl::DataSourceInitialized(bool success) {
return; 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(); StartPipeline();
} }
...@@ -2100,7 +2105,18 @@ void WebMediaPlayerImpl::UpdatePlayState() { ...@@ -2100,7 +2105,18 @@ void WebMediaPlayerImpl::UpdatePlayState() {
bool can_auto_suspend = true; bool can_auto_suspend = true;
#else #else
bool is_remote = false; 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 length, it might be a dynamically generated video, and suspending
// will not work at all.)
if (IsStreaming()) {
bool at_beginning =
ready_state_ == WebMediaPlayer::kReadyStateHaveNothing ||
CurrentTime() == 0.0;
if (!at_beginning || GetPipelineMediaDuration() == kInfiniteDuration)
can_auto_suspend = false;
}
#endif #endif
bool is_suspended = pipeline_controller_.IsSuspended(); bool is_suspended = pipeline_controller_.IsSuspended();
......
...@@ -11,7 +11,7 @@ async_test(function(t) ...@@ -11,7 +11,7 @@ async_test(function(t)
var mediaFile = findMediaFile("video", "../../../media/content/test"); var mediaFile = findMediaFile("video", "../../../media/content/test");
var type = mimeTypeForExtension(mediaFile.split(".").pop()); var type = mimeTypeForExtension(mediaFile.split(".").pop());
v.src = "http://127.0.0.1:8000/media/video-throttled-load.cgi?name=" + mediaFile + "&throttle=50&type=" + type; v.src = "http://127.0.0.1:8000/media/video-throttled-load.cgi?name=" + mediaFile + "&throttle=50&type=" + type;
v.onloadeddata = t.step_func(function() v.onloadstart = t.step_func(function()
{ {
assert_equals(v.networkState, v.NETWORK_LOADING); assert_equals(v.networkState, v.NETWORK_LOADING);
// The delaying-the-load-event flag is now false. // The delaying-the-load-event flag is now false.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment