223089 2021-03-11 14:03:20 -0800 Use BinarySemaphore in SerializedScriptValue::writeBlobsToDiskForIndexedDBSynchronously() 2021-03-11 14:41:18 -0800 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez achristensen alecflett beidson darin ews-watchlist ggaren jsbell sam webkit-bug-importer oldest_to_newest 1738609 0 cdumez 2021-03-11 14:03:20 -0800 Use BinarySemaphore in SerializedScriptValue::writeBlobsToDiskForIndexedDBSynchronously() instead of a Condition. This simplifies the code a bit. 1738612 1 422963 cdumez 2021-03-11 14:05:23 -0800 Created attachment 422963 Patch 1738613 2 422963 ggaren 2021-03-11 14:07:39 -0800 Comment on attachment 422963 Patch r=me 1738614 3 achristensen 2021-03-11 14:08:30 -0800 View in context: https://bugs.webkit.org/attachment.cgi?id=422963&action=review > Source/WebCore/bindings/js/SerializedScriptValue.cpp:4159 > + callOnMainThread([this, &semaphore, &value] { Won't this do something different in UIWebView? Does this matter? 1738616 4 cdumez 2021-03-11 14:11:50 -0800 (In reply to Alex Christensen from comment #3) > View in context: > https://bugs.webkit.org/attachment.cgi?id=422963&action=review > > > Source/WebCore/bindings/js/SerializedScriptValue.cpp:4159 > > + callOnMainThread([this, &semaphore, &value] { > > Won't this do something different in UIWebView? Does this matter? I mentioned this in the changelog. I changed this intentionally. We're in WebCore here so we should be using callOnMainThread() in my opinion. Also note that the surrounding code is using isMainThread(), not isMainRunLoop(). 1738617 5 422963 achristensen 2021-03-11 14:12:23 -0800 Comment on attachment 422963 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422963&action=review Sounds good > Source/WebCore/bindings/js/SerializedScriptValue.cpp:4159 > + callOnMainThread([this, &semaphore, &value] { Won't this do something different in UIWebView? Does this matter? 1738620 6 cdumez 2021-03-11 14:16:22 -0800 (In reply to Chris Dumez from comment #4) > (In reply to Alex Christensen from comment #3) > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=422963&action=review > > > > > Source/WebCore/bindings/js/SerializedScriptValue.cpp:4159 > > > + callOnMainThread([this, &semaphore, &value] { > > > > Won't this do something different in UIWebView? Does this matter? > > I mentioned this in the changelog. I changed this intentionally. We're in > WebCore here so we should be using callOnMainThread() in my opinion. Also > note that the surrounding code is using isMainThread(), not isMainRunLoop(). I also checked the call site, it does: if (!isMainThread()) { value->writeBlobsToDiskForIndexedDBSynchronously(); return; } value->writeBlobsToDiskForIndexedDB() So for this call site, it was OK to call writeBlobsToDiskForIndexedDB() directly if isMainThread() returns true (potentially being the WebThread). 1738633 7 ews-feeder 2021-03-11 14:40:05 -0800 Committed r274300: <https://commits.webkit.org/r274300> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422963. 1738637 8 webkit-bug-importer 2021-03-11 14:41:18 -0800 <rdar://problem/75333893> 422963 2021-03-11 14:05:23 -0800 2021-03-11 14:40:06 -0800 Patch bug-223089-20210311140522.patch text/plain 2476 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjc0Mjk2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDI2YjM4ZGVhYTY1YmIz OTAwMjU4NDI1NzkwZjIxN2FkZGU2NTZlOC4uNjlkOTY1OWM0MzE1MWFmOGQ3ODkzODI3YmJjOTg0 Zjk0ODMyZjViMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDIxLTAzLTExICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgVXNlIEJpbmFyeVNlbWFwaG9y ZSBpbiBTZXJpYWxpemVkU2NyaXB0VmFsdWU6OndyaXRlQmxvYnNUb0Rpc2tGb3JJbmRleGVkREJT eW5jaHJvbm91c2x5KCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTIyMzA4OQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFVzZSBCaW5hcnlTZW1hcGhvcmUgaW4gU2VyaWFsaXplZFNjcmlwdFZhbHVlOjp3cml0 ZUJsb2JzVG9EaXNrRm9ySW5kZXhlZERCU3luY2hyb25vdXNseSgpIGluc3RlYWQgb2YgYQorICAg ICAgICBDb25kaXRpb24uIFRoaXMgc2ltcGxpZmllcyB0aGUgY29kZSBhIGJpdC4gQWxzbyB1c2Ug Y2FsbE9uTWFpblRocmVhZCgpIGluc3RlYWQgb2YgYSBtYWluIFJ1bkxvb3AKKyAgICAgICAgZGlz cGF0Y2guIFdlJ3JlIGluIFdlYkNvcmUgYW5kIGl0IHNlZW1zIHNhZmVyIHdpdGggcmVnYXJkcyB0 byBpT1MgV0sxIChXZWJUaHJlYWQpLgorCisgICAgICAgICogYmluZGluZ3MvanMvU2VyaWFsaXpl ZFNjcmlwdFZhbHVlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNlcmlhbGl6ZWRTY3JpcHRWYWx1 ZTo6d3JpdGVCbG9ic1RvRGlza0ZvckluZGV4ZWREQlN5bmNocm9ub3VzbHkpOgorCiAyMDIxLTAz LTExICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgSW50cm9kdWNl IFdvcmtRdWV1ZTo6bWFpbigpIHRvIGdldCB0aGUgbWFpbiB0aHJlYWQncyB3b3JrIHF1ZXVlCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9TZXJpYWxpemVkU2NyaXB0VmFs dWUuY3BwIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvU2VyaWFsaXplZFNjcmlwdFZhbHVl LmNwcAppbmRleCA3NGEwOTllMjdjOTJiYTdmNWQwZDgwNzIxMDZkOGU0OGFlNTFlNDMxLi4zNGM1 M2Y0M2MzYWRlM2NjZTIxNDhiMjBjZDQwZGEzYWUwZTQxZjlmIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViQ29yZS9iaW5kaW5ncy9qcy9TZXJpYWxpemVkU2NyaXB0VmFsdWUuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL2JpbmRpbmdzL2pzL1NlcmlhbGl6ZWRTY3JpcHRWYWx1ZS5jcHAKQEAgLTkwLDYg KzkwLDcgQEAKICNpbmNsdWRlIDx3dGYvTWFpblRocmVhZC5oPgogI2luY2x1ZGUgPHd0Zi9SdW5M b29wLmg+CiAjaW5jbHVkZSA8d3RmL1ZlY3Rvci5oPgorI2luY2x1ZGUgPHd0Zi90aHJlYWRzL0Jp bmFyeVNlbWFwaG9yZS5oPgogCiAjaWYgRU5BQkxFKE9GRlNDUkVFTl9DQU5WQVMpCiAjaW5jbHVk ZSAiSlNPZmZzY3JlZW5DYW52YXMuaCIKQEAgLTQxNTMsMjEgKzQxNTQsMTcgQEAgSURCVmFsdWUg U2VyaWFsaXplZFNjcmlwdFZhbHVlOjp3cml0ZUJsb2JzVG9EaXNrRm9ySW5kZXhlZERCU3luY2hy b25vdXNseSgpCiB7CiAgICAgQVNTRVJUKCFpc01haW5UaHJlYWQoKSk7CiAKKyAgICBCaW5hcnlT ZW1hcGhvcmUgc2VtYXBob3JlOwogICAgIElEQlZhbHVlIHZhbHVlOwotICAgIExvY2sgbG9jazsK LSAgICBDb25kaXRpb24gY29uZGl0aW9uOwotICAgIGxvY2subG9jaygpOwotCi0gICAgUnVuTG9v cDo6bWFpbigpLmRpc3BhdGNoKFt0aGlzLCBjb25kaXRpb25QdHIgPSAmY29uZGl0aW9uLCB2YWx1 ZVB0ciA9ICZ2YWx1ZV0gewotICAgICAgICB3cml0ZUJsb2JzVG9EaXNrRm9ySW5kZXhlZERCKFtj b25kaXRpb25QdHIsIHZhbHVlUHRyXShJREJWYWx1ZSYmIHJlc3VsdCkgeworICAgIGNhbGxPbk1h aW5UaHJlYWQoW3RoaXMsICZzZW1hcGhvcmUsICZ2YWx1ZV0geworICAgICAgICB3cml0ZUJsb2Jz VG9EaXNrRm9ySW5kZXhlZERCKFsmc2VtYXBob3JlLCAmdmFsdWVdKElEQlZhbHVlJiYgcmVzdWx0 KSB7CiAgICAgICAgICAgICBBU1NFUlQoaXNNYWluVGhyZWFkKCkpOwotICAgICAgICAgICAgdmFs dWVQdHItPnNldEFzSXNvbGF0ZWRDb3B5KHJlc3VsdCk7CisgICAgICAgICAgICB2YWx1ZS5zZXRB c0lzb2xhdGVkQ29weShyZXN1bHQpOwogCi0gICAgICAgICAgICBjb25kaXRpb25QdHItPm5vdGlm eUFsbCgpOworICAgICAgICAgICAgc2VtYXBob3JlLnNpZ25hbCgpOwogICAgICAgICB9KTsKICAg ICB9KTsKLQotICAgIGNvbmRpdGlvbi53YWl0KGxvY2spOworICAgIHNlbWFwaG9yZS53YWl0KCk7 CiAKICAgICByZXR1cm4gdmFsdWU7CiB9Cg==