213689 2020-06-27 17:47:59 -0700 Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback() 2020-06-28 14:34:25 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez darin ddkilzer ggaren pvollan webkit-bug-importer oldest_to_newest 1666824 0 cdumez 2020-06-27 17:47:59 -0700 Fix thread-safety issue in webProcessPoolHighDynamicRangeDidChangeCallback(). 1666825 1 cdumez 2020-06-27 17:48:15 -0700 <rdar://problem/64443996> 1666826 2 402976 cdumez 2020-06-27 17:50:30 -0700 Created attachment 402976 Patch 1666907 3 402976 darin 2020-06-28 13:23:51 -0700 Comment on attachment 402976 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. 1666908 4 cdumez 2020-06-28 13:28:22 -0700 (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. 1666910 5 403008 cdumez 2020-06-28 13:47:13 -0700 Created attachment 403008 Patch 1666921 6 ews-feeder 2020-06-28 14:34:25 -0700 Committed r263636: <https://trac.webkit.org/changeset/263636> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403008. 402976 2020-06-27 17:50:30 -0700 2020-06-28 13:47:11 -0700 Patch bug-213689-20200627175030.patch text/plain 2225 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjYzNjI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDlhMjJjMGI1YTU3NDRlMmQ3 NDAxYzI0YjljMjdiMTE1MzQwZTI4Y2QuLmQxY2FhMTU2OTI4NmM1MDY5NTUzYTNhZmFhODE4YTBj NzIzMjI1MjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMjAtMDYtMjcgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBGaXggdGhyZWFkLXNhZmV0eSBpc3N1 ZSBpbiB3ZWJQcm9jZXNzUG9vbEhpZ2hEeW5hbWljUmFuZ2VEaWRDaGFuZ2VDYWxsYmFjaygpCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTM2ODkKKyAg ICAgICAgPHJkYXI6Ly9wcm9ibGVtLzY0NDQzOTk2PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIHdlYlByb2Nlc3NQb29sSGlnaER5bmFtaWNSYW5nZURp ZENoYW5nZUNhbGxiYWNrKCkgZ2V0cyBjYWxsZWQgb24gYSBiYWNrZ3JvdW5kIHRocmVhZCBzbyB3 ZQorICAgICAgICBuZWVkIHRvIG1ha2Ugc3VyZSB3ZSBkaXNwYXRjaCB0byB0aGUgbWFpbiB0aHJl YWQgYmVmb3JlIHdlIGl0ZXJhdGUgb3ZlciB0aGUgcHJvY2VzcyBwb29scy4KKworICAgICAgICAq IFVJUHJvY2Vzcy9Db2NvYS9XZWJQcm9jZXNzUG9vbENvY29hLm1tOgorICAgICAgICAoV2ViS2l0 Ojp3ZWJQcm9jZXNzUG9vbEhpZ2hEeW5hbWljUmFuZ2VEaWRDaGFuZ2VDYWxsYmFjayk6CisKIDIw MjAtMDYtMjYgIERhdmlkIEtpbHplciAgPGRka2lsemVyQGFwcGxlLmNvbT4KIAogICAgICAgICBD cmFzaCBhdCBXZWJLaXQ6IF9fNjItW1dLQ29udGVudFZpZXcoV0tJbnRlcmFjdGlvbikgcmVzaWdu Rmlyc3RSZXNwb25kZXJGb3JXZWJWaWV3XV9ibG9ja19pbnZva2UKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJLaXQvVUlQcm9jZXNzL0NvY29hL1dlYlByb2Nlc3NQb29sQ29jb2EubW0gYi9Tb3VyY2Uv V2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9XZWJQcm9jZXNzUG9vbENvY29hLm1tCmluZGV4IDBhNWJk ODhiZDQ2NWFlODhjY2IxOTczMDg2MTA2YmNiYTFkNjA5ODguLmQ5YzgxNjczOTE4ZGJmMTRkOTYx ZThiNmFkN2UxNGExYTc5MmIzN2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNz L0NvY29hL1dlYlByb2Nlc3NQb29sQ29jb2EubW0KKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nl c3MvQ29jb2EvV2ViUHJvY2Vzc1Bvb2xDb2NvYS5tbQpAQCAtOTAzLDkgKzkwMywxMSBAQCB2b2lk IFdlYlByb2Nlc3NQb29sOjpub3RpZnlQcmVmZXJlbmNlc0NoYW5nZWQoY29uc3QgU3RyaW5nJiBk b21haW4sIGNvbnN0IFN0cmluZwogI2lmIFBMQVRGT1JNKE1BQykKIHN0YXRpYyB2b2lkIHdlYlBy b2Nlc3NQb29sSGlnaER5bmFtaWNSYW5nZURpZENoYW5nZUNhbGxiYWNrKENNTm90aWZpY2F0aW9u Q2VudGVyUmVmLCBjb25zdCB2b2lkKiwgQ0ZTdHJpbmdSZWYgbm90aWZpY2F0aW9uTmFtZSwgY29u c3Qgdm9pZCosIENGVHlwZVJlZikKIHsKLSAgICBhdXRvIHNjcmVlblByb3BlcnRpZXMgPSBXZWJD b3JlOjpjb2xsZWN0U2NyZWVuUHJvcGVydGllcygpOwotICAgIGZvciAoYXV0byYgcHJvY2Vzc1Bv b2wgOiBXZWJQcm9jZXNzUG9vbDo6YWxsUHJvY2Vzc1Bvb2xzKCkpCi0gICAgICAgIHByb2Nlc3NQ b29sLT5zZW5kVG9BbGxQcm9jZXNzZXMoTWVzc2FnZXM6OldlYlByb2Nlc3M6OlNldFNjcmVlblBy b3BlcnRpZXMoc2NyZWVuUHJvcGVydGllcykpOworICAgIGRpc3BhdGNoX2FzeW5jKGRpc3BhdGNo X2dldF9tYWluX3F1ZXVlKCksIF57CisgICAgICAgIGF1dG8gc2NyZWVuUHJvcGVydGllcyA9IFdl YkNvcmU6OmNvbGxlY3RTY3JlZW5Qcm9wZXJ0aWVzKCk7CisgICAgICAgIGZvciAoYXV0byYgcHJv Y2Vzc1Bvb2wgOiBXZWJQcm9jZXNzUG9vbDo6YWxsUHJvY2Vzc1Bvb2xzKCkpCisgICAgICAgICAg ICBwcm9jZXNzUG9vbC0+c2VuZFRvQWxsUHJvY2Vzc2VzKE1lc3NhZ2VzOjpXZWJQcm9jZXNzOjpT ZXRTY3JlZW5Qcm9wZXJ0aWVzKHNjcmVlblByb3BlcnRpZXMpKTsKKyAgICB9KTsKIH0KIAogdm9p ZCBXZWJQcm9jZXNzUG9vbDo6cmVnaXN0ZXJIaWdoRHluYW1pY1JhbmdlQ2hhbmdlQ2FsbGJhY2so KQo= 403008 2020-06-28 13:47:13 -0700 2020-06-28 14:34:25 -0700 Patch bug-213689-20200628134712.patch text/plain 2186 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjYzNjM0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDAxNTY4MWJmMGE2YTM4YWJm YTUxMDNlNmUzYTVjZTMwZjgzY2UyYWMuLjI3MWMyNzM4MzFiNDE1NGMwOGUzMjcwOGY5ZTAxYjNj MTY0YTRmNWMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMjAtMDYtMjggIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBGaXggdGhyZWFkLXNhZmV0eSBpc3N1 ZSBpbiB3ZWJQcm9jZXNzUG9vbEhpZ2hEeW5hbWljUmFuZ2VEaWRDaGFuZ2VDYWxsYmFjaygpCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTM2ODkKKyAg ICAgICAgPHJkYXI6Ly9wcm9ibGVtLzY0NDQzOTk2PgorCisgICAgICAgIFJldmlld2VkIGJ5IERh cmluIEFkbGVyLgorCisgICAgICAgIHdlYlByb2Nlc3NQb29sSGlnaER5bmFtaWNSYW5nZURpZENo YW5nZUNhbGxiYWNrKCkgZ2V0cyBjYWxsZWQgb24gYSBiYWNrZ3JvdW5kIHRocmVhZCBzbyB3ZQor ICAgICAgICBuZWVkIHRvIG1ha2Ugc3VyZSB3ZSBkaXNwYXRjaCB0byB0aGUgbWFpbiB0aHJlYWQg YmVmb3JlIHdlIGl0ZXJhdGUgb3ZlciB0aGUgcHJvY2VzcyBwb29scy4KKworICAgICAgICAqIFVJ UHJvY2Vzcy9Db2NvYS9XZWJQcm9jZXNzUG9vbENvY29hLm1tOgorICAgICAgICAoV2ViS2l0Ojp3 ZWJQcm9jZXNzUG9vbEhpZ2hEeW5hbWljUmFuZ2VEaWRDaGFuZ2VDYWxsYmFjayk6CisKIDIwMjAt MDYtMjggIERhbmllbCBCYXRlcyAgPGRhYmF0ZXNAYXBwbGUuY29tPgogCiAgICAgICAgIFtpT1Nd IENsZWFuIHVwIHZpc2libGUgcG9zaXRpb24gY29tcGFyaXNvbnMgaW4gV2ViUGFnZTo6cmVxdWVz dERvY3VtZW50RWRpdGluZ0NvbnRleHQoKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvQ29jb2EvV2ViUHJvY2Vzc1Bvb2xDb2NvYS5tbSBiL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL0NvY29hL1dlYlByb2Nlc3NQb29sQ29jb2EubW0KaW5kZXggMGE1YmQ4OGJkNDY1YWU4OGNj YjE5NzMwODYxMDZiY2JhMWQ2MDk4OC4uMWZhNjAzMWQ5YWQ4NjcyNDg3OTgyNmFmNjAzNmVmZDJj YmQ1YzIxYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQ29jb2EvV2ViUHJv Y2Vzc1Bvb2xDb2NvYS5tbQorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9XZWJQ cm9jZXNzUG9vbENvY29hLm1tCkBAIC05MDMsOSArOTAzLDExIEBAIHZvaWQgV2ViUHJvY2Vzc1Bv b2w6Om5vdGlmeVByZWZlcmVuY2VzQ2hhbmdlZChjb25zdCBTdHJpbmcmIGRvbWFpbiwgY29uc3Qg U3RyaW5nCiAjaWYgUExBVEZPUk0oTUFDKQogc3RhdGljIHZvaWQgd2ViUHJvY2Vzc1Bvb2xIaWdo RHluYW1pY1JhbmdlRGlkQ2hhbmdlQ2FsbGJhY2soQ01Ob3RpZmljYXRpb25DZW50ZXJSZWYsIGNv bnN0IHZvaWQqLCBDRlN0cmluZ1JlZiBub3RpZmljYXRpb25OYW1lLCBjb25zdCB2b2lkKiwgQ0ZU eXBlUmVmKQogewotICAgIGF1dG8gc2NyZWVuUHJvcGVydGllcyA9IFdlYkNvcmU6OmNvbGxlY3RT Y3JlZW5Qcm9wZXJ0aWVzKCk7Ci0gICAgZm9yIChhdXRvJiBwcm9jZXNzUG9vbCA6IFdlYlByb2Nl c3NQb29sOjphbGxQcm9jZXNzUG9vbHMoKSkKLSAgICAgICAgcHJvY2Vzc1Bvb2wtPnNlbmRUb0Fs bFByb2Nlc3NlcyhNZXNzYWdlczo6V2ViUHJvY2Vzczo6U2V0U2NyZWVuUHJvcGVydGllcyhzY3Jl ZW5Qcm9wZXJ0aWVzKSk7CisgICAgZGlzcGF0Y2hfYXN5bmMoZGlzcGF0Y2hfZ2V0X21haW5fcXVl dWUoKSwgXnsKKyAgICAgICAgYXV0byBwcm9wZXJ0aWVzID0gV2ViQ29yZTo6Y29sbGVjdFNjcmVl blByb3BlcnRpZXMoKTsKKyAgICAgICAgZm9yIChhdXRvJiBwb29sIDogV2ViUHJvY2Vzc1Bvb2w6 OmFsbFByb2Nlc3NQb29scygpKQorICAgICAgICAgICAgcG9vbC0+c2VuZFRvQWxsUHJvY2Vzc2Vz KE1lc3NhZ2VzOjpXZWJQcm9jZXNzOjpTZXRTY3JlZW5Qcm9wZXJ0aWVzKHByb3BlcnRpZXMpKTsK KyAgICB9KTsKIH0KIAogdm9pZCBXZWJQcm9jZXNzUG9vbDo6cmVnaXN0ZXJIaWdoRHluYW1pY1Jh bmdlQ2hhbmdlQ2FsbGJhY2soKQo=