Bug 111487 - Objective-C API: JSValue should implement init and return nil in exceptional cases
Summary: Objective-C API: JSValue should implement init and return nil in exceptional ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-05 15:05 PST by Mark Hahnenberg
Modified: 2013-03-05 16:18 PST (History)
1 user (show)

See Also:


Attachments
patch (1.51 KB, patch)
2013-03-05 15:07 PST, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
return nil (1.57 KB, patch)
2013-03-05 15:20 PST, Mark Hahnenberg
mhahnenberg: review-
Details | Formatted Diff | Diff
updated change log (1.55 KB, patch)
2013-03-05 15:24 PST, Mark Hahnenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2013-03-05 15:05:37 PST
In keeping with good Objective-C practice, JSValue should override init and throw an exception since there's not really a way to put the returned object in a coherent state that won't crash at some point later in time. Better to fail fast.
Comment 1 Mark Hahnenberg 2013-03-05 15:07:49 PST
Created attachment 191576 [details]
patch
Comment 2 Mark Hahnenberg 2013-03-05 15:09:11 PST
<rdar://problem/13351901>
Comment 3 Darin Adler 2013-03-05 15:12:54 PST
Comment on attachment 191576 [details]
patch

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

> Source/JavaScriptCore/API/JSValue.mm:955
> +    @throw [NSException exceptionWithName:@"InvalidMessageException"
> +                        reason:@"JSValue cannot safely support init."
> +                        userInfo:nil];

Why throw? Just return nil. And the initWithValue method should also return nil when the arguments are nil. That’s how Cocoa init functions normally work. Unless I’m missing some compelling argument for throwing exceptions instead.

Please don’t align the lines like this. WebKit coding style says just indent 4 spaces.
Comment 4 Mark Hahnenberg 2013-03-05 15:20:00 PST
Created attachment 191579 [details]
return nil
Comment 5 Mark Hahnenberg 2013-03-05 15:21:10 PST
Comment on attachment 191579 [details]
return nil

Need to update ChangeLog.
Comment 6 Mark Hahnenberg 2013-03-05 15:24:19 PST
Created attachment 191582 [details]
updated change log
Comment 7 WebKit Review Bot 2013-03-05 16:18:37 PST
Comment on attachment 191582 [details]
updated change log

Clearing flags on attachment: 191582

Committed r144843: <http://trac.webkit.org/changeset/144843>
Comment 8 WebKit Review Bot 2013-03-05 16:18:40 PST
All reviewed patches have been landed.  Closing bug.