Bug 167390

Summary: Notify clients when the user plays media otherwise prevented from autoplaying
Product: WebKit Reporter: Matt Rajca <mrajca>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ryanhaddad
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
mrajca: review-, mrajca: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Description Matt Rajca 2017-01-24 15:09:53 PST
Notify clients when the user plays media (with a user gesture) otherwise prevented from autoplaying.
Comment 1 Matt Rajca 2017-01-24 15:14:19 PST
Created attachment 299637 [details]
Patch
Comment 2 Matt Rajca 2017-01-24 15:50:55 PST
Created attachment 299644 [details]
Patch
Comment 3 Alex Christensen 2017-01-24 18:57:15 PST
Comment on attachment 299644 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKPageUIClient.h:729
>      WKRequestPointerLockCallback                                        requestPointerLock;
>      WKDidLosePointerLockCallback                                        didLosePointerLock;
> +    WKDidPlayMediaPreventedFromPlayingWithoutUserGesture                didPlayMediaPreventedFromPlayingWithoutUserGesture;

I'm not sure if we need to make another version of the client to maintain binary compatibility.  It seems like the pointer lock stuff was added recently enough that maybe the answer is no.
Comment 4 Alex Christensen 2017-01-25 15:37:10 PST
Comment on attachment 299644 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.messages.in:441
> +    DidPlayMediaPreventedFromPlayingWithoutUserGesture()

This name is a bit verbose, but I can't think of anything better.

> Tools/TestWebKitAPI/Tests/WebKit2/js-play-with-controls.html:5
> +                setTimeout(function() {

Could we remove this setTimeout?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WebsitePolicies.mm:217
> +TEST(WebKit2, WebsitePoliciesPlayAfterPreventedAutoplay)

It would be good to test that a video that shouldn't autoplay doesn't call the didPlayMediaPreventedFromPlayingWithoutUserGesture when you play it.
Comment 5 Matt Rajca 2017-01-25 16:13:45 PST
(In reply to comment #4)
> Comment on attachment 299644 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299644&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.messages.in:441
> > +    DidPlayMediaPreventedFromPlayingWithoutUserGesture()
> 
> This name is a bit verbose, but I can't think of anything better.
> 
> > Tools/TestWebKitAPI/Tests/WebKit2/js-play-with-controls.html:5
> > +                setTimeout(function() {
> 
> Could we remove this setTimeout?

Done.

> 
> > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WebsitePolicies.mm:217
> > +TEST(WebKit2, WebsitePoliciesPlayAfterPreventedAutoplay)
> 
> It would be good to test that a video that shouldn't autoplay doesn't call
> the didPlayMediaPreventedFromPlayingWithoutUserGesture when you play it.

I'll add one.
Comment 6 Matt Rajca 2017-01-25 16:13:59 PST
Created attachment 299762 [details]
Patch
Comment 7 Matt Rajca 2017-01-25 17:09:09 PST
Created attachment 299773 [details]
Patch
Comment 8 Matt Rajca 2017-01-25 17:42:17 PST
Created attachment 299779 [details]
Patch
Comment 9 WebKit Commit Bot 2017-01-25 18:19:19 PST
Comment on attachment 299779 [details]
Patch

Clearing flags on attachment: 299779

Committed r211193: <http://trac.webkit.org/changeset/211193>
Comment 10 WebKit Commit Bot 2017-01-25 18:19:23 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Ryan Haddad 2017-01-25 20:25:35 PST
Reverted r211193 for reason:

This change broke internal builds.

Committed r211200: <http://trac.webkit.org/changeset/211200>
Comment 12 Matt Rajca 2017-01-26 11:05:56 PST
Created attachment 299820 [details]
Patch
Comment 13 WebKit Commit Bot 2017-01-26 12:42:33 PST
Comment on attachment 299820 [details]
Patch

Clearing flags on attachment: 299820

Committed r211226: <http://trac.webkit.org/changeset/211226>
Comment 14 WebKit Commit Bot 2017-01-26 12:42:37 PST
All reviewed patches have been landed.  Closing bug.