Bug 113689 – REGRESSION (r147241): Failures in three IDB layout tests on GTK

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Zan Dobersek]

Three IDB layout tests started failing on GTK after r147241.
http://trac.webkit.org/changeset/147241

These are the failing tests:
storage/indexeddb/clone-exception.html
storage/indexeddb/exceptions.html
storage/indexeddb/structured-clone.html

From a quick look at the changes it seems the JSC bindings are not handling this as expected (i.e. as V8 bindings do).

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=storage%2Findexeddb%2Fclone-exception.html%2Cstorage%2Findexeddb%2Fexceptions.html%2Cstorage%2Findexeddb%2Fstructured-clone.html

--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/clone-exception-expected.txt
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/clone-exception-actual.txt
@@ -10,27 +10,21 @@
 
 onUpgradeNeeded():
 Expecting exception from db.createObjectStore('store').put(NON_CLONEABLE, 0);
-PASS Exception was thrown.
-PASS code is 25
-PASS ename is 'DataCloneError'
+FAIL No exception thrown! Should have been 25
 
 doSecondOpen():
 indexedDB.open(dbname + '2')
 
 onUpgradeNeeded():
 Expecting exception from db.createObjectStore('store').put(NON_CLONEABLE, 0);
-PASS Exception was thrown.
-PASS code is 25
-PASS ename is 'DataCloneError'
+FAIL No exception thrown! Should have been 25
 
 doThirdOpen():
 indexedDB.open(dbname + '3')
 
 onUpgradeNeeded():
 Expecting exception from db.createObjectStore('store').put(NON_CLONEABLE, INVALID_KEY);
-PASS Exception was thrown.
-PASS code is 25
-PASS ename is 'DataCloneError'
+FAIL No exception thrown! Should have been 25
 PASS successfullyParsed is true
 
 TEST COMPLETE

--------

--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/exceptions-expected.txt
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/exceptions-actual.txt
@@ -143,8 +143,7 @@
 PASS ename is 'TransactionInactiveError'
 The data being stored could not be cloned by the internal structured cloning algorithm.
 Expecting exception from store.add(self, 0)
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 
 IDBObjectStore.clear()
 This method throws a DOMException of type ReadOnlyError if the transaction which this IDBObjectStore belongs to is has its mode set to "readonly".
@@ -240,8 +239,7 @@
 PASS ename is 'TransactionInactiveError'
 The data being stored could not be cloned by the internal structured cloning algorithm.
 Expecting exception from store.put(self, 0)
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 db.close()
 ro_transaction.oncomplete = transactionComplete
 rw_transaction.oncomplete = transactionComplete
@@ -444,8 +442,7 @@
 PASS ename is 'DataError'
 If the structured clone algorithm throws an exception, that exception is rethrown.
 Expecting exception from cursor.update(self)
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 The transaction this IDBCursor belongs to is not active.
 Expecting exception from cursorFromInactiveTransaction.update({})
 PASS Exception was thrown.

--------

--- /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/structured-clone-expected.txt
+++ /home/slave/webkitgtk/gtk-linux-64-release/build/layout-test-results/storage/indexeddb/structured-clone-actual.txt
@@ -781,22 +781,17 @@
 
 Other JavaScript object types:
 Expecting exception from store.put(new Error, 'key')
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 Expecting exception from store.put(new Function, 'key')
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 
 Other host object types:
 Expecting exception from store.put(self, 'key')
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 Expecting exception from store.put(document, 'key')
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 Expecting exception from store.put(document.body, 'key')
-PASS Exception was thrown.
-PASS code is DOMException.DATA_CLONE_ERR
+FAIL No exception thrown! Should have been DOMException.DATA_CLONE_ERR
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Michael Pruett]

Created attachment 195998 [details]
Patch

[Joshua Bell]

Sorry about that. We need to try and get ScriptState and SSV better aligned *internally* in the V8 binding and between V8 and and JSC.

Some vague notes in https://bugs.webkit.org/show_bug.cgi?id=113091

Apologies for not anticipating this failure. :(

[Michael Pruett]

(In reply to comment #2)
> Sorry about that. We need to try and get ScriptState and SSV better aligned *internally* in the V8 binding and between V8 and and JSC.
> 
> Some vague notes in https://bugs.webkit.org/show_bug.cgi?id=113091
> 
> Apologies for not anticipating this failure. :(

The JSC implementation of ScriptValue::serialize(ScriptState*, MessagePortArray*, ArrayBufferArray*, bool&), added in r116763, was flawed in that it called the specialized SerializedScriptValue::create(JSContextRef, JSValueRef, MessagePortArray*, ArrayBufferArray*, JSValueRef*).

This variant of SerializedScriptValue::create(), which returns the exception thrown in its final parameter and clears the exception on the JSContextRef, was intended for use in the WebKit layer and not in WebCore bindings (r51256).

The fix in r147241 exposed this formerly latent error.

[Kentaro Hara]

Comment on attachment 195998 [details]
Patch

Although a right way to fix the bug would be making SSV's APIs better (in both V8 and JSC), this looks reasonable as an immediate fix.

[WebKit Review Bot]

Comment on attachment 195998 [details]
Patch

Clearing flags on attachment: 195998

Committed r147382: <http://trac.webkit.org/changeset/147382>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.