Bug 31062 – [v8] Fix some exception propagation issues

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit Misc.

[Christian Plesner Hansen]

Fixed problem where we would attempt to re-throw a caught
exception while the TryCatch that caught the exception was still
in effect.  Changed to use the TryCatch::ReThrow.

[Christian Plesner Hansen]

Created attachment 42385 [details]
initial

[Eric Seidel (no email)]

Comment on attachment 42385 [details]
initial

Can't this be tested?  r- for lack of tests.

[Christian Plesner Hansen]

Created attachment 42467 [details]
addtest

[Christian Plesner Hansen]

Created attachment 42468 [details]
revert-unrelated

[Christian Plesner Hansen]

The case I really wanted to fix is covered by fast/workers/worker-constructor.html but all worker tests are disabled.  I've added a test of another case so we at least test something.

[David Levin]

(In reply to comment #5)
> The case I really wanted to fix is covered by
> fast/workers/worker-constructor.html but all worker tests are disabled. 

The worker tests are run in chromium as part of ui_tests (see TEST_F(WorkerTest, WorkerFastLayoutTests)). They can't run in test shell because of the multiprocess nature of workers in chromium.

[Christian Plesner Hansen]

Makes sense, that's also where I saw them fail.

Note that this is not a fix for incorrect behavior as such but for flakiness.  Previously we would get different behavior depending on whether you overflowed the stack in JS or C++ code, and we almost always did in JS code which made these tests pass.  With my change we also get the right behavior in C++.  I don't know how I would test the absence of flakiness beyond the test I've already added.

[Adam Barth]

Comment on attachment 42468 [details]
revert-unrelated

Ok.  Would you like this landed by commit-queue?

[Christian Plesner Hansen]

> Ok.  Would you like this landed by commit-queue?

Yes please -- is there an alternative (I'm not a committer)?

[WebKit Commit Bot]

Comment on attachment 42468 [details]
revert-unrelated

Rejecting patch 42468 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11601 test cases.
fast/dom/Document/exception-propagation.html -> new (results generated in /Users/eseidel/Projects/CommitQueue/LayoutTests/fast/dom/Document)

Exiting early after 1 failures. 5614 tests run.
93.49s total testing time

5613 test cases (99%) succeeded
1 test case (<1%) was new
1 test case (<1%) had stderr output

[Eric Seidel (no email)]

Comment on attachment 42468 [details]
revert-unrelated

Looks like this is missing expected results, which is why it failed.  run-webkit-tests will generate expected results for you.  Or perhaps you can convince a committer to generate them for you and add land this manually.

[Eric Seidel (no email)]

Created attachment 42978 [details]
expected results generated by the commit bot

[Eric Seidel (no email)]

We need a combined patch including the updated results.

[Eric Seidel (no email)]

Comment on attachment 42468 [details]
revert-unrelated

Marking this r-, since it can't be landed as is.

[Anders Carlsson]

V8 is gone from WebKit.