Bug 213689 – Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback().

[Chris Dumez]

<rdar://problem/64443996>

[Chris Dumez]

Created attachment 402976 [details]
Patch

[Darin Adler]

Comment on attachment 402976 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:906
> +    dispatch_async(dispatch_get_main_queue(), ^{

Just curious: why did you select dispatch to the main queue over other techniques for running on the main thread?

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:907
> +        auto screenProperties = WebCore::collectScreenProperties();

In a small function/tight context like this, I like just using one word names, like "properties" here.

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:908
> +        for (auto& processPool : WebProcessPool::allProcessPools())

And "pool" here.

[Chris Dumez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 402976 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402976&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:906
> > +    dispatch_async(dispatch_get_main_queue(), ^{
> 
> Just curious: why did you select dispatch to the main queue over other
> techniques for running on the main thread?

I am in a .mm file so I can use dispatch_async(). Dispatch_async() has the benefit of not requiring any “main thread / run loop initialization” and work always. I recently tried to fix a similar bug with RunLoop dispatch and it caused crashes in the api tests because the main runloop was not initialized yet.

> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:907
> > +        auto screenProperties = WebCore::collectScreenProperties();
> 
> In a small function/tight context like this, I like just using one word
> names, like "properties" here.
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:908
> > +        for (auto& processPool : WebProcessPool::allProcessPools())
> 
> And "pool" here.

[Chris Dumez]

Created attachment 403008 [details]
Patch

[EWS]

Committed r263636: <https://trac.webkit.org/changeset/263636>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403008 [details].