WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
143324
JavaScriptCore API should support type checking for Array and Date
https://bugs.webkit.org/show_bug.cgi?id=143324
Summary
JavaScriptCore API should support type checking for Array and Date
Geoffrey Garen
Reported
2015-04-01 17:04:59 PDT
JavaScriptCore API should support type checking for Array and Date
Attachments
Patch
(7.46 KB, patch)
2015-04-01 17:05 PDT
,
Geoffrey Garen
darin
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Geoffrey Garen
Comment 1
2015-04-01 17:05:58 PDT
Created
attachment 249953
[details]
Patch
WebKit Commit Bot
Comment 2
2015-04-01 17:08:20 PDT
Attachment 249953
[details]
did not pass style-queue: ERROR: Source/JavaScriptCore/API/JSValueRef.h:139: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/API/JSValueRef.h:148: The parameter name "value" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 3
2015-04-02 08:57:48 PDT
Comment on
attachment 249953
[details]
Patch These probably need availability macros.
Darin Adler
Comment 4
2015-04-02 09:09:35 PDT
Comment on
attachment 249953
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=249953&action=review
Should make these properties as suggested by the API experts here at Apple.
> Source/JavaScriptCore/API/JSValueRef.cpp:179 > + JSValue jsValue = toJS(exec, value); > + return jsValue.inherits(JSArray::info());
I’d like this better as a one-liner.
> Source/JavaScriptCore/API/JSValueRef.cpp:192 > + JSValue jsValue = toJS(exec, value); > + return jsValue.inherits(DateInstance::info());
Ditto.
> Source/JavaScriptCore/API/tests/testapi.c:1416 > + JSStringRelease(str);
Nice to fix the leak. I think the code below also leaks an exception object.
Geoffrey Garen
Comment 5
2015-04-02 11:04:46 PDT
> View in context: >
https://bugs.webkit.org/attachment.cgi?id=249953&action=review
> > Should make these properties as suggested by the API experts here at Apple.
I'll follow up with a patch to do that for all of these related is* methods, once the discussion is settled.
> > Source/JavaScriptCore/API/JSValueRef.cpp:179 > > + JSValue jsValue = toJS(exec, value); > > + return jsValue.inherits(JSArray::info()); > > I’d like this better as a one-liner. > > > Source/JavaScriptCore/API/JSValueRef.cpp:192 > > + JSValue jsValue = toJS(exec, value); > > + return jsValue.inherits(DateInstance::info()); > > Ditto.
OK.
> Nice to fix the leak. I think the code below also leaks an exception object.
The exception local variable is a JSValueRef, so it's garbage collected.
Geoffrey Garen
Comment 6
2015-04-02 17:10:29 PDT
Committed
r182297
: <
http://trac.webkit.org/changeset/182297
>
Joseph Pecoraro
Comment 7
2015-04-04 11:01:52 PDT
Comment on
attachment 249953
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=249953&action=review
> Source/JavaScriptCore/API/JSValue.h:422 > +- (BOOL)isArray;
Should these have NS_AVAILABLE macros?
> Source/JavaScriptCore/API/JSValueRef.h:139 > +JS_EXPORT bool JSValueIsArray(JSContextRef ctx, JSValueRef value);
Should these have CF_AVAILABLE macros?
Joseph Pecoraro
Comment 8
2015-04-04 11:04:24 PDT
(In reply to
comment #7
)
> Comment on
attachment 249953
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=249953&action=review
> > > Source/JavaScriptCore/API/JSValue.h:422 > > +- (BOOL)isArray; > > Should these have NS_AVAILABLE macros? > > > Source/JavaScriptCore/API/JSValueRef.h:139 > > +JS_EXPORT bool JSValueIsArray(JSContextRef ctx, JSValueRef value); > > Should these have CF_AVAILABLE macros?
Ahh, I see when this landed it had them!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug