Commit 2ac85ff9 authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

[multibuffer] Always clear cancel_upon_defer_ when playback starts.

During the changes made for preload=metadata suspend, it was assumed
that OnBufferingHaveEnough() had no lingering effects after the next
call to MediaIsPlaying(). It turns out this was not true. The method
would only clear |cancel_upon_defer_| the first time.

The fix is to always clear |cancel_upon_defer_| once the DataSource
has been informed of playback. A new unittest verifies this.

BUG=1004530
TEST=new unittest, clip from bug does not create more than 1 loader
after a seek from the suspended state.

R=tguilbert

Change-Id: I7655d7ff0ee0ba1024ec0486331d84aeda16918c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807160
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarThomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697421}
parent 3c3d0664
......@@ -304,11 +304,16 @@ void MultibufferDataSource::MediaPlaybackRateChanged(double playback_rate) {
void MultibufferDataSource::MediaIsPlaying() {
DCHECK(render_task_runner_->BelongsToCurrentThread());
// Always clear this since it can be set by OnBufferingHaveEnough() calls at
// any point in time.
cancel_on_defer_ = false;
if (media_has_played_)
return;
media_has_played_ = true;
cancel_on_defer_ = false;
// Once we start playing, we need preloading.
preload_ = AUTO;
UpdateBufferSizes();
......
......@@ -1305,6 +1305,7 @@ TEST_F(MultibufferDataSourceTest,
// Marking the media as playing should prevent deferral. It also tells the
// data source to start buffering beyond the initial load.
EXPECT_FALSE(data_source_->cancel_on_defer_for_testing());
data_source_->MediaIsPlaying();
data_source_->OnBufferingHaveEnough(false);
CheckCapacityDefer();
......@@ -1316,6 +1317,7 @@ TEST_F(MultibufferDataSourceTest,
ReceiveData(kDataSize);
ASSERT_TRUE(active_loader());
data_source_->OnBufferingHaveEnough(true);
EXPECT_TRUE(data_source_->cancel_on_defer_for_testing());
ASSERT_TRUE(active_loader());
ASSERT_FALSE(data_provider()->deferred());
......@@ -1329,6 +1331,26 @@ TEST_F(MultibufferDataSourceTest,
EXPECT_GT(bytes_received, 0);
EXPECT_LT(bytes_received + kDataSize, kFileSize);
EXPECT_FALSE(active_loader_allownull());
// Verify playback resumes correctly too.
data_source_->MediaIsPlaying();
EXPECT_FALSE(data_source_->cancel_on_defer_for_testing());
// A read from a previously buffered range won't create a new loader yet.
EXPECT_CALL(*this, ReadCallback(kDataSize));
ReadAt(kDataSize);
EXPECT_FALSE(active_loader_allownull());
// Reads from an unbuffered range will though...
EXPECT_CALL(*this, ReadCallback(kDataSize));
ReadAt(kFarReadPosition);
// Receive enough data to exhaust current capacity which would destroy the
// loader upon deferral if the flag hasn't been cleared properly.
for (int i = 0; i <= (preload_high() / kDataSize) + 1; ++i) {
ReceiveData(kDataSize);
ASSERT_TRUE(active_loader());
}
}
TEST_F(MultibufferDataSourceTest, SeekPastEOF) {
......
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