<!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>[178850] 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/178850">178850</a></dd>
<dt>Author</dt> <dd>zandobersek@gmail.com</dd>
<dt>Date</dt> <dd>2015-01-21 11:13:58 -0800 (Wed, 21 Jan 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
https://bugs.webkit.org/show_bug.cgi?id=139724
Reviewed by Darin Adler.
AsyncRequestImpl casting in AsyncRequest::completeRequest() relies on
the parameter pack to properly determine the type of the AsyncRequestImpl
object. The casting result can be incorrect when an argument is passed
by reference while the original object was constructed with a callback
that expects an argument that's passed by value.
To avoid this the AsyncRequestImpl template is now instantiated with
raw types, with callback parameters remaining raw types in case
of being integral (enforcing passing-by-value these arguments when
dispatching), and const references being used otherwise (enforcing
passing-by-reference).
This enables casting AsyncRequest objects to the correct AsyncRequestImpl<>
by running the parameter pack through std::decay.
* DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp:
(WebKit::UniqueIDBDatabase::putRecord):
(WebKit::UniqueIDBDatabase::getRecord):
(WebKit::UniqueIDBDatabase::openCursor):
(WebKit::UniqueIDBDatabase::cursorAdvance):
(WebKit::UniqueIDBDatabase::cursorIterate):
(WebKit::UniqueIDBDatabase::count):
(WebKit::UniqueIDBDatabase::deleteRange):
* Shared/AsyncRequest.h:
(WebKit::AsyncRequest::completeRequest):
* WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:
(WebKit::WebIDBServerConnection::getOrEstablishIDBDatabaseMetadata):
(WebKit::WebIDBServerConnection::get):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2DatabaseProcessIndexedDBUniqueIDBDatabasecpp">trunk/Source/WebKit2/DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp</a></li>
<li><a href="#trunkSourceWebKit2SharedAsyncRequesth">trunk/Source/WebKit2/Shared/AsyncRequest.h</a></li>
<li><a href="#trunkSourceWebKit2WebProcessDatabasesIndexedDBWebIDBServerConnectioncpp">trunk/Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (178849 => 178850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2015-01-21 18:59:08 UTC (rev 178849)
+++ trunk/Source/WebKit2/ChangeLog 2015-01-21 19:13:58 UTC (rev 178850)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2015-01-21 Zan Dobersek <zdobersek@igalia.com>
+
+ AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=139724
+
+ Reviewed by Darin Adler.
+
+ AsyncRequestImpl casting in AsyncRequest::completeRequest() relies on
+ the parameter pack to properly determine the type of the AsyncRequestImpl
+ object. The casting result can be incorrect when an argument is passed
+ by reference while the original object was constructed with a callback
+ that expects an argument that's passed by value.
+
+ To avoid this the AsyncRequestImpl template is now instantiated with
+ raw types, with callback parameters remaining raw types in case
+ of being integral (enforcing passing-by-value these arguments when
+ dispatching), and const references being used otherwise (enforcing
+ passing-by-reference).
+
+ This enables casting AsyncRequest objects to the correct AsyncRequestImpl<>
+ by running the parameter pack through std::decay.
+
+ * DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp:
+ (WebKit::UniqueIDBDatabase::putRecord):
+ (WebKit::UniqueIDBDatabase::getRecord):
+ (WebKit::UniqueIDBDatabase::openCursor):
+ (WebKit::UniqueIDBDatabase::cursorAdvance):
+ (WebKit::UniqueIDBDatabase::cursorIterate):
+ (WebKit::UniqueIDBDatabase::count):
+ (WebKit::UniqueIDBDatabase::deleteRange):
+ * Shared/AsyncRequest.h:
+ (WebKit::AsyncRequest::completeRequest):
+ * WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:
+ (WebKit::WebIDBServerConnection::getOrEstablishIDBDatabaseMetadata):
+ (WebKit::WebIDBServerConnection::get):
+
</ins><span class="cx"> 2015-01-21 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> [WK2] Stop using WKDiagnosticLoggingResultType.h C API header in Cocoa API
</span></span></pre></div>
<a id="trunkSourceWebKit2DatabaseProcessIndexedDBUniqueIDBDatabasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp (178849 => 178850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp 2015-01-21 18:59:08 UTC (rev 178849)
+++ trunk/Source/WebKit2/DatabaseProcess/IndexedDB/UniqueIDBDatabase.cpp 2015-01-21 19:13:58 UTC (rev 178850)
</span><span class="lines">@@ -568,7 +568,7 @@
</span><span class="cx">
</span><span class="cx"> ASSERT(m_metadata->objectStores.contains(objectStoreID));
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<const IDBKeyData&, uint32_t, const String&>::create([this, callback](const IDBKeyData& keyData, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<IDBKeyData, uint32_t, String>::create([this, callback](const IDBKeyData& keyData, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(keyData, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(IDBKeyData(), INVALID_STATE_ERR, "Unable to put record into database");
</span><span class="lines">@@ -591,7 +591,7 @@
</span><span class="cx">
</span><span class="cx"> ASSERT(m_metadata->objectStores.contains(objectStoreID));
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<const IDBGetResult&, uint32_t, const String&>::create([this, callback](const IDBGetResult& result, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<IDBGetResult, uint32_t, String>::create([this, callback](const IDBGetResult& result, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(result, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(IDBGetResult(), INVALID_STATE_ERR, "Unable to get record from database");
</span><span class="lines">@@ -614,7 +614,7 @@
</span><span class="cx">
</span><span class="cx"> ASSERT(m_metadata->objectStores.contains(objectStoreID));
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<int64_t, const IDBKeyData&, const IDBKeyData&, PassRefPtr<SharedBuffer>, uint32_t, const String&>::create([this, callback](int64_t cursorID, const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<int64_t, IDBKeyData, IDBKeyData, PassRefPtr<SharedBuffer>, uint32_t, String>::create([this, callback](int64_t cursorID, const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(cursorID, key, primaryKey, value, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(0, nullptr, nullptr, nullptr, INVALID_STATE_ERR, "Unable to get record from database");
</span><span class="lines">@@ -635,7 +635,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<const IDBKeyData&, const IDBKeyData&, PassRefPtr<SharedBuffer>, uint32_t, const String&>::create([this, callback](const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<IDBKeyData, IDBKeyData, PassRefPtr<SharedBuffer>, uint32_t, String>::create([this, callback](const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(key, primaryKey, value, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(nullptr, nullptr, nullptr, INVALID_STATE_ERR, "Unable to advance cursor in database");
</span><span class="lines">@@ -656,7 +656,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<const IDBKeyData&, const IDBKeyData&, PassRefPtr<SharedBuffer>, uint32_t, const String&>::create([this, callback](const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<IDBKeyData, IDBKeyData, PassRefPtr<SharedBuffer>, uint32_t, String>::create([this, callback](const IDBKeyData& key, const IDBKeyData& primaryKey, PassRefPtr<SharedBuffer> value, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(key, primaryKey, value, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(nullptr, nullptr, nullptr, INVALID_STATE_ERR, "Unable to iterate cursor in database");
</span><span class="lines">@@ -677,7 +677,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<int64_t, uint32_t, const String&>::create([this, callback](int64_t count, uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<int64_t, uint32_t, String>::create([this, callback](int64_t count, uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(count, errorCode, errorMessage);
</span><span class="cx"> }, [this, callback] {
</span><span class="cx"> callback(0, INVALID_STATE_ERR, "Unable to get count from database");
</span><span class="lines">@@ -698,7 +698,7 @@
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- RefPtr<AsyncRequest> request = AsyncRequestImpl<uint32_t, const String&>::create([callback](uint32_t errorCode, const String& errorMessage) {
</del><ins>+ RefPtr<AsyncRequest> request = AsyncRequestImpl<uint32_t, String>::create([callback](uint32_t errorCode, const String& errorMessage) {
</ins><span class="cx"> callback(errorCode, errorMessage);
</span><span class="cx"> }, [callback] {
</span><span class="cx"> callback(INVALID_STATE_ERR, "Unable to deleteRange from database");
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedAsyncRequesth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/AsyncRequest.h (178849 => 178850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/AsyncRequest.h 2015-01-21 18:59:08 UTC (rev 178849)
+++ trunk/Source/WebKit2/Shared/AsyncRequest.h 2015-01-21 19:13:58 UTC (rev 178850)
</span><span class="lines">@@ -57,12 +57,14 @@
</span><span class="cx"> template <typename... Arguments>
</span><span class="cx"> class AsyncRequestImpl final : public AsyncRequest {
</span><span class="cx"> public:
</span><del>- static PassRefPtr<AsyncRequest> create(std::function<void (Arguments...)> completionHandler)
</del><ins>+ template<typename T> using ArgumentType = std::conditional<std::is_integral<T>::value, T, const T&>;
+
+ static PassRefPtr<AsyncRequest> create(std::function<void (typename ArgumentType<Arguments>::type...)> completionHandler)
</ins><span class="cx"> {
</span><span class="cx"> return adoptRef(new AsyncRequestImpl<Arguments...>(WTF::move(completionHandler), nullptr));
</span><span class="cx"> }
</span><span class="cx">
</span><del>- static PassRefPtr<AsyncRequest> create(std::function<void (Arguments...)> completionHandler, std::function<void ()> abortHandler)
</del><ins>+ static PassRefPtr<AsyncRequest> create(std::function<void (typename ArgumentType<Arguments>::type...)> completionHandler, std::function<void ()> abortHandler)
</ins><span class="cx"> {
</span><span class="cx"> return adoptRef(new AsyncRequestImpl<Arguments...>(WTF::move(completionHandler), WTF::move(abortHandler)));
</span><span class="cx"> }
</span><span class="lines">@@ -72,14 +74,15 @@
</span><span class="cx"> ASSERT(!m_completionHandler);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void completeRequest(Arguments&&... arguments)
</del><ins>+ template<typename... RequestArguments>
+ void completeRequest(RequestArguments&&... arguments)
</ins><span class="cx"> {
</span><del>- m_completionHandler(std::forward<Arguments>(arguments)...);
</del><ins>+ m_completionHandler(std::forward<RequestArguments>(arguments)...);
</ins><span class="cx"> m_completionHandler = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- AsyncRequestImpl(std::function<void (Arguments...)> completionHandler, std::function<void ()> abortHandler)
</del><ins>+ AsyncRequestImpl(std::function<void (typename ArgumentType<Arguments>::type...)> completionHandler, std::function<void ()> abortHandler)
</ins><span class="cx"> : AsyncRequest(WTF::move(abortHandler))
</span><span class="cx"> , m_completionHandler(WTF::move(completionHandler))
</span><span class="cx"> {
</span><span class="lines">@@ -91,12 +94,12 @@
</span><span class="cx"> m_completionHandler = nullptr;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- std::function<void (Arguments...)> m_completionHandler;
</del><ins>+ std::function<void (typename ArgumentType<Arguments>::type...)> m_completionHandler;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> template<typename... Arguments> void AsyncRequest::completeRequest(Arguments&&... arguments)
</span><span class="cx"> {
</span><del>- AsyncRequestImpl<Arguments...>* request = static_cast<AsyncRequestImpl<Arguments...>*>(this);
</del><ins>+ auto* request = static_cast<AsyncRequestImpl<typename std::decay<Arguments>::type...>*>(this);
</ins><span class="cx"> request->completeRequest(std::forward<Arguments>(arguments)...);
</span><span class="cx"> m_abortHandler = nullptr;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessDatabasesIndexedDBWebIDBServerConnectioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp (178849 => 178850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp 2015-01-21 18:59:08 UTC (rev 178849)
+++ trunk/Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp 2015-01-21 19:13:58 UTC (rev 178850)
</span><span class="lines">@@ -114,7 +114,7 @@
</span><span class="cx">
</span><span class="cx"> void WebIDBServerConnection::getOrEstablishIDBDatabaseMetadata(GetIDBDatabaseMetadataFunction completionCallback)
</span><span class="cx"> {
</span><del>- RefPtr<AsyncRequest> serverRequest = AsyncRequestImpl<const IDBDatabaseMetadata&, bool>::create(completionCallback);
</del><ins>+ RefPtr<AsyncRequest> serverRequest = AsyncRequestImpl<IDBDatabaseMetadata, bool>::create(completionCallback);
</ins><span class="cx">
</span><span class="cx"> serverRequest->setAbortHandler([completionCallback] {
</span><span class="cx"> IDBDatabaseMetadata metadata;
</span><span class="lines">@@ -399,7 +399,7 @@
</span><span class="cx">
</span><span class="cx"> void WebIDBServerConnection::get(IDBTransactionBackend& transaction, const GetOperation& operation, std::function<void (const IDBGetResult&, PassRefPtr<IDBDatabaseError>)> completionCallback)
</span><span class="cx"> {
</span><del>- RefPtr<AsyncRequest> serverRequest = AsyncRequestImpl<const IDBGetResult&, PassRefPtr<IDBDatabaseError>>::create(completionCallback);
</del><ins>+ RefPtr<AsyncRequest> serverRequest = AsyncRequestImpl<IDBGetResult, PassRefPtr<IDBDatabaseError>>::create(completionCallback);
</ins><span class="cx">
</span><span class="cx"> serverRequest->setAbortHandler([completionCallback] {
</span><span class="cx"> completionCallback(IDBGetResult(), IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Unknown error occured getting record"));
</span></span></pre>
</div>
</div>
</body>
</html>