Summary: | Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, ddkilzer, ggaren, pvollan, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2020-06-27 17:47:59 PDT
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]. |