[webkit-dev] How to use ASSERT_NO_EXCEPTION

2011-12-15 Thread Darin Adler
Here’s my first draft of a note about ASSERT_NO_EXCEPTION. We might want to add 
this to the WebCore website; not sure.

Many core DOM functions in classes in WebCore serve a dual purpose.

They are used to implement DOM API that can be called from JavaScript. This 
often means that they will handle exceptional cases by raising a DOM exception. 
For example, if you call appendChild and pass a null pointer for the child, you 
will get a NOT_FOUND_ERR exception.

Those same functions are often used to implement the internals of the web 
engine. In those cases, they are called by callers who can guarantee none of 
the exceptional cases exist. Before ASSERT_NO_EXCEPTION, here’s how you would 
write a call like that:

ExceptionCode ec;
appendChild(newChild, ec);
ASSERT(!ec);

That’s pretty ugly, and we can do better. ASSERT_NO_EXCEPTION lets us do these 
two things:

#include ExceptionCodePlaceholder.h

appendChild(newChild, ASSERT_NO_EXCEPTION);

That’s pretty good, but this is even better:

appendChild(newChild);

To allow the last style, we add the ExceptionCodePlaceholder.h include to the 
header file and make ASSERT_NO_EXCEPTION the default argument for the 
ExceptionCode in the function’s declaration in the header. If you look at 
ContainerNode.h you can see that in use for appendChild.

Here are some rules of thumb for using this:

1) If there’s a DOM function where callers inside WebCore can easily 
guarantee that no exception will be raised, it’s recommended to add 
ASSERT_NO_EXCEPTION as a default value for the ExceptionCode argument.

2) If you need to call a function like this, first double check that you 
can indeed guarantee that no exception will occur, then either use 
ASSERT_NO_EXCEPTION directly and 

3) Do not use ASSERT_NO_EXCEPTION if the exception is possible. Be sure 
that you know why there is no exception possible before using this technique. 
In some cases, you may even need to add a comment to the source code explaining 
why no exception is possible.

-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] How to use ASSERT_NO_EXCEPTION

2011-12-15 Thread Joe Mason
 -Original Message-
 From: webkit-dev-boun...@lists.webkit.org [mailto:webkit-dev-
 boun...@lists.webkit.org] On Behalf Of Darin Adler
 Sent: Thursday, December 15, 2011 11:37 AM
 To: WebKit Development
 Subject: [webkit-dev] How to use ASSERT_NO_EXCEPTION
 
 Those same functions are often used to implement the internals of the
 web engine. In those cases, they are called by callers who can
 guarantee none of the exceptional cases exist. Before
 ASSERT_NO_EXCEPTION, here's how you would write a call like that:
 
 ExceptionCode ec;
 appendChild(newChild, ec);
 ASSERT(!ec);
 
 That's pretty ugly, and we can do better. ASSERT_NO_EXCEPTION lets us
 do these two things:
 
 #include ExceptionCodePlaceholder.h
 
 appendChild(newChild, ASSERT_NO_EXCEPTION);
 
 That's pretty good, but this is even better:
 
 appendChild(newChild);

I disagree that the last style is better.  Having the text 
ASSERT_NO_EXCEPTION in every function call makes it clear to all readers that 
there's a theoretical possibility of an exception here, and the author has made 
sure that it can't happen.  If the assertion is hidden in the default 
parameter, people who come to the code without reading this note (which will be 
very common) won't know the rules.

 Here are some rules of thumb for using this:
 
 1) If there's a DOM function where callers inside WebCore can
 easily guarantee that no exception will be raised, it's recommended to
 add ASSERT_NO_EXCEPTION as a default value for the ExceptionCode
 argument.
 
 2) If you need to call a function like this, first double check
 that you can indeed guarantee that no exception will occur, then either
 use ASSERT_NO_EXCEPTION directly and
 
 3) Do not use ASSERT_NO_EXCEPTION if the exception is possible. Be
 sure that you know why there is no exception possible before using this
 technique. In some cases, you may even need to add a comment to the
 source code explaining why no exception is possible.

Whenever there are rules of thumb like this, we need constant vigilance by the 
reviewers to make sure they're followed.  This is made easier if the coding 
style enforces the rules, so that places where they're ignored stand out.  
Having ASSERT_NO_EXCEPTION appearing in the code is a valuable reminder to 
authors and reviewers that they should be checking for exception-safety.  
Without it, I suspect we will often forget to check for this.

I think the first rule of thumb should be reversed, and explicit 
ASSERT_NO_EXCEPTION should be the norm.

Joe


-
This transmission (including any attachments) may contain confidential 
information, privileged material (including material protected by the 
solicitor-client or other applicable privileges), or constitute non-public 
information. Any use of this information by anyone other than the intended 
recipient is prohibited. If you have received this transmission in error, 
please immediately reply to the sender and delete this information from your 
system. Use, dissemination, distribution, or reproduction of this transmission 
by unintended recipients is not authorized and may be unlawful.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] How to use ASSERT_NO_EXCEPTION

2011-12-15 Thread Antti Koivisto
On Thu, Dec 15, 2011 at 8:36 AM, Darin Adler da...@apple.com wrote:

ExceptionCode ec;
appendChild(newChild, ec);
ASSERT(!ec);


Often code like this indicates misuse of DOM API functions for internal
purposes. This is inefficient (due to exception related checking and other
spec mandated behaviors) and architecturally bad (ties our internal data
structures to DOM way of presenting them, see stylesheet related classes).

I'm not sure it is a good idea to make these cases less visible.


   antti
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev