|Summary:||DOMCoreException needs NoStaticTables modifier|
|Product:||WebKit||Reporter:||Eric U. <ericu>|
|Component:||DOM||Assignee:||Eric U. <ericu>|
|Severity:||Normal||CC:||ap, commit-queue, dimich, ian|
|Version:||528+ (Nightly build)|
Description Eric U. 2010-03-22 12:34:32 PDT
Without it, its fields are inaccessible from all but one thread. If you've got workers and the main thread both throwing exceptions, you'll have issues--methods of the object are inaccessible, and you can at least crash a debug build by enumerating them. So far I've managed to see worker exceptions .toString returning [object DOMException] instead of the custom string; I'm not sure if anything worse can happen. It's a one-line fix; I'll have a patch up shortly.
Comment 1 Alexey Proskuryakov 2010-03-22 17:25:02 PDT
It's so wrong that we need to make anything with "DOM" in its name supported in workers. I guess the evil started with XMLHttpRequest and other non-DOM APIs using exceptions that didn't belong to them.
Comment 3 Eric U. 2010-03-22 17:49:59 PDT
(In reply to comment #1) > It's so wrong that we need to make anything with "DOM" in its name supported in > workers. I guess the evil started with XMLHttpRequest and other non-DOM APIs > using exceptions that didn't belong to them. No argument here; I think we're stuck with the terminology now.
Comment 4 Dmitry Titov 2010-03-23 13:43:15 PDT
(In reply to comment #1) > It's so wrong that we need to make anything with "DOM" in its name supported in > workers. I guess the evil started with XMLHttpRequest and other non-DOM APIs > using exceptions that didn't belong to them. DOMString (as it is known in IDL files) probably was the first claw (or victim) of the evil. Alexey, do you suggest some renaming or a new class for the exception used in Workers? The patch otherwise looks ok to me.
Comment 5 Alexey Proskuryakov 2010-03-23 13:55:25 PDT
Comment on attachment 51373 [details] Patch Renaming may make sense - DOMCoreException is just an internal name for WebKit, and I suspect that it was chosen simply to avoid file name conflicts. On the other hand, it's still DOMException, so hiding that fact under a different name would be confusing. r=me with the understanding that this will be covered by regression tests soon.
Comment 6 WebKit Commit Bot 2010-03-23 14:28:45 PDT
Comment on attachment 51373 [details] Patch Rejecting patch 51373 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 12532 test cases. fast/workers/worker-context-multi-port.html -> failed Exiting early after 1 failures. 8830 tests run. 153.25s total testing time 8829 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 5 test cases (<1%) had stderr output Full output: http://webkit-commit-queue.appspot.com/results/1206001
Comment 7 Dmitry Titov 2010-03-23 16:31:59 PDT
The test has this in its expected results: PASS posting a null port did throw: [object DOMException] Needs new expected results it seems.
Comment 9 Eric U. 2010-03-24 11:00:51 PDT
Comment on attachment 51525 [details] Patch Fixed the test expectation not to rely on the old broken behavior. Sorry about that!
Comment 10 WebKit Commit Bot 2010-03-24 15:22:47 PDT
Comment on attachment 51525 [details] Patch Clearing flags on attachment: 51525 Committed r56462: <http://trac.webkit.org/changeset/56462>
Comment 11 WebKit Commit Bot 2010-03-24 15:22:52 PDT
All reviewed patches have been landed. Closing bug.