Bug 180781 - Playing media elements which call "pause(); play()" will have the play promise rejected.
Summary: Playing media elements which call "pause(); play()" will have the play promis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-13 16:24 PST by Jer Noble
Modified: 2017-12-19 15:16 PST (History)
7 users (show)

See Also:


Attachments
Patch (7.98 KB, patch)
2017-12-13 16:28 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (2.30 MB, application/zip)
2017-12-13 17:13 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.97 MB, application/zip)
2017-12-13 17:37 PST, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.51 MB, application/zip)
2017-12-13 17:42 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (3.23 MB, application/zip)
2017-12-13 18:55 PST, Build Bot
no flags Details
Patch (12.53 KB, patch)
2017-12-15 11:18 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (2.19 MB, application/zip)
2017-12-15 12:18 PST, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (2.60 MB, application/zip)
2017-12-15 12:28 PST, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.15 MB, application/zip)
2017-12-15 12:43 PST, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-elcapitan (3.43 MB, application/zip)
2017-12-15 13:06 PST, Build Bot
no flags Details
Patch for landing (14.54 KB, patch)
2017-12-15 17:58 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Follow-up patch (4.05 KB, patch)
2017-12-19 08:51 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (2.17 MB, application/zip)
2017-12-19 09:53 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (2.76 MB, application/zip)
2017-12-19 10:13 PST, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (2.96 MB, application/zip)
2017-12-19 10:26 PST, Build Bot
no flags Details
Follow-up patch (8.19 KB, patch)
2017-12-19 12:42 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (2.50 MB, application/zip)
2017-12-19 13:31 PST, Build Bot
no flags Details
Follow-up patch (9.62 KB, patch)
2017-12-19 13:55 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Follow-up patch (9.62 KB, patch)
2017-12-19 13:58 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2017-12-13 16:24:53 PST
Playing media elements which call "pause(); play()" will have the play promise rejected.
Comment 1 Jer Noble 2017-12-13 16:25:52 PST
<rdar://problem/33191377>
Comment 2 Jer Noble 2017-12-13 16:28:34 PST
Created attachment 329280 [details]
Patch
Comment 3 Build Bot 2017-12-13 17:13:42 PST Comment hidden (obsolete)
Comment 4 Build Bot 2017-12-13 17:13:43 PST Comment hidden (obsolete)
Comment 5 Build Bot 2017-12-13 17:37:13 PST Comment hidden (obsolete)
Comment 6 Build Bot 2017-12-13 17:37:14 PST Comment hidden (obsolete)
Comment 7 Build Bot 2017-12-13 17:42:33 PST Comment hidden (obsolete)
Comment 8 Build Bot 2017-12-13 17:42:35 PST Comment hidden (obsolete)
Comment 9 Build Bot 2017-12-13 18:54:59 PST Comment hidden (obsolete)
Comment 10 Build Bot 2017-12-13 18:55:00 PST Comment hidden (obsolete)
Comment 11 Jer Noble 2017-12-15 11:18:20 PST
Created attachment 329498 [details]
Patch
Comment 12 Eric Carlson 2017-12-15 11:25:12 PST
Comment on attachment 329498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329498&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:3501
> +        if (m_readyState <= HAVE_CURRENT_DATA)
> +            scheduleEvent(eventNames().waitingEvent);
> +        else if (m_readyState >= HAVE_FUTURE_DATA)
> +            scheduleNotifyAboutPlaying();

Nit: you should mention this fix in the ChangeLog.
Comment 13 Build Bot 2017-12-15 12:18:50 PST Comment hidden (obsolete)
Comment 14 Build Bot 2017-12-15 12:18:52 PST Comment hidden (obsolete)
Comment 15 Build Bot 2017-12-15 12:28:50 PST Comment hidden (obsolete)
Comment 16 Build Bot 2017-12-15 12:28:51 PST Comment hidden (obsolete)
Comment 17 Build Bot 2017-12-15 12:43:36 PST Comment hidden (obsolete)
Comment 18 Build Bot 2017-12-15 12:43:37 PST Comment hidden (obsolete)
Comment 19 Build Bot 2017-12-15 13:06:39 PST Comment hidden (obsolete)
Comment 20 Build Bot 2017-12-15 13:06:40 PST Comment hidden (obsolete)
Comment 21 Jer Noble 2017-12-15 17:58:06 PST
Created attachment 329551 [details]
Patch for landing
Comment 22 WebKit Commit Bot 2017-12-18 09:43:55 PST
Comment on attachment 329551 [details]
Patch for landing

Clearing flags on attachment: 329551

Committed r226059: <https://trac.webkit.org/changeset/226059>
Comment 23 WebKit Commit Bot 2017-12-18 09:43:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 24 Darin Adler 2017-12-18 10:26:49 PST
Comment on attachment 329551 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=329551&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:3454
> +        return; // Treat as success because we will begin playback on cessation of the interruption.

This comment no longer makes much sense. Before it was explaining the reason this returned true instead of false. But what does "treat as success" mean now? I think we should just remove the comment unless we have something interesting to say here.
Comment 25 Matt Lewis 2017-12-18 16:12:29 PST
This patch caused 3 API test to start Timing out:
 RequiresUserActionForPlaybackTest.DeprecatedRequiresUserActionForVideoButNotAudioPlayback
 RequiresUserActionForPlaybackTest.RequiresUserActionForVideoButNotAudioPlayback
 WebKit.WebsitePoliciesAutoplayEnabled

https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/2026

https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20%28Tests%29/builds/2026/steps/run-api-tests/logs/stdio
Comment 26 Jer Noble 2017-12-19 08:51:53 PST
Reopening to attach new patch.
Comment 27 Jer Noble 2017-12-19 08:51:56 PST
Created attachment 329764 [details]
Follow-up patch
Comment 28 Build Bot 2017-12-19 09:53:39 PST Comment hidden (obsolete)
Comment 29 Build Bot 2017-12-19 09:53:40 PST Comment hidden (obsolete)
Comment 30 Build Bot 2017-12-19 10:13:07 PST Comment hidden (obsolete)
Comment 31 Build Bot 2017-12-19 10:13:09 PST Comment hidden (obsolete)
Comment 32 Build Bot 2017-12-19 10:26:45 PST Comment hidden (obsolete)
Comment 33 Build Bot 2017-12-19 10:26:46 PST Comment hidden (obsolete)
Comment 34 Jer Noble 2017-12-19 12:42:53 PST
Created attachment 329796 [details]
Follow-up patch
Comment 35 Build Bot 2017-12-19 13:31:21 PST
Comment on attachment 329796 [details]
Follow-up patch

Attachment 329796 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5757553

New failing tests:
media/video-load-require-user-gesture.html
Comment 36 Build Bot 2017-12-19 13:31:23 PST
Created attachment 329805 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 37 Jer Noble 2017-12-19 13:55:40 PST
Created attachment 329814 [details]
Follow-up patch
Comment 38 Jer Noble 2017-12-19 13:58:23 PST
Created attachment 329816 [details]
Follow-up patch
Comment 39 WebKit Commit Bot 2017-12-19 15:16:13 PST
Comment on attachment 329816 [details]
Follow-up patch

Clearing flags on attachment: 329816

Committed r226150: <https://trac.webkit.org/changeset/226150>
Comment 40 WebKit Commit Bot 2017-12-19 15:16:15 PST
All reviewed patches have been landed.  Closing bug.