Summary: | Enable automatic optimized fullscreen mode. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeremy Jones <jeremyj-wk> | ||||||
Component: | Media | Assignee: | Jeremy Jones <jeremyj-wk> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | calvaris, commit-queue, eric.carlson, esprehn+autocc, glenn, gyuyoung.kim, jer.noble, philipj, sergio, simon.fraser | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | iPhone / iPad | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jeremy Jones
2014-12-09 02:34:02 PST
Created attachment 242900 [details]
Patch
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. (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 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"? Created attachment 242951 [details]
Patch for landing.
(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 on attachment 242951 [details] Patch for landing. Clearing flags on attachment: 242951 Committed r177039: <http://trac.webkit.org/changeset/177039> |