Bug 38230 – When argument conversion in the bindings throws an exception, we plow ahead

• Status: ASSIGNED
• Component: WebCore JavaScript

[Adam Barth]

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.

[Darin Adler]

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”.

[Adam Barth]

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.

[Darin Adler]

(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.

[Adam Barth]

> 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.

[Darin Adler]

(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!

[Darin Adler]

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.