Bug 83850

Summary: JSDocument::setLocation does too much bare-handed lifting
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Adam Barth 2012-04-12 18:50:07 PDT
JSDocument::setLocation does too much bare-handed lifting
Comment 1 Adam Barth 2012-04-12 18:51:51 PDT
Created attachment 137022 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-13 01:57:52 PDT
Comment on attachment 137022 [details]
Patch

Clearing flags on attachment: 137022

Committed r114098: <http://trac.webkit.org/changeset/114098>
Comment 3 WebKit Review Bot 2012-04-13 01:57:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Darin Adler 2012-04-13 09:20:03 PDT
Comment on attachment 137022 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review

> Source/WebCore/bindings/js/JSDocumentCustom.cpp:74
> +    UString locationString = value.toString(exec)->value(exec);
> +    if (exec->hadException())
> +        return;

Is there no exception possible in toString? If there is, I think we’d need two separate return statements.
Comment 5 Darin Adler 2012-04-13 09:20:49 PDT
Comment on attachment 137022 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review

>> Source/WebCore/bindings/js/JSDocumentCustom.cpp:74
>> +        return;
> 
> Is there no exception possible in toString? If there is, I think we’d need two separate return statements.

Or maybe toString can have an exception, but value can't.
Comment 6 Adam Barth 2012-04-13 09:29:07 PDT
(In reply to comment #5)
> (From update of attachment 137022 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137022&action=review
> 
> >> Source/WebCore/bindings/js/JSDocumentCustom.cpp:74
> >> +        return;
> > 
> > Is there no exception possible in toString? If there is, I think we’d need two separate return statements.
> 
> Or maybe toString can have an exception, but value can't.

I believe that's the case, but I'll verify by reading the code.