Skip to content

GitLab

T
tangled
Commit 69a2ffa1 authored by Mounir Lamouri's avatar Mounir Lamouri Committed by Commit Bot
Browse files
Options

HTMLMediaElement: asynchronously pause a video when removed from document. 

This changes follow the spec with regards to pausing a video when it
leaves a document: when a video leaves a document, the removal process
should continue and, asynchronously, Blink checks that the video is
still in an active document. It allows a video to move from one part of
the document to another without pausing. Moving into a document or
into two different outside of the document trees will also not pause.

This is following the specification and Edge/Safari/Firefox
implementations.

Bug: 382879, 490511
Change-Id: I48abdaa5f81790e2cae7edc7b0bfb444f6ee7787
Reviewed-on: https://chromium-review.googlesource.com/c/1074808
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597242}
parent f4d302c1
Show whitespace changes
Inline Side-by-side
Showing with 42 additions and 27 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 69a2ffa1
...@@ -3841,8 +3841,6 @@ crbug.com/655458 external/wpt/workers/semantics/multiple-workers/007.html [ Time ...@@ -3841,8 +3841,6 @@ crbug.com/655458 external/wpt/workers/semantics/multiple-workers/007.html [ Time
# worker script, because the script url has a .html file extension. # worker script, because the script url has a .html file extension.
crbug.com/655458 external/wpt/workers/semantics/multiple-workers/003.html [ Timeout ] crbug.com/655458 external/wpt/workers/semantics/multiple-workers/003.html [ Timeout ]
crbug.com/490511 external/wpt/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-remove-from-document-networkState.html [ Timeout ]
crbug.com/435547 http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html [ Skip ] crbug.com/435547 http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html [ Skip ]
crbug.com/697971 [ Mac10.12 ] fast/text/selection/flexbox-selection-nested.html [ Skip ] crbug.com/697971 [ Mac10.12 ] fast/text/selection/flexbox-selection-nested.html [ Skip ]
......
third_party/WebKit/LayoutTests/external/wpt/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-within-document-expected.txt deleted 100644 → 0
View file @ f4d302c1
This is a testharness.js-based test.
FAIL paused state when moving within a document assert_false: paused after moving expected false got true
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/external/wpt/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-remove-from-document-expected.txt deleted 100644 → 0
View file @ f4d302c1
This is a testharness.js-based test.
FAIL paused state when removing from a document assert_false: paused after removing expected false got true
Harness: the test ran to completion.
third_party/WebKit/LayoutTests/media/autoplay/document-user-activation.html
View file @ 69a2ffa1
...@@ -88,7 +88,7 @@ promise_test(t => { ...@@ -88,7 +88,7 @@ promise_test(t => {
// TODO(mlamouri): should not allowed to autoplay if the document was scrolled // TODO(mlamouri): should not allowed to autoplay if the document was scrolled
// with touch events. // with touch events.
promise_test(t => { promise_test(async t => {
t.add_cleanup(() => { t.add_cleanup(() => {
internals.settings.setAutoplayPolicy('no-user-gesture-required'); internals.settings.setAutoplayPolicy('no-user-gesture-required');
}); });
...@@ -111,6 +111,13 @@ promise_test(t => { ...@@ -111,6 +111,13 @@ promise_test(t => {
video.src = '../content/test.ogv'; video.src = '../content/test.ogv';
document.body.appendChild(video); // Moved to `window.document`. document.body.appendChild(video); // Moved to `window.document`.
document.body.removeChild(video); // To avoid polluting the DOM. document.body.removeChild(video); // To avoid polluting the DOM.
await new Promise(resolve => {
video.addEventListener('volumechange', resolve, { once: true });
video.volume = 0.5;
});
return video.play(); return video.play();
}, 'After being moved to a document with user gesture, autoplay is allowed'); }, 'After being moved to a document with user gesture, autoplay is allowed');
......
third_party/WebKit/LayoutTests/media/remove-from-document-config-controls-no-crash.html
View file @ 69a2ffa1
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
<script src="../resources/js-test.js"></script> <script src="../resources/js-test.js"></script>
</head> </head>
<body> <body>
<video id="v"></video> <video controls id="v"></video>
<script> <script>
description("Verify that removing a video element from the DOM does not crash."); description("Verify that removing a video element from the DOM does not crash.");
......
third_party/WebKit/LayoutTests/media/remove-from-document.html
View file @ 69a2ffa1
...@@ -5,17 +5,20 @@ ...@@ -5,17 +5,20 @@
<video autoplay></video> <video autoplay></video>
<script> <script>
async_test(function(t) { async_test(function(t) {
var video = document.querySelector("video"); const video = document.querySelector("video");
video.src = "content/test.ogv"; video.src = "content/test.ogv";
video.oncanplaythrough = t.step_func_done(function() { video.oncanplaythrough = t.step_func(() => {
assert_not_equals(video.networkState, HTMLMediaElement.NETWORK_EMPTY); assert_not_equals(video.networkState, HTMLMediaElement.NETWORK_EMPTY);
assert_false(video.paused); assert_false(video.paused);
document.body.removeChild(video); document.body.removeChild(video);
assert_not_equals(video.networkState, HTMLMediaElement.NETWORK_EMPTY); assert_not_equals(video.networkState, HTMLMediaElement.NETWORK_EMPTY);
setTimeout(t.step_func_done(() => {
assert_true(video.paused); assert_true(video.paused);
}));
}); });
}); });
</script> </script>
third_party/blink/renderer/core/html/media/html_media_element.cc
View file @ 69a2ffa1
...@@ -478,6 +478,10 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tag_name, ...@@ -478,6 +478,10 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tag_name,
document.GetTaskRunner(TaskType::kInternalMedia), document.GetTaskRunner(TaskType::kInternalMedia),
this, this,
&HTMLMediaElement::CheckViewportIntersectionTimerFired), &HTMLMediaElement::CheckViewportIntersectionTimerFired),
removed_from_document_timer_(
document.GetTaskRunner(TaskType::kInternalMedia),
this,
&HTMLMediaElement::OnRemovedFromDocumentTimerFired),
played_time_ranges_(), played_time_ranges_(),
async_event_queue_(EventQueue::Create(GetExecutionContext(), async_event_queue_(EventQueue::Create(GetExecutionContext(),
TaskType::kMediaElementEvent)), TaskType::kMediaElementEvent)),
...@@ -578,6 +582,8 @@ void HTMLMediaElement::DidMoveToNewDocument(Document& old_document) { ...@@ -578,6 +582,8 @@ void HTMLMediaElement::DidMoveToNewDocument(Document& old_document) {
GetDocument().GetTaskRunner(TaskType::kInternalMedia)); GetDocument().GetTaskRunner(TaskType::kInternalMedia));
deferred_load_timer_.MoveToNewTaskRunner( deferred_load_timer_.MoveToNewTaskRunner(
GetDocument().GetTaskRunner(TaskType::kInternalMedia)); GetDocument().GetTaskRunner(TaskType::kInternalMedia));
removed_from_document_timer_.MoveToNewTaskRunner(
GetDocument().GetTaskRunner(TaskType::kInternalMedia));
autoplay_policy_->DidMoveToNewDocument(old_document); autoplay_policy_->DidMoveToNewDocument(old_document);
...@@ -729,11 +735,9 @@ void HTMLMediaElement::RemovedFrom(ContainerNode& insertion_point) { ...@@ -729,11 +735,9 @@ void HTMLMediaElement::RemovedFrom(ContainerNode& insertion_point) {
BLINK_MEDIA_LOG << "removedFrom(" << (void*)this << ", " << insertion_point BLINK_MEDIA_LOG << "removedFrom(" << (void*)this << ", " << insertion_point
<< ")"; << ")";
removed_from_document_timer_.StartOneShot(TimeDelta(), FROM_HERE);
HTMLElement::RemovedFrom(insertion_point); HTMLElement::RemovedFrom(insertion_point);
if (insertion_point.InActiveDocument()) {
if (network_state_ > kNetworkEmpty)
PauseInternal();
}
} }
void HTMLMediaElement::AttachLayoutTree(AttachContext& context) { void HTMLMediaElement::AttachLayoutTree(AttachContext& context) {
...@@ -1003,7 +1007,7 @@ void HTMLMediaElement::InvokeResourceSelectionAlgorithm() { ...@@ -1003,7 +1007,7 @@ void HTMLMediaElement::InvokeResourceSelectionAlgorithm() {
// 3 - Set the media element's delaying-the-load-event flag to true (this // 3 - Set the media element's delaying-the-load-event flag to true (this
// delays the load event) // delays the load event)
SetShouldDelayLoadEvent(true); SetShouldDelayLoadEvent(true);
if (GetMediaControls()) if (GetMediaControls() && isConnected())
GetMediaControls()->Reset(); GetMediaControls()->Reset();
// 4 - Await a stable state, allowing the task that invoked this algorithm to // 4 - Await a stable state, allowing the task that invoked this algorithm to
...@@ -3614,6 +3618,7 @@ void HTMLMediaElement::ContextDestroyed(ExecutionContext*) { ...@@ -3614,6 +3618,7 @@ void HTMLMediaElement::ContextDestroyed(ExecutionContext*) {
GetLayoutObject()->UpdateFromElement(); GetLayoutObject()->UpdateFromElement();
StopPeriodicTimers(); StopPeriodicTimers();
removed_from_document_timer_.Stop();
} }
bool HTMLMediaElement::HasPendingActivity() const { bool HTMLMediaElement::HasPendingActivity() const {
...@@ -4149,6 +4154,13 @@ EnumerationHistogram& HTMLMediaElement::ShowControlsHistogram() const { ...@@ -4149,6 +4154,13 @@ EnumerationHistogram& HTMLMediaElement::ShowControlsHistogram() const {
return histogram; return histogram;
} }
void HTMLMediaElement::OnRemovedFromDocumentTimerFired(TimerBase*) {
if (InActiveDocument())
return;
PauseInternal();
}
void HTMLMediaElement::ClearWeakMembers(Visitor* visitor) { void HTMLMediaElement::ClearWeakMembers(Visitor* visitor) {
if (!ThreadHeap::IsHeapObjectAlive(audio_source_node_)) { if (!ThreadHeap::IsHeapObjectAlive(audio_source_node_)) {
GetAudioSourceProvider().SetClient(nullptr); GetAudioSourceProvider().SetClient(nullptr);
......
third_party/blink/renderer/core/html/media/html_media_element.h
View file @ 69a2ffa1
...@@ -560,11 +560,14 @@ class CORE_EXPORT HTMLMediaElement ...@@ -560,11 +560,14 @@ class CORE_EXPORT HTMLMediaElement
void OnVisibilityChangedForLazyLoad(bool); void OnVisibilityChangedForLazyLoad(bool);
void OnRemovedFromDocumentTimerFired(TimerBase*);
TaskRunnerTimer<HTMLMediaElement> load_timer_; TaskRunnerTimer<HTMLMediaElement> load_timer_;
TaskRunnerTimer<HTMLMediaElement> progress_event_timer_; TaskRunnerTimer<HTMLMediaElement> progress_event_timer_;
TaskRunnerTimer<HTMLMediaElement> playback_progress_timer_; TaskRunnerTimer<HTMLMediaElement> playback_progress_timer_;
TaskRunnerTimer<HTMLMediaElement> audio_tracks_timer_; TaskRunnerTimer<HTMLMediaElement> audio_tracks_timer_;
TaskRunnerTimer<HTMLMediaElement> check_viewport_intersection_timer_; TaskRunnerTimer<HTMLMediaElement> check_viewport_intersection_timer_;
TaskRunnerTimer<HTMLMediaElement> removed_from_document_timer_;
Member<TimeRanges> played_time_ranges_; Member<TimeRanges> played_time_ranges_;
Member<EventQueue> async_event_queue_; Member<EventQueue> async_event_queue_;
......
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