<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[185365] trunk/Source/WebKit2</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/185365">185365</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-06-09 10:07:19 -0700 (Tue, 09 Jun 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[SOUP] Network Cache: run the IO completion handler in the given queue instead of the whole operation
https://bugs.webkit.org/show_bug.cgi?id=145797
Reviewed by Žan Doberšek.
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.
* NetworkProcess/cache/NetworkCacheIOChannel.h:
* NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
(WebKit::NetworkCache::inputStreamReadReadyCallback):
(WebKit::NetworkCache::IOChannel::read):
(WebKit::NetworkCache::IOChannel::readSync):
(WebKit::NetworkCache::outputStreamWriteReadyCallback):
(WebKit::NetworkCache::IOChannel::write):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheIOChannelh">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h</a></li>
<li><a href="#trunkSourceWebKit2NetworkProcesscacheNetworkCacheIOChannelSoupcpp">trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (185364 => 185365)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/ChangeLog 2015-06-09 17:07:19 UTC (rev 185365)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-06-09 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [SOUP] Network Cache: run the IO completion handler in the given queue instead of the whole operation
+ https://bugs.webkit.org/show_bug.cgi?id=145797
+
+ Reviewed by Žan Doberšek.
+
+ 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.
+
+ * NetworkProcess/cache/NetworkCacheIOChannel.h:
+ * NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
+ (WebKit::NetworkCache::inputStreamReadReadyCallback):
+ (WebKit::NetworkCache::IOChannel::read):
+ (WebKit::NetworkCache::IOChannel::readSync):
+ (WebKit::NetworkCache::outputStreamWriteReadyCallback):
+ (WebKit::NetworkCache::IOChannel::write):
+
</ins><span class="cx"> 2015-06-09 David Kilzer <ddkilzer@apple.com>
</span><span class="cx">
</span><span class="cx"> REGRESSION (r185357): Fix build for iOS 8.x
</span></span></pre></div>
<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheIOChannelh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h (185364 => 185365)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h 2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannel.h 2015-06-09 17:07:19 UTC (rev 185365)
</span><span class="lines">@@ -60,12 +60,6 @@
</span><span class="cx"> private:
</span><span class="cx"> IOChannel(const String& filePath, IOChannel::Type);
</span><span class="cx">
</span><del>-#if USE(SOUP)
- void read(size_t offset, size_t, std::function<void (Data&, int error)>);
- void readSync(size_t offset, size_t, std::function<void (Data&, int error)>);
- void write(size_t offset, const Data&, std::function<void (int error)>);
-#endif
-
</del><span class="cx"> String m_path;
</span><span class="cx"> Type m_type;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2NetworkProcesscacheNetworkCacheIOChannelSoupcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp (185364 => 185365)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp 2015-06-09 16:56:37 UTC (rev 185364)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp 2015-06-09 17:07:19 UTC (rev 185365)
</span><span class="lines">@@ -29,6 +29,7 @@
</span><span class="cx"> #if ENABLE(NETWORK_CACHE)
</span><span class="cx">
</span><span class="cx"> #include "NetworkCacheFileSystemPosix.h"
</span><ins>+#include <wtf/MainThread.h>
</ins><span class="cx"> #include <wtf/gobject/GMainLoopSource.h>
</span><span class="cx"> #include <wtf/gobject/GMutexLocker.h>
</span><span class="cx"> #include <wtf/gobject/GUniquePtr.h>
</span><span class="lines">@@ -102,6 +103,7 @@
</span><span class="cx"> struct ReadAsyncData {
</span><span class="cx"> RefPtr<IOChannel> channel;
</span><span class="cx"> GRefPtr<SoupBuffer> buffer;
</span><ins>+ RefPtr<WorkQueue> queue;
</ins><span class="cx"> size_t bytesToRead;
</span><span class="cx"> std::function<void (Data&, int error)> completionHandler;
</span><span class="cx"> Data data;
</span><span class="lines">@@ -112,12 +114,22 @@
</span><span class="cx"> std::unique_ptr<ReadAsyncData> asyncData(static_cast<ReadAsyncData*>(userData));
</span><span class="cx"> gssize bytesRead = g_input_stream_read_finish(stream, result, nullptr);
</span><span class="cx"> if (bytesRead == -1) {
</span><del>- asyncData->completionHandler(asyncData->data, -1);
</del><ins>+ WorkQueue* queue = asyncData->queue.get();
+ auto* asyncDataPtr = asyncData.release();
+ runTaskInQueue([asyncDataPtr] {
+ std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+ asyncData->completionHandler(asyncData->data, -1);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (!bytesRead) {
</span><del>- asyncData->completionHandler(asyncData->data, 0);
</del><ins>+ WorkQueue* queue = asyncData->queue.get();
+ auto* asyncDataPtr = asyncData.release();
+ runTaskInQueue([asyncDataPtr] {
+ std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+ asyncData->completionHandler(asyncData->data, 0);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -126,7 +138,12 @@
</span><span class="cx">
</span><span class="cx"> size_t pendingBytesToRead = asyncData->bytesToRead - asyncData->data.size();
</span><span class="cx"> if (!pendingBytesToRead) {
</span><del>- asyncData->completionHandler(asyncData->data, 0);
</del><ins>+ WorkQueue* queue = asyncData->queue.get();
+ auto* asyncDataPtr = asyncData.release();
+ runTaskInQueue([asyncDataPtr] {
+ std::unique_ptr<ReadAsyncData> asyncData(asyncDataPtr);
+ asyncData->completionHandler(asyncData->data, 0);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -137,38 +154,44 @@
</span><span class="cx"> reinterpret_cast<GAsyncReadyCallback>(inputStreamReadReadyCallback), asyncData.release());
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IOChannel::read(size_t offset, size_t size, std::function<void (Data&, int error)> completionHandler)
</del><ins>+void IOChannel::read(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
</ins><span class="cx"> {
</span><ins>+ RefPtr<IOChannel> channel(this);
</ins><span class="cx"> if (!m_inputStream) {
</span><del>- Data data;
- completionHandler(data, -1);
</del><ins>+ runTaskInQueue([channel, completionHandler] {
+ Data data;
+ completionHandler(data, -1);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> size_t bufferSize = std::min(size, gDefaultReadBufferSize);
</span><span class="cx"> uint8_t* bufferData = static_cast<uint8_t*>(fastMalloc(bufferSize));
</span><span class="cx"> GRefPtr<SoupBuffer> buffer = adoptGRef(soup_buffer_new_with_owner(bufferData, bufferSize, bufferData, fastFree));
</span><del>- ReadAsyncData* asyncData = new ReadAsyncData { this, buffer.get(), size, completionHandler, { } };
</del><ins>+ ReadAsyncData* asyncData = new ReadAsyncData { this, buffer.get(), queue, size, completionHandler, { } };
</ins><span class="cx">
</span><span class="cx"> // FIXME: implement offset.
</span><span class="cx"> g_input_stream_read_async(m_inputStream.get(), const_cast<char*>(buffer->data), bufferSize, G_PRIORITY_DEFAULT, nullptr,
</span><span class="cx"> reinterpret_cast<GAsyncReadyCallback>(inputStreamReadReadyCallback), asyncData);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IOChannel::read(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
</del><ins>+// FIXME: It would be better to do without this.
+void IOChannel::readSync(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
</ins><span class="cx"> {
</span><ins>+ ASSERT(!isMainThread());
+
+ static GMutex mutex;
+ static GCond condition;
+ WTF::GMutexLocker<GMutex> lock(mutex);
</ins><span class="cx"> RefPtr<IOChannel> channel(this);
</span><del>- runTaskInQueue([channel, offset, size, completionHandler] {
- channel->read(offset, size, completionHandler);
- }, queue);
-}
</del><span class="cx">
</span><del>-// FIXME: It would be better to do without this.
-void IOChannel::readSync(size_t offset, size_t size, std::function<void (Data&, int error)> completionHandler)
-{
</del><span class="cx"> if (!m_inputStream) {
</span><del>- Data data;
- completionHandler(data, -1);
</del><ins>+ runTaskInQueue([channel, completionHandler] {
+ Data data;
+ completionHandler(data, -1);
+ g_cond_signal(&condition);
+ }, queue);
+ g_cond_wait(&condition, &mutex);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -196,18 +219,8 @@
</span><span class="cx"> bytesToRead = std::min(pendingBytesToRead, readBuffer->length);
</span><span class="cx"> } while (pendingBytesToRead);
</span><span class="cx">
</span><del>- completionHandler(data, 0);
-}
-
-void IOChannel::readSync(size_t offset, size_t size, WorkQueue* queue, std::function<void (Data&, int error)> completionHandler)
-{
- static GMutex mutex;
- static GCond condition;
-
- WTF::GMutexLocker<GMutex> lock(mutex);
- RefPtr<IOChannel> channel(this);
- runTaskInQueue([channel, offset, size, completionHandler] {
- channel->readSync(offset, size, completionHandler);
</del><ins>+ runTaskInQueue([channel, &data, completionHandler] {
+ completionHandler(data, 0);
</ins><span class="cx"> g_cond_signal(&condition);
</span><span class="cx"> }, queue);
</span><span class="cx"> g_cond_wait(&condition, &mutex);
</span><span class="lines">@@ -216,6 +229,7 @@
</span><span class="cx"> struct WriteAsyncData {
</span><span class="cx"> RefPtr<IOChannel> channel;
</span><span class="cx"> GRefPtr<SoupBuffer> buffer;
</span><ins>+ RefPtr<WorkQueue> queue;
</ins><span class="cx"> std::function<void (int error)> completionHandler;
</span><span class="cx"> };
</span><span class="cx">
</span><span class="lines">@@ -224,13 +238,23 @@
</span><span class="cx"> std::unique_ptr<WriteAsyncData> asyncData(static_cast<WriteAsyncData*>(userData));
</span><span class="cx"> gssize bytesWritten = g_output_stream_write_finish(stream, result, nullptr);
</span><span class="cx"> if (bytesWritten == -1) {
</span><del>- asyncData->completionHandler(-1);
</del><ins>+ WorkQueue* queue = asyncData->queue.get();
+ auto* asyncDataPtr = asyncData.release();
+ runTaskInQueue([asyncDataPtr] {
+ std::unique_ptr<WriteAsyncData> asyncData(asyncDataPtr);
+ asyncData->completionHandler(-1);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> gssize pendingBytesToWrite = asyncData->buffer->length - bytesWritten;
</span><span class="cx"> if (!pendingBytesToWrite) {
</span><del>- asyncData->completionHandler(0);
</del><ins>+ WorkQueue* queue = asyncData->queue.get();
+ auto* asyncDataPtr = asyncData.release();
+ runTaskInQueue([asyncDataPtr] {
+ std::unique_ptr<WriteAsyncData> asyncData(asyncDataPtr);
+ asyncData->completionHandler(0);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -241,33 +265,30 @@
</span><span class="cx"> reinterpret_cast<GAsyncReadyCallback>(outputStreamWriteReadyCallback), asyncData.release());
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IOChannel::write(size_t offset, const Data& data, std::function<void (int error)> completionHandler)
</del><ins>+void IOChannel::write(size_t offset, const Data& data, WorkQueue* queue, std::function<void (int error)> completionHandler)
</ins><span class="cx"> {
</span><ins>+ RefPtr<IOChannel> channel(this);
</ins><span class="cx"> if (!m_outputStream && !m_ioStream) {
</span><del>- completionHandler(-1);
</del><ins>+ runTaskInQueue([channel, completionHandler] {
+ completionHandler(-1);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> GOutputStream* stream = m_outputStream ? m_outputStream.get() : g_io_stream_get_output_stream(G_IO_STREAM(m_ioStream.get()));
</span><span class="cx"> if (!stream) {
</span><del>- completionHandler(-1);
</del><ins>+ runTaskInQueue([channel, completionHandler] {
+ completionHandler(-1);
+ }, queue);
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- WriteAsyncData* asyncData = new WriteAsyncData { this, data.soupBuffer(), completionHandler };
</del><ins>+ WriteAsyncData* asyncData = new WriteAsyncData { this, data.soupBuffer(), queue, completionHandler };
</ins><span class="cx"> // FIXME: implement offset.
</span><span class="cx"> g_output_stream_write_async(stream, asyncData->buffer->data, data.size(), G_PRIORITY_DEFAULT, nullptr,
</span><span class="cx"> reinterpret_cast<GAsyncReadyCallback>(outputStreamWriteReadyCallback), asyncData);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IOChannel::write(size_t offset, const Data& data, WorkQueue* queue, std::function<void (int error)> completionHandler)
-{
- RefPtr<IOChannel> channel(this);
- runTaskInQueue([channel, offset, data, completionHandler] {
- channel->write(offset, data, completionHandler);
- }, queue);
-}
-
</del><span class="cx"> } // namespace NetworkCache
</span><span class="cx"> } // namespace WebKit
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>