Throwing in an IndexedDB error or success event should lead to the transaction aborting
Created attachment 82054 [details] Patch
please review
Created attachment 82057 [details] Patch
Comment on attachment 82057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82057&action=review > Source/WebCore/dom/EventTarget.h:154 > + virtual void uncaughtExceptionInEventHandler(); Is there any way to avoid having this function show up all the way down in EventTarget? Alternatively, is it likely other EventTargets will find this method useful?
Comment on attachment 82057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82057&action=review >> Source/WebCore/dom/EventTarget.h:154 >> + virtual void uncaughtExceptionInEventHandler(); > > Is there any way to avoid having this function show up all the way down in EventTarget? Alternatively, is it likely other EventTargets will find this method useful? is there a better place for it? I don't think this is a great one, but I didn't see any other better ones either. I'm not aware of any other API that would make use of it, but it is possible one would in the future. Are you concerned about adding yet another vtable entry? I actually can't think of any way around putting some entry in EventTarget (even if it's something to say that it can be static casted to some other type that would have the method call on it).
Comment on attachment 82057 [details] Patch It's just my little peeve that there are a fair number of these one-off functions in base classes that are empty or hardcoded and only overridden in one subclass. I can't think of anything better, but I figured I should at least ask :)
Committed r78655: <http://trac.webkit.org/changeset/78655>
Comment on attachment 82057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82057&action=review > Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:155 > + innerTryCatch.Reset(); Just FYI: you don't need to Reset innerTryCatch---it catches all the exceptions by default (you need explicit Rethrow to propagate exception caught).
Patch reverted.
Mikhail: in the future, you should post some context in the bug when you're reverting. I now have absolutely no idea what the failures are and will likely need to re-land just to see what went wrong.
(In reply to comment #10) > Mikhail: in the future, you should post some context in the bug when you're reverting. I now have absolutely no idea what the failures are and will likely need to re-land just to see what went wrong. Never mind....the results were still available. Please do post a few links when reverting stuff though. Sorry for the trouble the patch caused.
Mads/Anton: Can you please take a look at this patch and see if you can tell what's wrong with it? It's causing subtle issues in a lot of layout tests. Most look something like this though: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Finspector-support%2Funcaught-dom3-exception.html&showExpectations=true&group=%40ToT%20-%20chromium.org
Jeremy, I'm sorry. Next time I will provide links to logs.
I'll have a look tomorrow.
Comment on attachment 82057 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82057&action=review > Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:151 > + v8::TryCatch innerTryCatch; I think this is your behavioral change. This inner TryCatch is not verbose which means that the errors are not reported. Does it help to put in innerTryCatch.SetVerbose(true).
(In reply to comment #15) > (From update of attachment 82057 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82057&action=review > > > Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:151 > > + v8::TryCatch innerTryCatch; > > I think this is your behavioral change. This inner TryCatch is not verbose which means that the errors are not reported. Does it help to put in innerTryCatch.SetVerbose(true). Followed up on chat. Turns out I don't even need the inner one and everything should just work if I check whether it's been caught before the reset. Clearly I really confused myself on this one. :-)
Created attachment 82849 [details] Patch
Committed r78908: <http://trac.webkit.org/changeset/78908>