Source/WebCore: The IDB backing store get() method shouldn't call IDB callbacks directly

https://bugs.webkit.org/show_bug.cgi?id=127453

Reviewed by Beth Dakin.

* Modules/indexeddb/IDBServerConnection.h:
(WebCore::IDBGetResult::IDBGetResult): Add a new structure to hold all of the
  possible results of a get() call.

* Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:
(WebCore::IDBServerConnectionLevelDB::get): Don't call IDBCallbacks directly.
  Instead, return the GetResult to the GetOperation which will make IDBCallbacks.
* Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.h:

* Modules/indexeddb/IDBTransactionBackendOperations.cpp:
(WebCore::GetOperation::perform): Get all of the IDBGetResults in the completion callback
  make the appropriate IDBCallback.
* Modules/indexeddb/IDBTransactionBackendOperations.h:

Source/WebKit2: The IDB backing store get() method shouldn't call IDB callbacks directly
https://bugs.webkit.org/show_bug.cgi?id=127453

Reviewed by Beth Dakin.

* WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:
(WebKit::WebIDBServerConnection::get):
* WebProcess/Databases/IndexedDB/WebIDBServerConnection.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@162569 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

+2014-01-22  Brady Eidson  <beidson@apple.com>
+
+        The IDB backing store get() method shouldn't call IDB callbacks directly
+        https://bugs.webkit.org/show_bug.cgi?id=127453
+
+        Reviewed by Beth Dakin.
+
+        * Modules/indexeddb/IDBServerConnection.h:
+        (WebCore::IDBGetResult::IDBGetResult): Add a new structure to hold all of the
+          possible results of a get() call.
+
+        * Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:
+        (WebCore::IDBServerConnectionLevelDB::get): Don't call IDBCallbacks directly.
+          Instead, return the GetResult to the GetOperation which will make IDBCallbacks.
+        * Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.h:
+
+        * Modules/indexeddb/IDBTransactionBackendOperations.cpp:
+        (WebCore::GetOperation::perform): Get all of the IDBGetResults in the completion callback
+          make the appropriate IDBCallback.
+        * Modules/indexeddb/IDBTransactionBackendOperations.h:
+
 2014-01-22  Dean Jackson  <dino@apple.com>
 
         Unreviewed attempt to fix 32-bit builds.

Source/WebCore/Modules/indexeddb/IDBServerConnection.h

@@ -46,6 +46,33 @@ class IDBTransactionBackend;
 struct IDBIndexMetadata;
 struct IDBObjectStoreMetadata;
 
+struct IDBGetResult {
+    IDBGetResult()
+    {
+    }
+
+    IDBGetResult(PassRefPtr<SharedBuffer> buffer)
+        : valueBuffer(buffer)
+    {
+    }
+
+    IDBGetResult(PassRefPtr<IDBKey> idbKey)
+        : key(idbKey)
+    {
+    }
+
+    IDBGetResult(PassRefPtr<SharedBuffer> buffer, PassRefPtr<IDBKey> idbKey, const IDBKeyPath& path)
+        : valueBuffer(buffer)
+        , key(idbKey)
+        , keyPath(path)
+    {
+    }
+
+    RefPtr<SharedBuffer> valueBuffer;
+    RefPtr<IDBKey> key;
+    IDBKeyPath keyPath;
+};
+
 // This interface provides a single asynchronous layer between the web-facing frontend
 // and the I/O performing backend of IndexedDatabase.
 // If an operation's completion needs to be confirmed that must be done through use of a callback function.
@@ -77,7 +104,7 @@ public:
     virtual void createObjectStore(IDBTransactionBackend&, const CreateObjectStoreOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
     virtual void createIndex(IDBTransactionBackend&, const CreateIndexOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
     virtual void deleteIndex(IDBTransactionBackend&, const DeleteIndexOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
-    virtual void get(IDBTransactionBackend&, const GetOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
+    virtual void get(IDBTransactionBackend&, const GetOperation&, std::function<void(const IDBGetResult&, PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
     virtual void put(IDBTransactionBackend&, const PutOperation&, std::function<void(PassRefPtr<IDBKey>, PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
     virtual void openCursor(IDBTransactionBackend&, const OpenCursorOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;
     virtual void count(IDBTransactionBackend&, const CountOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) = 0;

Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.cpp

@@ -94,7 +94,27 @@ void GetOperation::perform(std::function<void()> completionCallback)
     RefPtr<GetOperation> operation(this);
     STANDARD_DATABASE_ERROR_CALLBACK;
 
-    m_transaction->database().serverConnection().get(*m_transaction, *this, operationCallback);
+    m_transaction->database().serverConnection().get(*m_transaction, *this, [this, operation, operationCallback](const IDBGetResult& result, PassRefPtr<IDBDatabaseError> prpError) {
+        RefPtr<IDBDatabaseError> error = prpError;
+
+        if (error)
+            m_callbacks->onError(error);
+        else {
+            if (!result.valueBuffer) {
+                if (!result.key)
+                    m_callbacks->onSuccess();
+                else
+                    m_callbacks->onSuccess(result.key.get());
+            } else {
+                if (result.key)
+                    m_callbacks->onSuccess(result.valueBuffer, result.key, result.keyPath);
+                else
+                    m_callbacks->onSuccess(result.valueBuffer.get());
+            }
+        }
+
+        operationCallback(error.release());
+    });
 }
 
 void PutOperation::perform(std::function<void()> completionCallback)

Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.h

@@ -263,7 +263,6 @@ public:
     int64_t indexID() const { return m_indexID; }
     IndexedDB::CursorType cursorType() const { return m_cursorType; }
     IDBKeyRange* keyRange() const { return m_keyRange.get(); }
-    RefPtr<IDBCallbacks> callbacks() const { return m_callbacks.get(); }
     bool autoIncrement() const { return m_autoIncrement; }
     IDBKeyPath keyPath() const { return m_keyPath; }
 

Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp

@@ -244,7 +244,7 @@ void IDBServerConnectionLevelDB::deleteIndex(IDBTransactionBackend& transaction,
     ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
 }
 
-void IDBServerConnectionLevelDB::get(IDBTransactionBackend& transaction, const GetOperation& operation, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback)
+void IDBServerConnectionLevelDB::get(IDBTransactionBackend& transaction, const GetOperation& operation, std::function<void(const IDBGetResult&, PassRefPtr<IDBDatabaseError>)> completionCallback)
 {
     IDBBackingStoreTransactionLevelDB* backingStoreTransaction = m_backingStoreTransactions.get(transaction.id());
     ASSERT(backingStoreTransaction);
@@ -272,8 +272,7 @@ void IDBServerConnectionLevelDB::get(IDBTransactionBackend& transaction, const G
         }
 
         if (!backingStoreCursor) {
-            operation.callbacks()->onSuccess();
-            ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+            ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), nullptr);
             return;
         }
 
@@ -287,45 +286,44 @@ void IDBServerConnectionLevelDB::get(IDBTransactionBackend& transaction, const G
         Vector<char> value;
         ok = m_backingStore->getRecord(*backingStoreTransaction, transaction.database().id(), operation.objectStoreID(), *key, value);
         if (!ok) {
-            operation.callbacks()->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getRecord."));
-            ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+            ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getRecord."));
             return;
         }
 
         if (value.isEmpty()) {
-            operation.callbacks()->onSuccess();
-            ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+            ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), nullptr);
             return;
         }
 
-        if (operation.autoIncrement() && !operation.keyPath().isNull()) {
-            operation.callbacks()->onSuccess(SharedBuffer::adoptVector(value), key, operation.keyPath());
-            ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
-            return;
-        }
+        IDBGetResult result;
 
-        operation.callbacks()->onSuccess(SharedBuffer::adoptVector(value));
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
-        return;
+        if (operation.autoIncrement() && !operation.keyPath().isNull())
+            result = IDBGetResult(SharedBuffer::adoptVector(value), key, operation.keyPath());
+        else
+            result = IDBGetResult(SharedBuffer::adoptVector(value));
 
+        callOnMainThread([completionCallback, result]() {
+            completionCallback(result, nullptr);
+        });
+        return;
     }
 
     // From here we are dealing only with indexes.
     ok = m_backingStore->getPrimaryKeyViaIndex(*backingStoreTransaction, transaction.database().id(), operation.objectStoreID(), operation.indexID(), *key, primaryKey);
     if (!ok) {
-        operation.callbacks()->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getPrimaryKeyViaIndex."));
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getPrimaryKeyViaIndex."));
         return;
     }
     if (!primaryKey) {
-        operation.callbacks()->onSuccess();
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), nullptr);
         return;
     }
     if (operation.cursorType() == IndexedDB::CursorType::KeyOnly) {
         // Index Value Retrieval Operation
-        operation.callbacks()->onSuccess(primaryKey.get());
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        IDBGetResult result(primaryKey.release());
+        callOnMainThread([completionCallback, result]() {
+            completionCallback(result, nullptr);
+        });
         return;
     }
 
@@ -333,23 +331,27 @@ void IDBServerConnectionLevelDB::get(IDBTransactionBackend& transaction, const G
     Vector<char> value;
     ok = m_backingStore->getRecord(*backingStoreTransaction, transaction.database().id(), operation.objectStoreID(), *primaryKey, value);
     if (!ok) {
-        operation.callbacks()->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getRecord."));
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getRecord."));
         return;
     }
 
     if (value.isEmpty()) {
-        operation.callbacks()->onSuccess();
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        ASYNC_COMPLETION_CALLBACK_WITH_TWO_ARGS(completionCallback, IDBGetResult(), nullptr);
         return;
     }
     if (operation.autoIncrement() && !operation.keyPath().isNull()) {
-        operation.callbacks()->onSuccess(SharedBuffer::adoptVector(value), primaryKey, operation.keyPath());
-        ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+        IDBGetResult result(SharedBuffer::adoptVector(value), key, operation.keyPath());
+        callOnMainThread([completionCallback, result]() {
+            completionCallback(result, nullptr);
+        });
+
         return;
     }
-    operation.callbacks()->onSuccess(SharedBuffer::adoptVector(value));
-    ASYNC_COMPLETION_CALLBACK_WITH_NULL_ARG(completionCallback);
+
+    IDBGetResult result(SharedBuffer::adoptVector(value));
+    callOnMainThread([completionCallback, result]() {
+        completionCallback(result, nullptr);
+    });
 }
 
 void IDBServerConnectionLevelDB::put(IDBTransactionBackend& transaction, const PutOperation& operation, std::function<void(PassRefPtr<IDBKey>, PassRefPtr<IDBDatabaseError>)> completionCallback)

Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.h

@@ -66,7 +66,7 @@ public:
     virtual void createObjectStore(IDBTransactionBackend&, const CreateObjectStoreOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;
     virtual void createIndex(IDBTransactionBackend&, const CreateIndexOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;
     virtual void deleteIndex(IDBTransactionBackend&, const DeleteIndexOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;
-    virtual void get(IDBTransactionBackend&, const GetOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;
+    virtual void get(IDBTransactionBackend&, const GetOperation&, std::function<void(const IDBGetResult&, PassRefPtr<IDBDatabaseError>)> completionCallback) override;
     virtual void put(IDBTransactionBackend&, const PutOperation&, std::function<void(PassRefPtr<IDBKey>, PassRefPtr<IDBDatabaseError>)> completionCallback)  override;
     virtual void openCursor(IDBTransactionBackend&, const OpenCursorOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;
     virtual void count(IDBTransactionBackend&, const CountOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback) override;

Source/WebKit2/ChangeLog

+2014-01-22  Brady Eidson  <beidson@apple.com>
+
+        The IDB backing store get() method shouldn't call IDB callbacks directly
+        https://bugs.webkit.org/show_bug.cgi?id=127453
+        
+        Reviewed by Beth Dakin.
+
+        * WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:
+        (WebKit::WebIDBServerConnection::get):
+        * WebProcess/Databases/IndexedDB/WebIDBServerConnection.h:
+
 2014-01-22  Martin Hock  <mhock@apple.com>
 
         Enable Session API.

Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp

@@ -297,7 +297,7 @@ void WebIDBServerConnection::deleteIndex(IDBTransactionBackend&, const DeleteInd
 {
 }
 
-void WebIDBServerConnection::get(IDBTransactionBackend&, const GetOperation&, std::function<void(PassRefPtr<IDBDatabaseError>)> completionCallback)
+void WebIDBServerConnection::get(IDBTransactionBackend&, const GetOperation&, std::function<void(const IDBGetResult&, PassRefPtr<IDBDatabaseError>)> completionCallback)
 {
 }
 

Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.h

@@ -70,7 +70,7 @@ public:
     virtual void createObjectStore(WebCore::IDBTransactionBackend&, const WebCore::CreateObjectStoreOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
     virtual void createIndex(WebCore::IDBTransactionBackend&, const WebCore::CreateIndexOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
     virtual void deleteIndex(WebCore::IDBTransactionBackend&, const WebCore::DeleteIndexOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
-    virtual void get(WebCore::IDBTransactionBackend&, const WebCore::GetOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
+    virtual void get(WebCore::IDBTransactionBackend&, const WebCore::GetOperation&, std::function<void(const WebCore::IDBGetResult&, PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
     virtual void put(WebCore::IDBTransactionBackend&, const WebCore::PutOperation&, std::function<void(PassRefPtr<WebCore::IDBKey>, PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
     virtual void openCursor(WebCore::IDBTransactionBackend&, const WebCore::OpenCursorOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;
     virtual void count(WebCore::IDBTransactionBackend&, const WebCore::CountOperation&, std::function<void(PassRefPtr<WebCore::IDBDatabaseError>)> completionCallback) override;