Bug 180649 – The WebContent process should not establish a window server connection.

• Component: WebKit Misc.
• Status: RESOLVED WONTFIX

[Per Arne Vollan]

Since the WebContent process does not have windows, communication with the window server should not be needed.

[Per Arne Vollan]

Created attachment 328975 [details]
Patch

[EWS Watchlist]

Comment on attachment 328975 [details]
Patch

Attachment 328975 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5615506

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 328980 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Per Arne Vollan]

Created attachment 329004 [details]
Patch

[Per Arne Vollan]

Created attachment 329012 [details]
Patch

[Per Arne Vollan]

Created attachment 329026 [details]
Patch

[Per Arne Vollan]

Created attachment 329117 [details]
Patch

[Per Arne Vollan]

(In reply to Per Arne Vollan from comment #7)
> Created attachment 329117 [details]
> Patch

With this patch, there are no references to window server access in the system log (searching for 'CGSLookupServerRootPort').

[Per Arne Vollan]

Created attachment 329122 [details]
Patch

[Sam Weinig]

Comment on attachment 329122 [details]
Patch

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

> Source/WebCore/platform/mac/ThemeMac.mm:72
> +        return nullptr;

This should return nil.

> Source/WebKit/Shared/mac/ChildProcessMac.mm:217
>  void ChildProcess::stopNSAppRunLoop()
>  {
> -    ASSERT([NSApp isRunning]);
> +    if (![NSApp isRunning]) {
> +        CFRunLoopStop([[NSRunLoop currentRunLoop] getCFRunLoop]);
> +        return;
> +    }

It feels like this function is not named correctly anymore if it sometimes doesn't stop the NSApp runloop.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:116
> +    return 0;

This should return nil.

[Per Arne Vollan]

(In reply to Sam Weinig from comment #10)
> Comment on attachment 329122 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329122&action=review
> 
> > Source/WebCore/platform/mac/ThemeMac.mm:72
> > +        return nullptr;
> 
> This should return nil.
> 
> > Source/WebKit/Shared/mac/ChildProcessMac.mm:217
> >  void ChildProcess::stopNSAppRunLoop()
> >  {
> > -    ASSERT([NSApp isRunning]);
> > +    if (![NSApp isRunning]) {
> > +        CFRunLoopStop([[NSRunLoop currentRunLoop] getCFRunLoop]);
> > +        return;
> > +    }
> 
> It feels like this function is not named correctly anymore if it sometimes
> doesn't stop the NSApp runloop.
> 
> > Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:116
> > +    return 0;
> 
> This should return nil.

Thanks for reviewing! I will update the patch.

[Brent Fulgham]

Equivalent work was done under other Bugzilla entries. This bug is no longer needed.

[Maciej Stachowiak]

Comment on attachment 329122 [details]
Patch

Unflagging and obsoleting, since this was resolved as WONTFIX (though actually, I think this is a dupe of something we ended up doing later.