[webkit-reviews] Fix to Recent KJS_CHECK_EXCEPTION Change

[Mark Rowe]

On 10/06/2005, at 15:51 , Darin Adler wrote:

> Looks great!
>
> Three comments:
>
>     1) These changes need tests. It should be straightforward to  
> make tests that throw exceptions and then have exception handlers  
> that check the file and line number on the caught exceptions. We  
> really want to write tests whenever we fix a bug unless it's not  
> possible.
>
>     2) This macro getting bigger and it's used in a lot of places.  
> Since the new code is all for the exception case, which is not  
> particularly speed-critical, I'd like to see as much as possible of  
> the code put into a protected member function of the KJS::Node  
> class, and called from the macro.
>
>     3) KJS_CHECKEXCEPTION, KJS_CHECKEXCEPTIONREFERENCE, and  
> KJS_CHECKEXCEPTIONLIST all need the same fix.

I've attached an updated patch which integrates all of your  
suggestions, and a layout test that verifies that a line number +  
source URL is included on exceptions that pass through each of the  
four macros and ensures that outer nodes do not overwrite the details  
that inner nodes have set on the exception object.

> Thanks very much for tackling this important bug. I don't mean to  
> be a pain with all these review comments; keep it up!

Not a worry -- eventually perfection will be achieved ;-)

Regards,

--
Mark Rowe
<http://bdash.net.nz/>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: javascriptcore-css-exception-detail-p5.patch
Type: application/octet-stream
Size: 3060 bytes
Desc: not available
Url : http://lists.macosforge.org/pipermail/webkit-reviews/attachments/20050610/353a80f0/javascriptcore-css-exception-detail-p5.obj
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.macosforge.org/pipermail/webkit-reviews/attachments/20050610/353a80f0/exception-linenums.html
-------------- next part --------------
layer at (0,0) size 800x600
  RenderCanvas at (0,0) size 800x600
layer at (0,0) size 800x600
  RenderBlock {HTML} at (0,0) size 800x600
    RenderBody {BODY} at (8,8) size 784x579
      RenderBlock {PRE} at (0,0) size 784x75
        RenderText {TEXT} at (0,0) size 312x75
          text run at (0,0) width 312: "Exception at line 25 with sourceURL set"
          text run at (0,15) width 312: "Exception at line 34 with sourceURL set"
          text run at (0,30) width 312: "Exception at line 43 with sourceURL set"
          text run at (0,45) width 312: "Exception at line 52 with sourceURL set"
          text run at (0,60) width 304: "Exception at line 5 with sourceURL set"