Bug 38230 - When argument conversion in the bindings throws an exception, we plow ahead
Summary: When argument conversion in the bindings throws an exception, we plow ahead
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-27 17:03 PDT by Adam Barth
Modified: 2016-12-22 09:29 PST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-04-27 17:03:12 PDT
When argument conversion in the bindings throws an exception, we plow ahead.  Instead, we should notice that an exception has occurred and return early so we don't cause any side effects.  This is a widespread, but minor, problem in the bindings.
Comment 1 Darin Adler 2010-04-28 10:50:34 PDT
There are two aspects here. One is which exception we end up raising when there is another exception later. Another is observable side effects of “plowing ahead”. In cases where there is no observable side effect there is no harm in “plowing ahead”.
Comment 2 Adam Barth 2010-04-28 12:18:43 PDT
I would guess that there are almost always observable side-effects.  For example, you might convert a later argument to a string, re-entering JavaScript.
Comment 3 Darin Adler 2010-04-28 16:23:59 PDT
(In reply to comment #2)
> I would guess that there are almost always observable side-effects.  For
> example, you might convert a later argument to a string, re-entering
> JavaScript.

Exactly. Those are the things we have to consider when making test cases.

I suspect making the test cases will make it clear when the “almost” in “almost always” happens.
Comment 4 Adam Barth 2010-04-28 18:32:48 PDT
> Exactly. Those are the things we have to consider when making test cases.

What testing plan do you have in mind?  We can certainly test a few representative APIs, and we'll have a test for the code generator output.
Comment 5 Darin Adler 2010-04-29 17:45:05 PDT
(In reply to comment #4)
> What testing plan do you have in mind?  We can certainly test a few
> representative APIs, and we'll have a test for the code generator output.

I was thinking we should make a handwritten test for each function to see how it handles exceptions during argument evaluation.

The test fast/dom/wrapper-classes.html shows a technique for getting a test object of each class. Once we have an object of that class we can test each function that has arguments. For example, I'd start with HTMLElement.setId, making sure that if the conversion of its argument to a string raises an exception, the id attribute of the object isn't changed.

We could simplify by testing only one of a given class of function. For example, we could test only one reflected attribute. Maybe if you take that to the logical extreme then we need only a small number of tests!
Comment 6 Darin Adler 2016-12-22 09:29:36 PST
I suspect the actual problem described here is almost completely fixed now--automatically generated bindings, at least, exit when exceptions happen and have for some time--but I don’t think we have good test coverage.