Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback().
<rdar://problem/64443996>
Created attachment 402976 [details] Patch
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.
(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.
Created attachment 403008 [details] Patch
Committed r263636: <https://trac.webkit.org/changeset/263636> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403008 [details].