Commit dc5d47c8 authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

Revert "Remove kHaveFutureData restriction from play state notifications."

This reverts commit b0ec93f6.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=812423

Original change's description:
> Remove kHaveFutureData restriction from play state notifications.
> 
> This reland marks media-src-suspend-after-have-future-data and
> media-src-suspend-after-have-metadata as SlowTest(s) now that they no
> longer permanently time out. Each takes ~2ish seconds in release build
> and so meets the criteria for being a SlowTest.
> 
> This also changes WMPI to not try and get the current time value prior
> to the HaveCurrentData state (which Blink already respects).
> 
> ========= [ Original Description Follows ] ===============
> 
> HTMLMediaElement should tell WebMediaPlayer about play state changes
> as soon as the kHaveMetaData state is reached.
> 
> This will eventually allow us to delete a bunch of guessing-game type
> code for idle suspension and instead rely on actually getting a play()
> call from Blink to wake up when needed.
> 
> We can't stop WMPI from reaching the future data state quite yet though,
> because it has implications on the delivery of 'canplay' and 'canplaythrough'
> events that need to be part of a larger preload=metadata experiment. See old
> discussion on this here:
> 
> https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html
> 
> Note: This change exposed some bugs in how the ready state maximum is used
> and the states at which a video frame is expected. These issues are fixed
> in the HTMLVideoElement.
> 
> BUG=694855, 756897, 809998
> TEST=existing suspend tests no longer flake.
> 
> Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf
> Reviewed-on: https://chromium-review.googlesource.com/915081
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#536487}

TBR=dalecurtis@chromium.org,mlamouri@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 694855, 756897, 809998
Change-Id: I176b5116682851853ff2773e6e4a77a9ac2664b2
Reviewed-on: https://chromium-review.googlesource.com/920481Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536988}
parent d8ec149f
...@@ -671,10 +671,7 @@ void WebMediaPlayerImpl::Pause() { ...@@ -671,10 +671,7 @@ void WebMediaPlayerImpl::Pause() {
#endif #endif
pipeline_controller_.SetPlaybackRate(0.0); pipeline_controller_.SetPlaybackRate(0.0);
paused_time_ = pipeline_controller_.GetMediaTime();
// For states <= have_metadata, we may not have a renderer or current time.
if (highest_ready_state_ > WebMediaPlayer::kReadyStateHaveMetadata)
paused_time_ = pipeline_controller_.GetMediaTime();
if (observer_) if (observer_)
observer_->OnPaused(); observer_->OnPaused();
......
...@@ -132,9 +132,7 @@ crbug.com/510337 cssom/cssvalue-comparison.html [ Slow ] ...@@ -132,9 +132,7 @@ crbug.com/510337 cssom/cssvalue-comparison.html [ Slow ]
crbug.com/680917 http/tests/devtools/audits2/ [ Slow ] crbug.com/680917 http/tests/devtools/audits2/ [ Slow ]
# These tests are slow since they use throttled loading to affect ready states. # This test is intentionally SLOW as we're waiting for a connection timeout.
crbug.com/756897 http/tests/media/media-src-suspend-after-have-metadata.html [ Slow ]
crbug.com/756897 http/tests/media/media-src-suspend-after-have-future-data.html [ Slow ]
crbug.com/73609 http/tests/media/video-play-stall.html [ Slow ] crbug.com/73609 http/tests/media/video-play-stall.html [ Slow ]
# Many of the virtual animations tests are slow. # Many of the virtual animations tests are slow.
......
...@@ -2711,6 +2711,10 @@ crbug.com/745079 external/wpt/geolocation-API/PositionOptions.https.html [ Failu ...@@ -2711,6 +2711,10 @@ crbug.com/745079 external/wpt/geolocation-API/PositionOptions.https.html [ Failu
# Sheriff failures 2017-07-03 # Sheriff failures 2017-07-03
crbug.com/708994 http/tests/security/cross-frame-mouse-source-capabilities.html [ Timeout Pass ] crbug.com/708994 http/tests/security/cross-frame-mouse-source-capabilities.html [ Timeout Pass ]
# Flaky until play state changes are propogated to WebMediaPlayer in all states.
crbug.com/694855 http/tests/media/media-src-suspend-after-have-metadata.html [ Timeout Pass ]
crbug.com/694855 http/tests/media/media-src-suspend-after-have-future-data.html [ Timeout Pass ]
crbug.com/745887 [ Mac Win ] fast/frames/sandboxed-iframe-plugins.html [ Failure Pass ] crbug.com/745887 [ Mac Win ] fast/frames/sandboxed-iframe-plugins.html [ Failure Pass ]
crbug.com/626703 [ Win7 ] external/wpt/domxpath/xml_xpath_runner.html [ Timeout Pass ] crbug.com/626703 [ Win7 ] external/wpt/domxpath/xml_xpath_runner.html [ Timeout Pass ]
......
...@@ -1751,8 +1751,8 @@ void HTMLMediaElement::SetReadyState(ReadyState state) { ...@@ -1751,8 +1751,8 @@ void HTMLMediaElement::SetReadyState(ReadyState state) {
ready_state_ = kHaveCurrentData; ready_state_ = kHaveCurrentData;
} }
if (new_state > ready_state_maximum_) if (old_state > ready_state_maximum_)
ready_state_maximum_ = new_state; ready_state_maximum_ = old_state;
if (network_state_ == kNetworkEmpty) if (network_state_ == kNetworkEmpty)
return; return;
...@@ -3363,9 +3363,13 @@ TimeRanges* HTMLMediaElement::seekable() const { ...@@ -3363,9 +3363,13 @@ TimeRanges* HTMLMediaElement::seekable() const {
} }
bool HTMLMediaElement::PotentiallyPlaying() const { bool HTMLMediaElement::PotentiallyPlaying() const {
// Once we've reached the metadata state the WebMediaPlayer is ready to accept // "pausedToBuffer" means the media engine's rate is 0, but only because it
// play state changes. // had to stop playing when it ran out of buffered data. A movie in this state
return ready_state_ >= kHaveMetadata && CouldPlayIfEnoughData(); // is "potentially playing", modulo the checks in couldPlayIfEnoughData().
bool paused_to_buffer =
ready_state_maximum_ >= kHaveFutureData && ready_state_ < kHaveFutureData;
return (paused_to_buffer || ready_state_ >= kHaveFutureData) &&
CouldPlayIfEnoughData();
} }
bool HTMLMediaElement::CouldPlayIfEnoughData() const { bool HTMLMediaElement::CouldPlayIfEnoughData() const {
......
...@@ -387,10 +387,9 @@ bool HTMLVideoElement::HasAvailableVideoFrame() const { ...@@ -387,10 +387,9 @@ bool HTMLVideoElement::HasAvailableVideoFrame() const {
if (!GetWebMediaPlayer()) if (!GetWebMediaPlayer())
return false; return false;
// The ready state maximum is used here instead of the current ready state
// since a frame is still available during a seek.
return GetWebMediaPlayer()->HasVideo() && return GetWebMediaPlayer()->HasVideo() &&
ready_state_maximum_ >= kHaveCurrentData; GetWebMediaPlayer()->GetReadyState() >=
WebMediaPlayer::kReadyStateHaveCurrentData;
} }
void HTMLVideoElement::webkitEnterFullscreen() { void HTMLVideoElement::webkitEnterFullscreen() {
......
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