Bug 31062

Summary: [v8] Fix some exception propagation issues
Product: WebKit Reporter: Christian Plesner Hansen <christian.plesner.hansen>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, andersca, dglazkov, eric, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
initial
none
addtest
none
revert-unrelated
eric: review-, commit-queue: commit-queue-
expected results generated by the commit bot none

Christian Plesner Hansen
Reported 2009-11-03 08:23:39 PST
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.
Attachments
initial (5.62 KB, patch)
2009-11-03 08:25 PST, Christian Plesner Hansen
no flags
addtest (8.25 KB, patch)
2009-11-04 01:59 PST, Christian Plesner Hansen
no flags
revert-unrelated (7.50 KB, patch)
2009-11-04 02:02 PST, Christian Plesner Hansen
eric: review-
commit-queue: commit-queue-
expected results generated by the commit bot (219 bytes, text/plain)
2009-11-11 10:45 PST, Eric Seidel (no email)
no flags
Christian Plesner Hansen
Comment 1 2009-11-03 08:25:42 PST
Created attachment 42385 [details] initial
Eric Seidel (no email)
Comment 2 2009-11-03 12:07:33 PST
Comment on attachment 42385 [details] initial Can't this be tested? r- for lack of tests.
Christian Plesner Hansen
Comment 3 2009-11-04 01:59:58 PST
Created attachment 42467 [details] addtest
Christian Plesner Hansen
Comment 4 2009-11-04 02:02:07 PST
Created attachment 42468 [details] revert-unrelated
Christian Plesner Hansen
Comment 5 2009-11-04 02:04:14 PST
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
Comment 6 2009-11-11 00:21:18 PST
(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
Comment 7 2009-11-11 01:24:46 PST
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 8 2009-11-11 08:34:26 PST
Comment on attachment 42468 [details] revert-unrelated Ok. Would you like this landed by commit-queue?
Christian Plesner Hansen
Comment 9 2009-11-11 10:00:40 PST
> Ok. Would you like this landed by commit-queue? Yes please -- is there an alternative (I'm not a committer)?
WebKit Commit Bot
Comment 10 2009-11-11 10:26:03 PST
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 11 2009-11-11 10:29:14 PST
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)
Comment 12 2009-11-11 10:45:20 PST
Created attachment 42978 [details] expected results generated by the commit bot
Eric Seidel (no email)
Comment 13 2009-12-09 14:05:01 PST
We need a combined patch including the updated results.
Eric Seidel (no email)
Comment 14 2009-12-22 21:58:27 PST
Comment on attachment 42468 [details] revert-unrelated Marking this r-, since it can't be landed as is.
Anders Carlsson
Comment 15 2013-05-02 11:08:14 PDT
V8 is gone from WebKit.
Note You need to log in before you can comment on or make changes to this bug.