JSCallbackData::invokeCallback() should return the Exception to the caller

• Component: Bindings
• Status: RESOLVED FIXED

[Chris Dumez]

JSCallbackData::invokeCallback() calls a callback function. If this function throws an exception, it will report it and clear it on the VM.
However, in the case of NodeFilter, the DOM specification clearly states that we are supposed to rethrow the exception [1].

Once way to support this is to have JSCallbackData::invokeCallback() return the Exception to the caller and let the caller decide what to do with it (i.e. report it or rethrow it).

[1] https://dom.spec.whatwg.org/#traversal

Some more context at https://www.w3.org/Bugs/Public/show_bug.cgi?id=17713#c16

[Chris Dumez]

Created attachment 260185 [details]
Patch

[Mark Lam]

Comment on attachment 260185 [details]
Patch

r=me

[Build Bot]

Comment on attachment 260185 [details]
Patch

Attachment 260185 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/117903

New failing tests:
storage/websql/execute-sql-rowsAffected.html
storage/websql/transaction-success-callback-isolated-world.html
storage/websql/open-database-while-transaction-in-progress.html
storage/websql/transaction-error-callback.html
storage/websql/alter-to-info-table.html
storage/websql/hash-change-with-xhr.html
storage/websql/null-callbacks.html
storage/websql/database-lock-after-reload.html
storage/websql/sql-error-codes.html
storage/websql/executesql-accepts-only-one-statement.html
storage/websql/test-authorizer.html
storage/websql/sql-data-types.html
storage/websql/success-callback.html
storage/websql/change-version-handle-reuse.html
storage/websql/read-transactions-running-concurrently.html
http/tests/storage/callbacks-are-called-in-correct-context.html
storage/websql/statement-success-callback-isolated-world.html
js/dom/exceptions-thrown-in-callbacks.html
storage/websql/multiple-transactions.html
storage/websql/read-and-write-transactions-dont-run-together.html
storage/websql/multiple-databases-garbage-collection.html
storage/websql/statement-error-callback-isolated-world.html
storage/websql/change-version.html
storage/websql/statement-error-callback.html
storage/websql/multiple-transactions-on-different-handles.html
storage/websql/private-browsing-noread-nowrite.html

[Build Bot]

Created attachment 260194 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 260185 [details]
Patch

Attachment 260185 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/117923

New failing tests:
storage/websql/execute-sql-rowsAffected.html
storage/websql/transaction-success-callback-isolated-world.html
storage/websql/statement-error-callback.html
storage/websql/transaction-error-callback.html
storage/websql/alter-to-info-table.html
storage/websql/hash-change-with-xhr.html
storage/websql/null-callbacks.html
storage/websql/database-lock-after-reload.html
storage/websql/sql-error-codes.html
storage/websql/executesql-accepts-only-one-statement.html
storage/websql/test-authorizer.html
storage/websql/sql-data-types.html
storage/websql/success-callback.html
storage/websql/change-version-handle-reuse.html
storage/websql/read-transactions-running-concurrently.html
http/tests/storage/callbacks-are-called-in-correct-context.html
storage/websql/statement-success-callback-isolated-world.html
js/dom/exceptions-thrown-in-callbacks.html
storage/websql/multiple-transactions.html
storage/websql/read-and-write-transactions-dont-run-together.html
storage/websql/statement-error-callback-isolated-world.html
storage/websql/change-version.html
storage/websql/multiple-transactions-on-different-handles.html

[Build Bot]

Created attachment 260196 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Chris Dumez]

Created attachment 260200 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 260200 [details]
Patch

Clearing flags on attachment: 260200

Committed r189140: <http://trac.webkit.org/changeset/189140>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This broke bindings tests: https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/7914/steps/bindings-generation-tests/logs/stdio

I'm going to land current results as expected (and to once again wonder if having bindings tests is worth the effort).

[Chris Dumez]

(In reply to comment #10)
> This broke bindings tests:
> https://build.webkit.org/builders/
> Apple%20Yosemite%20Release%20WK2%20%28Tests%29/builds/7914/steps/bindings-
> generation-tests/logs/stdio
> 
> I'm going to land current results as expected (and to once again wonder if
> having bindings tests is worth the effort).

Sorry about that, I completely forgot. And thanks for fixing it for me.