Crash when IndexedDB's getAll is used inside a Web Worker.

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

Reviewed by Andy Estes.

Source/WebCore:

Test: storage/indexeddb/modern/worker-getall.html

* Modules/indexeddb/IDBGetAllResult.cpp:
(WebCore::IDBGetAllResult::IDBGetAllResult): Add an isolated-copying constructor.
(WebCore::IDBGetAllResult::isolatedCopy):
* Modules/indexeddb/IDBGetAllResult.h:

* Modules/indexeddb/shared/IDBResultData.cpp:
(WebCore::IDBResultData::isolatedCopy): Actually copy the IDBGetAllResult.

LayoutTests:

* storage/indexeddb/modern/resources/worker-getall.js: Added.
* storage/indexeddb/modern/worker-getall-expected.txt: Added.
* storage/indexeddb/modern/worker-getall.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 48b4be2..0504fba 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-06-09  Brady Eidson  <beidson@apple.com>
+
+        Crash when IndexedDB's getAll is used inside a Web Worker.
+        https://bugs.webkit.org/show_bug.cgi?id=172434
+
+        Reviewed by Andy Estes.
+
+        * storage/indexeddb/modern/resources/worker-getall.js: Added.
+        * storage/indexeddb/modern/worker-getall-expected.txt: Added.
+        * storage/indexeddb/modern/worker-getall.html: Added.
+
 2017-06-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         Mark fast/mediastream/getUserMedia-grant-persistency3.html as flaky.

diff --git a/LayoutTests/storage/indexeddb/modern/resources/worker-getall.js b/LayoutTests/storage/indexeddb/modern/resources/worker-getall.js
new file mode 100644 (file)
index 0000000..1a5c598
--- /dev/null
+++ b/LayoutTests/storage/indexeddb/modern/resources/worker-getall.js
@@ -0,0 +1,32 @@
+const errorHandler = function (event) {
+  console.error(event.target.error);
+}
+
+console.log('Deleting database...');
+var deleteRequest = indexedDB.deleteDatabase('test');
+deleteRequest.onerror = deleteRequest.onblocked = deleteRequest.onsuccess = function () {
+    console.log('Opening database...');
+    var openRequest = indexedDB.open('test');
+    openRequest.onerror = errorHandler;
+    openRequest.onupgradeneeded = function () {
+        var db = openRequest.result;
+        db.createObjectStore('test', {keyPath: 'a'});
+    }
+    openRequest.onsuccess = function (event) {
+        var db = event.target.result;
+
+        var tx = db.transaction('test', 'readwrite');
+        tx.onerror = errorHandler;
+        tx.onabort = errorHandler;
+        tx.oncomplete = function () {
+            console.log('All done!');
+                       postMessage('All done!');
+        };
+
+        var getAllRequest = tx.objectStore('test').getAll();
+        getAllRequest.onerror = errorHandler;
+        getAllRequest.onsuccess = function () {
+            console.log('Success!');
+        };
+    };
+};

diff --git a/LayoutTests/storage/indexeddb/modern/worker-getall-expected.txt b/LayoutTests/storage/indexeddb/modern/worker-getall-expected.txt
new file mode 100644 (file)
index 0000000..53049c7
--- /dev/null
+++ b/LayoutTests/storage/indexeddb/modern/worker-getall-expected.txt
@@ -0,0 +1 @@
+If this test completes without crashing, it passed.

diff --git a/LayoutTests/storage/indexeddb/modern/worker-getall.html b/LayoutTests/storage/indexeddb/modern/worker-getall.html
new file mode 100644 (file)
index 0000000..38ad0df
--- /dev/null
+++ b/LayoutTests/storage/indexeddb/modern/worker-getall.html
@@ -0,0 +1,14 @@
+<script type="text/javascript">
+if (testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+var w = new Worker('resources/worker-getall.js');
+w.onmessage = function() {
+    if (testRunner)
+        testRunner.notifyDone();
+}
+
+</script>
+If this test completes without crashing, it passed.

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 377c664..2f649e8 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2017-06-09  Brady Eidson  <beidson@apple.com>
+
+        Crash when IndexedDB's getAll is used inside a Web Worker.
+        https://bugs.webkit.org/show_bug.cgi?id=172434
+
+        Reviewed by Andy Estes.
+
+        Test: storage/indexeddb/modern/worker-getall.html
+
+        * Modules/indexeddb/IDBGetAllResult.cpp:
+        (WebCore::IDBGetAllResult::IDBGetAllResult): Add an isolated-copying constructor.
+        (WebCore::IDBGetAllResult::isolatedCopy):
+        * Modules/indexeddb/IDBGetAllResult.h:
+        
+        * Modules/indexeddb/shared/IDBResultData.cpp:
+        (WebCore::IDBResultData::isolatedCopy): Actually copy the IDBGetAllResult.
+
 2017-06-09  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed attempt to fix Mac build after r218039.

diff --git a/Source/WebCore/Modules/indexeddb/IDBGetAllResult.cpp b/Source/WebCore/Modules/indexeddb/IDBGetAllResult.cpp
index 4e7e6f4..4a45a37 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/IDBGetAllResult.cpp
+++ b/Source/WebCore/Modules/indexeddb/IDBGetAllResult.cpp
@@ -42,24 +42,31 @@ template<typename T> void isolatedCopyOfVariant(const WTF::Variant<Vector<IDBKey
         targetVector.uncheckedAppend(element.isolatedCopy());
 }
 
+IDBGetAllResult::IDBGetAllResult(const IDBGetAllResult& that, IsolatedCopyTag)
+{
+    isolatedCopy(that, *this);
+}
+
 IDBGetAllResult IDBGetAllResult::isolatedCopy() const
 {
-    IDBGetAllResult result;
-    result.m_type = m_type;
+    return { *this, IsolatedCopy };
+}
 
-    if (WTF::holds_alternative<std::nullptr_t>(m_results))
-        return result;
+void IDBGetAllResult::isolatedCopy(const IDBGetAllResult& source, IDBGetAllResult& destination)
+{
+    destination.m_type = source.m_type;
+
+    if (WTF::holds_alternative<std::nullptr_t>(source.m_results))
+        return;
 
-    switch (m_type) {
+    switch (source.m_type) {
     case IndexedDB::GetAllType::Keys:
-        isolatedCopyOfVariant<IDBKeyData>(m_results, result.m_results);
+        isolatedCopyOfVariant<IDBKeyData>(source.m_results, destination.m_results);
         break;
     case IndexedDB::GetAllType::Values:
-        isolatedCopyOfVariant<IDBValue>(m_results, result.m_results);
+        isolatedCopyOfVariant<IDBValue>(source.m_results, destination.m_results);
         break;
     }
-
-    return result;
 }
 
 void IDBGetAllResult::addKey(IDBKeyData&& key)

diff --git a/Source/WebCore/Modules/indexeddb/IDBGetAllResult.h b/Source/WebCore/Modules/indexeddb/IDBGetAllResult.h
index e5fc6a9..ef7c257 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/IDBGetAllResult.h
+++ b/Source/WebCore/Modules/indexeddb/IDBGetAllResult.h
@@ -54,6 +54,8 @@ public:
         }
     }
 
+    enum IsolatedCopyTag { IsolatedCopy };
+    IDBGetAllResult(const IDBGetAllResult&, IsolatedCopyTag);
     IDBGetAllResult isolatedCopy() const;
 
     IndexedDB::GetAllType type() const { return m_type; }
@@ -69,6 +71,8 @@ public:
     WEBCORE_EXPORT Vector<String> allBlobFilePaths() const;
 
 private:
+    static void isolatedCopy(const IDBGetAllResult& source, IDBGetAllResult& destination);
+
     IndexedDB::GetAllType m_type { IndexedDB::GetAllType::Keys };
     WTF::Variant<Vector<IDBKeyData>, Vector<IDBValue>, std::nullptr_t> m_results { nullptr };
 };

diff --git a/Source/WebCore/Modules/indexeddb/shared/IDBResultData.cpp b/Source/WebCore/Modules/indexeddb/shared/IDBResultData.cpp
index 6383f10..6c6ae6b 100644 (file)
--- a/Source/WebCore/Modules/indexeddb/shared/IDBResultData.cpp
+++ b/Source/WebCore/Modules/indexeddb/shared/IDBResultData.cpp
@@ -94,6 +94,8 @@ void IDBResultData::isolatedCopy(const IDBResultData& source, IDBResultData& des
         destination.m_resultKey = std::make_unique<IDBKeyData>(*source.m_resultKey, IDBKeyData::IsolatedCopy);
     if (source.m_getResult)
         destination.m_getResult = std::make_unique<IDBGetResult>(*source.m_getResult, IDBGetResult::IsolatedCopy);
+    if (source.m_getAllResult)
+        destination.m_getAllResult = std::make_unique<IDBGetAllResult>(*source.m_getAllResult, IDBGetAllResult::IsolatedCopy);
 }
 
 IDBResultData IDBResultData::error(const IDBResourceIdentifier& requestIdentifier, const IDBError& error)