Changeset 203931 in webkit

Timestamp:

Jul 29, 2016 5:30:13 PM (8 years ago)

Author:

dbates@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/non-existent-video-playback-interrupted-expected.txt (added)
• LayoutTests/media/non-existent-video-playback-interrupted.html (added)
• LayoutTests/media/video-playback-interrupted-expected.txt (added)
• LayoutTests/media/video-playback-interrupted.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-07-29  Daniel Bates  <dabates@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  <ryanhaddad@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-07-29  Daniel Bates  <dabates@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  <dabates@apple.com>
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -3069 +3069 @@
         removeBehaviorsRestrictionsAfterFirstUserGesture();
 
-    if (!playInternal())
+    if (!playInternal()) {
         promise.reject(NotAllowedError);
+        return;
+    }
 
     m_pendingPlayPromises.append(WTFMove(promise));
@@ -3093 +3095 @@
     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.
     }