Bug 183959 - Adopt WEBPROCESS_WINDOWSERVER_BLOCKING compiler guard in WebProcess
Summary: Adopt WEBPROCESS_WINDOWSERVER_BLOCKING compiler guard in WebProcess
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-23 16:00 PDT by Per Arne Vollan
Modified: 2018-03-28 14:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.95 KB, patch)
2018-03-28 11:56 PDT, Per Arne Vollan
bfulgham: review+
bfulgham: commit-queue-
Details | Formatted Diff | Diff
Patch (2.03 KB, patch)
2018-03-28 13:07 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (2.05 KB, patch)
2018-03-28 13:27 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2018-03-23 16:00:25 PDT
I suggest we add the compile guard ENABLE_WEBPROCESS_WINDOWSERVER_BLOCKING.
Comment 1 Per Arne Vollan 2018-03-28 11:52:47 PDT
I would like to add a compile guard, guarding the call to CGSSetDenyWindowServerConnections.
Comment 2 Radar WebKit Bug Importer 2018-03-28 11:53:41 PDT
<rdar://problem/38965719>
Comment 3 Per Arne Vollan 2018-03-28 11:56:10 PDT
Created attachment 336689 [details]
Patch
Comment 4 Brent Fulgham 2018-03-28 12:47:05 PDT
Comment on attachment 336689 [details]
Patch

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

This seems reasonable. Please add a log statement before landing.

> Source/WebKit/ChangeLog:3
> +        Add compile guard for blocking of the WindowServer in the WebProcess.

When I read this, I though you were reading a new compile macro -- but you are just using it. I think I'd title this, "Adopt WEBPROCESS_WINDOWSERVER_BLOCKING compiler guard in WebProcess", or something.

> Source/WebKit/WebProcess/WebProcess.cpp:234
> +    ASSERT_UNUSED(error, error == kCGErrorSuccess);

I think you should log an error here if we are unsuccessful in calling DenyWindowServerConnections.
Comment 5 Per Arne Vollan 2018-03-28 12:54:40 PDT
(In reply to Brent Fulgham from comment #4)
> Comment on attachment 336689 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=336689&action=review
> 
> This seems reasonable. Please add a log statement before landing.
> 
> > Source/WebKit/ChangeLog:3
> > +        Add compile guard for blocking of the WindowServer in the WebProcess.
> 
> When I read this, I though you were reading a new compile macro -- but you
> are just using it. I think I'd title this, "Adopt
> WEBPROCESS_WINDOWSERVER_BLOCKING compiler guard in WebProcess", or something.
> 
> > Source/WebKit/WebProcess/WebProcess.cpp:234
> > +    ASSERT_UNUSED(error, error == kCGErrorSuccess);
> 
> I think you should log an error here if we are unsuccessful in calling
> DenyWindowServerConnections.

Thanks for reviewing! I'll update the patch before landing.
Comment 6 Per Arne Vollan 2018-03-28 13:07:57 PDT
Created attachment 336706 [details]
Patch
Comment 7 Brent Fulgham 2018-03-28 13:17:33 PDT
Comment on attachment 336706 [details]
Patch

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

> Source/WebKit/WebProcess/WebProcess.cpp:236
> +        WTFLogAlways("Failed to deny WindowServer connections.");

Is CGError ever something interesting? Would printing its value in the log message be helpful?
Comment 8 Per Arne Vollan 2018-03-28 13:27:29 PDT
Created attachment 336708 [details]
Patch
Comment 9 Per Arne Vollan 2018-03-28 13:27:53 PDT
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 336706 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=336706&action=review
> 
> > Source/WebKit/WebProcess/WebProcess.cpp:236
> > +        WTFLogAlways("Failed to deny WindowServer connections.");
> 
> Is CGError ever something interesting? Would printing its value in the log
> message be helpful?

Done!
Comment 10 WebKit Commit Bot 2018-03-28 14:17:19 PDT
Comment on attachment 336708 [details]
Patch

Clearing flags on attachment: 336708

Committed r230050: <https://trac.webkit.org/changeset/230050>