Bug 125427

Summary: [Cocoa] Remove knowledge of the WKView from the WebFullScreenManagerProxy by adding a proper client
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eflews.bot, gyuyoung.kim, jer.noble, mitz, philn, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 125438, 125440    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch mitz: review+, eflews.bot: commit-queue-

Description Sam Weinig 2013-12-08 13:47:07 PST
[Cocoa] Remove knowledge of the WKView from the WebFullScreenManagerProxy by adding a proper client
Comment 1 Sam Weinig 2013-12-08 13:50:46 PST
Created attachment 218714 [details]
Patch
Comment 2 WebKit Commit Bot 2013-12-08 13:53:15 PST
Attachment 218714 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h', u'Source/WebKit2/UIProcess/API/mac/PageClientImpl.h', u'Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm', u'Source/WebKit2/UIProcess/API/mac/WKView+FullScreen.h', u'Source/WebKit2/UIProcess/API/mac/WKView+FullScreen.mm', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewInternal.h', u'Source/WebKit2/UIProcess/PageClient.h', u'Source/WebKit2/UIProcess/WebFullScreenManagerProxy.cpp', u'Source/WebKit2/UIProcess/WebFullScreenManagerProxy.h', u'Source/WebKit2/UIProcess/WebPageProxy.cpp', u'Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm', u'Source/WebKit2/UIProcess/mac/WebFullScreenManagerProxyMac.mm', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1
ERROR: Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:53:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 EFL EWS Bot 2013-12-08 13:56:23 PST
Comment on attachment 218714 [details]
Patch

Attachment 218714 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/47158094
Comment 4 Sam Weinig 2013-12-08 15:11:23 PST
Created attachment 218716 [details]
Patch
Comment 5 EFL EWS Bot 2013-12-08 15:16:31 PST
Comment on attachment 218716 [details]
Patch

Attachment 218716 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/44478116
Comment 6 mitz 2013-12-08 15:22:31 PST
Comment on attachment 218716 [details]
Patch

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

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:567
> +#endif
> +
> +#if ENABLE(FULLSCREEN_API)

This is unnecessary

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:580
> +    if (!m_wkView.hasFullScreenWindowController)
> +        return false;
> +

Is this part necessary?

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:603
> +#endif

This #endif could use a newline before it and a comment after it.

> Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:98
> +@property (readonly) BOOL hasFullScreenWindowController;
> +@property (readonly) WKFullScreenWindowController *fullScreenWindowController;
> +- (void)closeFullScreenWindowController;

As internal methods and properties, these should probably be prefixed with an underscore.
Comment 7 Build Bot 2013-12-08 15:52:26 PST
Comment on attachment 218716 [details]
Patch

Attachment 218716 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/45388138
Comment 8 Sam Weinig 2013-12-08 20:08:06 PST
Committed r160296: <http://trac.webkit.org/changeset/160296>