Bug 125427 - [Cocoa] Remove knowledge of the WKView from the WebFullScreenManagerProxy by adding a proper client
Summary: [Cocoa] Remove knowledge of the WKView from the WebFullScreenManagerProxy by ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
Depends on: 125438 125440
  Show dependency treegraph
Reported: 2013-12-08 13:47 PST by Sam Weinig
Modified: 2013-12-09 01:42 PST (History)
7 users (show)

See Also:

Patch (37.59 KB, patch)
2013-12-08 13:50 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (26.73 KB, patch)
2013-12-08 15:11 PST, Sam Weinig
mitz: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
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]

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]
Comment 5 EFL EWS Bot 2013-12-08 15:16:31 PST
Comment on attachment 218716 [details]

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]

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

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

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]

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>