- Revision
- 203931
- Author
- dba...@webkit.org
- Date
- 2016-07-29 17:30:13 -0700 (Fri, 29 Jul 2016)
Log Message
Crash under HTMLMediaElement::{resolve, reject}PendingPlayPromises() when playback is interrupted
https://bugs.webkit.org/show_bug.cgi?id=160366
<rdar://problem/27317407>
Reviewed by Eric Carlson.
Source/WebCore:
Fixes a crash/assertion failure in DeferredWrapper::{resolve, rejectWithValue}() caused by a Promise
being settled twice. In particular, if a system interruption occurs when media.play() is invoked
the returned Promise may ultimately be settled twice upon cessation of the interruption.
A Promise can be settled (resolved) exactly once. When a system interruption occurs media
playback is paused and resumes on cessation of the interruption. Currently we also immediately
reject the Promise p retuned by media.play() if the interruption occurs during its invocation.
So, when we resume playback on cessation of an interruption we try to resolve p again. But a
Promise can only be resolved once and hence we violate the assertions that p has both a valid
reference to a JSPromiseDeferred object and a reference to the global object of the page.
Tests: media/non-existent-video-playback-interrupted.html
media/video-playback-interrupted.html
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::play): Modified to reject the Promise and return immediately if
playInternal() returns false.
(WebCore::HTMLMediaElement::playInternal): Treat an interruption as success and return true
so that HTMLMediaElement::play() adds the Promise to the end of the list of pending promises.
We treat an interruption as a success because we will resume playback (assuming the media
can be loaded and is well-formed) upon cessation of the interruption and therefore can either
fulfill or reject the Promise object returned by media.play().
LayoutTests:
* media/non-existent-video-playback-interrupted-expected.txt: Added.
* media/non-existent-video-playback-interrupted.html: Added.
* media/video-playback-interrupted-expected.txt: Added.
* media/video-playback-interrupted.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (203930 => 203931)
--- trunk/LayoutTests/ChangeLog 2016-07-30 00:13:49 UTC (rev 203930)
+++ trunk/LayoutTests/ChangeLog 2016-07-30 00:30:13 UTC (rev 203931)
@@ -1,3 +1,16 @@
+2016-07-29 Daniel Bates <daba...@apple.com>
+
+ Crash under HTMLMediaElement::{resolve, reject}PendingPlayPromises() when playback is interrupted
+ https://bugs.webkit.org/show_bug.cgi?id=160366
+ <rdar://problem/27317407>
+
+ Reviewed by Eric Carlson.
+
+ * media/non-existent-video-playback-interrupted-expected.txt: Added.
+ * media/non-existent-video-playback-interrupted.html: Added.
+ * media/video-playback-interrupted-expected.txt: Added.
+ * media/video-playback-interrupted.html: Added.
+
2016-07-29 Ryan Haddad <ryanhad...@apple.com>
Land test expectations for rdar://problem/27611932.
Added: trunk/LayoutTests/media/non-existent-video-playback-interrupted-expected.txt (0 => 203931)
--- trunk/LayoutTests/media/non-existent-video-playback-interrupted-expected.txt (rev 0)
+++ trunk/LayoutTests/media/non-existent-video-playback-interrupted-expected.txt 2016-07-30 00:30:13 UTC (rev 203931)
@@ -0,0 +1,11 @@
+
+RUN(internals.setMediaSessionRestrictions("video", "InterruptedPlaybackNotPermitted"))
+RUN(video.src = ""
+EXPECTED (video.paused == 'true') OK
+RUN(internals.beginMediaSessionInterruption("System"))
+RUN(video.play().then(didResolvePromise).catch(didRejectPromise))
+RUN(internals.endMediaSessionInterruption("MayResumePlaying"))
+Promise rejected. OK
+EXPECTED (error.name == 'NotSupportedError') OK
+END OF TEST
+
Added: trunk/LayoutTests/media/non-existent-video-playback-interrupted.html (0 => 203931)
--- trunk/LayoutTests/media/non-existent-video-playback-interrupted.html (rev 0)
+++ trunk/LayoutTests/media/non-existent-video-playback-interrupted.html 2016-07-30 00:30:13 UTC (rev 203931)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+var error;
+
+function didResolvePromise()
+{
+ logResult(Failed, "Expected promise to be rejected. Was resolved.");
+
+ // Wait some time before ending the test towards ensuring that we ended the session interruption.
+ endTestLater();
+}
+
+function didRejectPromise(e)
+{
+ error = e;
+ logResult(true, "Promise rejected.");
+ testExpected("error.name", "NotSupportedError");
+
+ // Wait some time before ending the test towards ensuring that we ended the session interruption.
+ endTestLater();
+}
+
+function runTest()
+{
+ if (!window.internals) {
+ failTest("This test must be run in DumpRenderTree or WebKitTestRunner.");
+ return;
+ }
+ findMediaElement();
+ run('internals.setMediaSessionRestrictions("video", "InterruptedPlaybackNotPermitted")');
+ run('video.src = ""
+ testExpected("video.paused", true);
+ run('internals.beginMediaSessionInterruption("System")');
+ run("video.play().then(didResolvePromise).catch(didRejectPromise)");
+ run('internals.endMediaSessionInterruption("MayResumePlaying")');
+}
+
+window._onload_ = runTest;
+</script>
+</head>
+<body>
+<video></video>
+</body>
+</html>
Added: trunk/LayoutTests/media/video-playback-interrupted-expected.txt (0 => 203931)
--- trunk/LayoutTests/media/video-playback-interrupted-expected.txt (rev 0)
+++ trunk/LayoutTests/media/video-playback-interrupted-expected.txt 2016-07-30 00:30:13 UTC (rev 203931)
@@ -0,0 +1,11 @@
+
+RUN(internals.setMediaSessionRestrictions("video", "InterruptedPlaybackNotPermitted"))
+RUN(video.src = "" "content/test"))
+EXPECTED (video.paused == 'true') OK
+RUN(internals.beginMediaSessionInterruption("System"))
+RUN(video.play().then(didResolvePromise).catch(didRejectPromise))
+RUN(internals.endMediaSessionInterruption("MayResumePlaying"))
+Promise resolved. OK
+EXPECTED (video.paused == 'false') OK
+END OF TEST
+
Added: trunk/LayoutTests/media/video-playback-interrupted.html (0 => 203931)
--- trunk/LayoutTests/media/video-playback-interrupted.html (rev 0)
+++ trunk/LayoutTests/media/video-playback-interrupted.html 2016-07-30 00:30:13 UTC (rev 203931)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<script>
+function didResolvePromise()
+{
+ logResult(true, "Promise resolved.");
+ testExpected("video.paused", false);
+
+ // Wait some time before ending the test towards ensuring that we ended the session interruption.
+ endTestLater();
+}
+
+function didRejectPromise(error)
+{
+ logResult(Failed, "Expected promise to be resolved. Was rejected with error " + error);
+
+ // Wait some time before ending the test towards ensuring that we ended the session interruption.
+ endTestLater();
+}
+
+function runTest()
+{
+ if (!window.internals) {
+ failTest("This test must be run in DumpRenderTree or WebKitTestRunner.");
+ return;
+ }
+ findMediaElement();
+ run('internals.setMediaSessionRestrictions("video", "InterruptedPlaybackNotPermitted")');
+ run('video.src = "" "content/test")');
+ testExpected("video.paused", true);
+ run('internals.beginMediaSessionInterruption("System")');
+ run("video.play().then(didResolvePromise).catch(didRejectPromise)");
+ run('internals.endMediaSessionInterruption("MayResumePlaying")');
+}
+
+window._onload_ = runTest;
+</script>
+</head>
+<body>
+<video></video>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (203930 => 203931)
--- trunk/Source/WebCore/ChangeLog 2016-07-30 00:13:49 UTC (rev 203930)
+++ trunk/Source/WebCore/ChangeLog 2016-07-30 00:30:13 UTC (rev 203931)
@@ -1,5 +1,36 @@
2016-07-29 Daniel Bates <daba...@apple.com>
+ Crash under HTMLMediaElement::{resolve, reject}PendingPlayPromises() when playback is interrupted
+ https://bugs.webkit.org/show_bug.cgi?id=160366
+ <rdar://problem/27317407>
+
+ Reviewed by Eric Carlson.
+
+ Fixes a crash/assertion failure in DeferredWrapper::{resolve, rejectWithValue}() caused by a Promise
+ being settled twice. In particular, if a system interruption occurs when media.play() is invoked
+ the returned Promise may ultimately be settled twice upon cessation of the interruption.
+
+ A Promise can be settled (resolved) exactly once. When a system interruption occurs media
+ playback is paused and resumes on cessation of the interruption. Currently we also immediately
+ reject the Promise p retuned by media.play() if the interruption occurs during its invocation.
+ So, when we resume playback on cessation of an interruption we try to resolve p again. But a
+ Promise can only be resolved once and hence we violate the assertions that p has both a valid
+ reference to a JSPromiseDeferred object and a reference to the global object of the page.
+
+ Tests: media/non-existent-video-playback-interrupted.html
+ media/video-playback-interrupted.html
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::play): Modified to reject the Promise and return immediately if
+ playInternal() returns false.
+ (WebCore::HTMLMediaElement::playInternal): Treat an interruption as success and return true
+ so that HTMLMediaElement::play() adds the Promise to the end of the list of pending promises.
+ We treat an interruption as a success because we will resume playback (assuming the media
+ can be loaded and is well-formed) upon cessation of the interruption and therefore can either
+ fulfill or reject the Promise object returned by media.play().
+
+2016-07-29 Daniel Bates <daba...@apple.com>
+
[iOS] HTMLMediaElement::updateVolume() calls MediaPlayer::volume() on null media player
https://bugs.webkit.org/show_bug.cgi?id=160353
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (203930 => 203931)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-07-30 00:13:49 UTC (rev 203930)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-07-30 00:30:13 UTC (rev 203931)
@@ -3068,8 +3068,10 @@
if (ScriptController::processingUserGestureForMedia())
removeBehaviorsRestrictionsAfterFirstUserGesture();
- if (!playInternal())
+ if (!playInternal()) {
promise.reject(NotAllowedError);
+ return;
+ }
m_pendingPlayPromises.append(WTFMove(promise));
}
@@ -3092,7 +3094,7 @@
if (!m_mediaSession->clientWillBeginPlayback()) {
LOG(Media, " returning because of interruption");
- return false;
+ return true; // Treat as success because we will begin playback on cessation of the interruption.
}
// 4.8.10.9. Playing the media resource
