Bug 220939 - Rename MediaElementSession::playbackPermitted() to MediaElementSession::playbackStateChangePermitted()
Summary: Rename MediaElementSession::playbackPermitted() to MediaElementSession::playb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peng Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-25 11:49 PST by Peng Liu
Modified: 2021-03-30 12:15 PDT (History)
13 users (show)

See Also:


Attachments
Patch (11.26 KB, patch)
2021-03-29 17:01 PDT, Peng Liu
youennf: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (11.37 KB, patch)
2021-03-30 10:24 PDT, Peng Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Liu 2021-01-25 11:49:14 PST
MediaElementSession::playbackPermitted() is used in many places to check whether a playback state change is allowed or not. We had better add a parameter to indicate the state change request. Otherwise, it may return false when it is expected to return true.
Comment 1 Radar WebKit Bug Importer 2021-02-01 11:51:33 PST
<rdar://problem/73839280>
Comment 2 Peng Liu 2021-03-29 17:01:52 PDT
Created attachment 424602 [details]
Patch
Comment 3 Eric Carlson 2021-03-30 07:04:11 PDT
Comment on attachment 424602 [details]
Patch

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

> Source/WebCore/html/MediaElementSession.cpp:286
>          ALWAYS_LOG(LOGIDENTIFIER, "Returning FALSE because element is suspended");

Now that we always pass the new state, it would be nice to include that in the logging so we know what state change was allowed/denied.
Comment 4 Peng Liu 2021-03-30 09:42:01 PDT
Comment on attachment 424602 [details]
Patch

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

>> Source/WebCore/html/MediaElementSession.cpp:286
>>          ALWAYS_LOG(LOGIDENTIFIER, "Returning FALSE because element is suspended");
> 
> Now that we always pass the new state, it would be nice to include that in the logging so we know what state change was allowed/denied.

Good idea! Will add that.
Comment 5 Peng Liu 2021-03-30 10:24:19 PDT
Created attachment 424656 [details]
Patch for landing
Comment 6 EWS 2021-03-30 12:15:02 PDT
Committed r275225: <https://commits.webkit.org/r275225>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424656 [details].