Bug 183959 – Adopt WEBPROCESS_WINDOWSERVER_BLOCKING compiler guard in WebProcess

• Status: REOPENED
• Component: WebKit2

[Per Arne Vollan]

I suggest we add the compile guard ENABLE_WEBPROCESS_WINDOWSERVER_BLOCKING.

[Per Arne Vollan]

I would like to add a compile guard, guarding the call to CGSSetDenyWindowServerConnections.

[Radar WebKit Bug Importer]

<rdar://problem/38965719>

[Per Arne Vollan]

Created attachment 336689 [details]
Patch

[Brent Fulgham]

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.

[Per Arne Vollan]

(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.

[Per Arne Vollan]

Created attachment 336706 [details]
Patch

[Brent Fulgham]

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?

[Per Arne Vollan]

Created attachment 336708 [details]
Patch

[Per Arne Vollan]

(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!

[WebKit Commit Bot]

Comment on attachment 336708 [details]
Patch

Clearing flags on attachment: 336708

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