Summary: | Use BinarySemaphore in SerializedScriptValue::writeBlobsToDiskForIndexedDBSynchronously() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, alecflett, beidson, darin, ews-watchlist, ggaren, jsbell, sam, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Chris Dumez
2021-03-11 14:03:20 PST
Created attachment 422963 [details]
Patch
Comment on attachment 422963 [details]
Patch
r=me
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? (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(). Comment on attachment 422963 [details] 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? (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). Committed r274300: <https://commits.webkit.org/r274300> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422963 [details]. |