Unreviewed, rolling out r201693.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Jun 2016 14:54:36 +0000 (14:54 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Jun 2016 14:54:36 +0000 (14:54 +0000)
Can't fix right now

Reverted changeset:

"Modern IDB: Crash seen in IDBConnectionProxy::putOrAdd on
GuardMalloc bot"
https://bugs.webkit.org/show_bug.cgi?id=158124
http://trac.webkit.org/changeset/201693

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp
Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h
Source/WebCore/bindings/js/SerializedScriptValue.cpp
Source/WebCore/bindings/js/SerializedScriptValue.h
Source/WebCore/platform/network/BlobRegistry.h
Source/WebCore/platform/network/BlobRegistryImpl.cpp
Source/WebCore/platform/network/BlobRegistryImpl.h

index e5119bc..4066d14 100644 (file)
@@ -1,3 +1,16 @@
+2016-06-05  Brady Eidson  <beidson@apple.com>
+
+        Unreviewed, rolling out r201693.
+
+        Can't fix right now
+
+        Reverted changeset:
+
+        "Modern IDB: Crash seen in IDBConnectionProxy::putOrAdd on
+        GuardMalloc bot"
+        https://bugs.webkit.org/show_bug.cgi?id=158124
+        http://trac.webkit.org/changeset/201693
+
 2016-06-04  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: Crash seen in IDBConnectionProxy::putOrAdd on GuardMalloc bot
index 012d524..d7e4ef6 100644 (file)
@@ -966,18 +966,20 @@ void IDBTransaction::putOrAddOnServer(IDBClient::TransactionOperation& operation
         return;
     }
 
-    value->writeBlobsToDiskForIndexedDB([protectedThis = Ref<IDBTransaction>(*this), this, protectedOperation = Ref<IDBClient::TransactionOperation>(operation), keyData = IDBKeyData(key.get()).isolatedCopy(), overwriteMode](const IDBValue& idbValue) mutable {
+    RefPtr<IDBTransaction> protectedThis(this);
+    RefPtr<IDBClient::TransactionOperation> protectedOperation(&operation);
+    value->writeBlobsToDiskForIndexedDB([protectedThis = WTFMove(protectedThis), this, protectedOperation = WTFMove(protectedOperation), key = WTFMove(key), overwriteMode](const IDBValue& idbValue) mutable {
         ASSERT(currentThread() == originThreadID());
         ASSERT(isMainThread());
         if (idbValue.data().data()) {
-            m_database->connectionProxy().putOrAdd(protectedOperation.get(), WTFMove(keyData), idbValue, overwriteMode);
+            m_database->connectionProxy().putOrAdd(*protectedOperation, key.get(), idbValue, overwriteMode);
             return;
         }
 
         // If the IDBValue doesn't have any data, then something went wrong writing the blobs to disk.
         // In that case, we cannot successfully store this record, so we callback with an error.
         auto result = IDBResultData::error(protectedOperation->identifier(), { IDBDatabaseException::UnknownError, ASCIILiteral("Error preparing Blob/File data to be stored in object store") });
-        callOnMainThread([protectedThis = WTFMove(protectedThis), protectedOperation = WTFMove(protectedOperation), result = WTFMove(result)]() mutable {
+        callOnMainThread([protectedThis = WTFMove(protectedThis), protectedOperation = WTFMove(protectedOperation), result = WTFMove(result)]() {
             protectedOperation->completed(result);
         });
     });
index bb560b1..64ee930 100644 (file)
@@ -156,12 +156,12 @@ void IDBConnectionProxy::deleteIndex(TransactionOperation& operation, uint64_t o
     callConnectionOnMainThread(&IDBConnectionToServer::deleteIndex, requestData, WTFMove(objectStoreIdentifier), indexName);
 }
 
-void IDBConnectionProxy::putOrAdd(TransactionOperation& operation, IDBKeyData&& keyData, const IDBValue& value, const IndexedDB::ObjectStoreOverwriteMode mode)
+void IDBConnectionProxy::putOrAdd(TransactionOperation& operation, IDBKey* key, const IDBValue& value, const IndexedDB::ObjectStoreOverwriteMode mode)
 {
     const IDBRequestData requestData(operation);
     saveOperation(operation);
 
-    callConnectionOnMainThread(&IDBConnectionToServer::putOrAdd, requestData, keyData, value, mode);
+    callConnectionOnMainThread(&IDBConnectionToServer::putOrAdd, requestData, IDBKeyData(key), value, mode);
 }
 
 void IDBConnectionProxy::getRecord(TransactionOperation& operation, const IDBKeyRangeData& keyRange)
index 9d58a81..4d370c9 100644 (file)
@@ -70,7 +70,7 @@ public:
     void clearObjectStore(TransactionOperation&, uint64_t objectStoreIdentifier);
     void createIndex(TransactionOperation&, const IDBIndexInfo&);
     void deleteIndex(TransactionOperation&, uint64_t objectStoreIdentifier, const String& indexName);
-    void putOrAdd(TransactionOperation&, IDBKeyData&&, const IDBValue&, const IndexedDB::ObjectStoreOverwriteMode);
+    void putOrAdd(TransactionOperation&, IDBKey*, const IDBValue&, const IndexedDB::ObjectStoreOverwriteMode);
     void getRecord(TransactionOperation&, const IDBKeyRangeData&);
     void getCount(TransactionOperation&, const IDBKeyRangeData&);
     void deleteRecord(TransactionOperation&, const IDBKeyRangeData&);
@@ -124,7 +124,7 @@ private:
     void callConnectionOnMainThread(void (IDBConnectionToServer::*method)(Parameters...), Arguments&&... arguments)
     {
         if (isMainThread())
-            (m_connectionToServer.*method)(std::forward<Arguments>(arguments)...);
+            (m_connectionToServer.*method)(arguments...);
         else
             postMainThreadTask(m_connectionToServer, method, arguments...);
     }
index b202891..4b9ac9a 100644 (file)
@@ -2796,7 +2796,7 @@ Vector<String> SerializedScriptValue::blobURLsIsolatedCopy() const
     return result;
 }
 
-void SerializedScriptValue::writeBlobsToDiskForIndexedDB(NoncopyableFunction<void (const IDBValue&)>&& completionHandler)
+void SerializedScriptValue::writeBlobsToDiskForIndexedDB(std::function<void (const IDBValue&)> completionHandler)
 {
     ASSERT(isMainThread());
     ASSERT(hasBlobURLs());
index 4c670e6..7c75682 100644 (file)
@@ -31,7 +31,6 @@
 #include <runtime/ArrayBuffer.h>
 #include <runtime/JSCJSValue.h>
 #include <wtf/Forward.h>
-#include <wtf/NoncopyableFunction.h>
 #include <wtf/RefCounted.h>
 #include <wtf/text/WTFString.h>
 
@@ -87,7 +86,7 @@ public:
 
 #if ENABLE(INDEXED_DATABASE)
     Vector<String> blobURLsIsolatedCopy() const;
-    void writeBlobsToDiskForIndexedDB(NoncopyableFunction<void (const IDBValue&)>&& completionHandler);
+    void writeBlobsToDiskForIndexedDB(std::function<void (const IDBValue&)> completionHandler);
     IDBValue writeBlobsToDiskForIndexedDBSynchronously();
 #endif // ENABLE(INDEXED_DATABASE)
 
index 12c6e92..e23cfc0 100644 (file)
@@ -33,7 +33,6 @@
 
 #include <functional>
 #include <wtf/Forward.h>
-#include <wtf/NoncopyableFunction.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -68,7 +67,7 @@ public:
 
     virtual unsigned long long blobSize(const URL&) = 0;
 
-    virtual void writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, NoncopyableFunction<void (const Vector<String>& filePaths)>&& completionHandler) = 0;
+    virtual void writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, std::function<void (const Vector<String>& filePaths)> completionHandler) = 0;
 
     virtual bool isBlobRegistryImpl() const { return false; }
 
index 5d4aac8..ba28f32 100644 (file)
@@ -256,7 +256,7 @@ struct BlobForFileWriting {
     Vector<std::pair<String, ThreadSafeDataBuffer>> filePathsOrDataBuffers;
 };
 
-void BlobRegistryImpl::writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, NoncopyableFunction<void (const Vector<String>& filePaths)>&& completionHandler)
+void BlobRegistryImpl::writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, std::function<void (const Vector<String>& filePaths)> completionHandler)
 {
     Vector<BlobForFileWriting> blobsForWriting;
     for (auto& url : blobURLs) {
@@ -287,45 +287,47 @@ void BlobRegistryImpl::writeBlobsToTemporaryFiles(const Vector<String>& blobURLs
     blobUtilityQueue().dispatch([blobsForWriting = WTFMove(blobsForWriting), completionHandler = WTFMove(completionHandler)]() mutable {
         Vector<String> filePaths;
 
-        auto performWriting = [blobsForWriting = WTFMove(blobsForWriting), &filePaths]() {
-            for (auto& blob : blobsForWriting) {
-                PlatformFileHandle file;
-                String tempFilePath = openTemporaryFile(ASCIILiteral("Blob"), file);
-
-                ScopeGuard fileCloser([file]() mutable {
-                    closeFile(file);
-                });
-                
-                if (tempFilePath.isEmpty() || !isHandleValid(file)) {
-                    LOG_ERROR("Failed to open temporary file for writing a Blob to IndexedDB");
-                    return false;
-                }
+        ScopeGuard completionCaller([completionHandler]() mutable {
+            callOnMainThread([completionHandler = WTFMove(completionHandler)]() {
+                Vector<String> filePaths;
+                completionHandler(filePaths);
+            });
+        });
+
+        for (auto& blob : blobsForWriting) {
+            PlatformFileHandle file;
+            String tempFilePath = openTemporaryFile(ASCIILiteral("Blob"), file);
+
+            ScopeGuard fileCloser([file]() {
+                PlatformFileHandle handle = file;
+                closeFile(handle);
+            });
+            
+            if (tempFilePath.isEmpty() || !isHandleValid(file)) {
+                LOG_ERROR("Failed to open temporary file for writing a Blob to IndexedDB");
+                return;
+            }
 
-                for (auto& part : blob.filePathsOrDataBuffers) {
-                    if (part.second.data()) {
-                        int length = part.second.data()->size();
-                        if (writeToFile(file, reinterpret_cast<const char*>(part.second.data()->data()), length) != length) {
-                            LOG_ERROR("Failed writing a Blob to temporary file for storage in IndexedDB");
-                            return false;
-                        }
-                    } else {
-                        ASSERT(!part.first.isEmpty());
-                        if (!appendFileContentsToFileHandle(part.first, file)) {
-                            LOG_ERROR("Failed copying File contents to a Blob temporary file for storage in IndexedDB (%s to %s)", part.first.utf8().data(), tempFilePath.utf8().data());
-                            return false;
-                        }
+            for (auto& part : blob.filePathsOrDataBuffers) {
+                if (part.second.data()) {
+                    int length = part.second.data()->size();
+                    if (writeToFile(file, reinterpret_cast<const char*>(part.second.data()->data()), length) != length) {
+                        LOG_ERROR("Failed writing a Blob to temporary file for storage in IndexedDB");
+                        return;
+                    }
+                } else {
+                    ASSERT(!part.first.isEmpty());
+                    if (!appendFileContentsToFileHandle(part.first, file)) {
+                        LOG_ERROR("Failed copying File contents to a Blob temporary file for storage in IndexedDB (%s to %s)", part.first.utf8().data(), tempFilePath.utf8().data());
+                        return;
                     }
                 }
-
-                filePaths.append(tempFilePath.isolatedCopy());
             }
 
-            return true;
-        };
-
-        if (!performWriting())
-            filePaths.clear();
+            filePaths.append(tempFilePath.isolatedCopy());
+        }
 
+        completionCaller.disable();
         callOnMainThread([completionHandler = WTFMove(completionHandler), filePaths = WTFMove(filePaths)]() {
             completionHandler(filePaths);
         });
index 3cfda98..4e3997a 100644 (file)
@@ -68,7 +68,7 @@ private:
 
     unsigned long long blobSize(const URL&) override;
 
-    void writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, NoncopyableFunction<void (const Vector<String>& filePaths)>&& completionHandler) override;
+    void writeBlobsToTemporaryFiles(const Vector<String>& blobURLs, std::function<void (const Vector<String>& filePaths)> completionHandler) override;
 
     HashMap<String, RefPtr<BlobData>> m_blobs;
 };