I misunderstood what the WorkQueue parameter meant in the IO channel operations. It's the queue where the completion handler should be run, not the whole operation. Since our operations are already non-blocking, we can just run the read/writes in the main thread, and schedule the completion handler in the given work queue when the operation finishes.
Created attachment 254561 [details] Patch
Attachment 254561 [details] did not pass style-queue: ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:156: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:178: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:265: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 254561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254561&action=review > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:119 > + auto asyncData = std::unique_ptr<ReadAsyncData>(asyncDataPtr); This can be just std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr); Same for the rest of such statements. > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:188 > + Data data; > + completionHandler(data, -1); You could probably just construct the Data object in the completionHandler call: completionHandler(Data(), -1); > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:191 > + g_cond_wait(&condition, &mutex); Can this function be called on the main thread, with a null queue? That would schedule the callback in the main context and continue to block the main thread in g_cond_wait(), with the callback never invoked. > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:223 > g_cond_wait(&condition, &mutex); Same problem here.
(In reply to comment #3) > Comment on attachment 254561 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254561&action=review > > > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:119 > > + auto asyncData = std::unique_ptr<ReadAsyncData>(asyncDataPtr); > > This can be just > > std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr); > > Same for the rest of such statements. Ok, no much difference anyway. > > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:188 > > + Data data; > > + completionHandler(data, -1); > > You could probably just construct the Data object in the completionHandler > call: > > completionHandler(Data(), -1); No, that doesn't build. no known conversion for argument 1 from ‘WebKit::NetworkCache::Data’ to ‘WebKit::NetworkCache::Data&’ > > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:191 > > + g_cond_wait(&condition, &mutex); > > Can this function be called on the main thread, with a null queue? No this, is always called from a thread. I'll add an assert to ensure it. > That would schedule the callback in the main context and continue to block > the main thread in g_cond_wait(), with the callback never invoked. > > > Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:223 > > g_cond_wait(&condition, &mutex); > > Same problem here. Yes, we definitely don't want to block the main thread ever.
Created attachment 254570 [details] Updated patch
Attachment 254570 [details] did not pass style-queue: ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:157: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:179: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:268: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 254570 [details] Updated patch Looks good.
Committed r185365: <http://trac.webkit.org/changeset/185365>