Bug 139437 - Enable automatic optimized fullscreen mode.
Summary: Enable automatic optimized fullscreen mode.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Jeremy Jones
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-09 02:34 PST by Jeremy Jones
Modified: 2014-12-09 14:39 PST (History)
10 users (show)

See Also:

Attachments
Patch (8.06 KB, patch)
2014-12-09 02:50 PST, Jeremy Jones 		eric.carlson: review+
Details | Formatted Diff | Diff
Patch for landing. (8.26 KB, patch)
2014-12-09 11:43 PST, Jeremy Jones 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Jones 2014-12-09 02:34:02 PST 
Enable automatic optimized fullscreen mode.
Comment 1 Jeremy Jones 2014-12-09 02:35:11 PST 
rdar://problem/18518346
Comment 2 Jeremy Jones 2014-12-09 02:50:35 PST 
Created attachment 242900 [details]
Patch
Comment 3 Eric Carlson 2014-12-09 07:10:36 PST 
Comment on attachment 242900 [details]
Patch

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

r=me but this needs a thumbs-up from a WK2 reviewer as well.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:244
> +    if (_page->videoFullscreenManager()->mode() & WebCore::HTMLMediaElement::VideoFullscreenModeOptimized)
>          return true;
>      
>      return false;

Nit: this could be simplified to "return _page->videoFullscreenManager()->mode() & WebCore::HTMLMediaElement::VideoFullscreenModeOptimized"

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:255
> +    if (_page->videoFullscreenManager()->willAutomaticallyEnterOptimizedMode())
> +        return true;
> +    
> +    return false;

Nit: this could be simplified to "return _page->videoFullscreenManager()->willAutomaticallyEnterOptimizedMode()"

> Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm:191
> +    if ([m_webView _willAutomaticallyEnterOptimizedVideoMode])
> +        return true;
> +    
> +    return false;

This could be simplified as well, but I think it is clearer as-is.
Comment 4 Jeremy Jones 2014-12-09 10:58:24 PST 
(In reply to comment #3)
> Comment on attachment 242900 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242900&action=review
> 
> r=me but this needs a thumbs-up from a WK2 reviewer as well.
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:244
> > +    if (_page->videoFullscreenManager()->mode() & WebCore::HTMLMediaElement::VideoFullscreenModeOptimized)
> >          return true;
> >      
> >      return false;
> 
> Nit: this could be simplified to "return
> _page->videoFullscreenManager()->mode() &
> WebCore::HTMLMediaElement::VideoFullscreenModeOptimized"

Done.

> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:255
> > +    if (_page->videoFullscreenManager()->willAutomaticallyEnterOptimizedMode())
> > +        return true;
> > +    
> > +    return false;
> 
> Nit: this could be simplified to "return
> _page->videoFullscreenManager()->willAutomaticallyEnterOptimizedMode()"

Done.

> 
> > Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm:191
> > +    if ([m_webView _willAutomaticallyEnterOptimizedVideoMode])
> > +        return true;
> > +    
> > +    return false;
> 
> This could be simplified as well, but I think it is clearer as-is.

Left as-is.
Comment 5 Simon Fraser (smfr) 2014-12-09 11:05:26 PST 
Comment on attachment 242900 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:238
> +    if (!_page || !_page->videoFullscreenManager())

Can _page actually be null? When does this happen?

> Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm:189
> +    if ([m_webView _isPlayingFullscreenOptimizedVideo])
> +        return true;
> +    
> +    if ([m_webView _willAutomaticallyEnterOptimizedVideoMode])
> +        return true;

_willAutomaticallyEnterOptimizedVideoMode is confusing.

Isn't this really "playing fullscreen video" && "supports optimized fullscreen"?
Comment 6 Jeremy Jones 2014-12-09 11:43:11 PST 
Created attachment 242951 [details]
Patch for landing.
Comment 7 Jeremy Jones 2014-12-09 11:45:29 PST 
(In reply to comment #5)
> Comment on attachment 242900 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242900&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:238
> > +    if (!_page || !_page->videoFullscreenManager())
> 
> Can _page actually be null? When does this happen?
> 
> > Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm:189
> > +    if ([m_webView _isPlayingFullscreenOptimizedVideo])
> > +        return true;
> > +    
> > +    if ([m_webView _willAutomaticallyEnterOptimizedVideoMode])
> > +        return true;
> 
> _willAutomaticallyEnterOptimizedVideoMode is confusing.
> 
> Isn't this really "playing fullscreen video" && "supports optimized
> fullscreen"?

Changed these to _isShowingVideoOptimized and _mayAutomaticallyShowVideoOptimized
Comment 8 WebKit Commit Bot 2014-12-09 13:54:11 PST 
Comment on attachment 242951 [details]
Patch for landing.

Clearing flags on attachment: 242951

Committed r177039: <http://trac.webkit.org/changeset/177039>