Summary: RELEASE_ASSERT(!thisObject) in ObjCCallbackFunctionImpl::call when calling JSExport ObjC Constructor without operator new Test: /* * shell> xcrun clang -framework Foundation -framework JavaScriptCore construct.m */ #import <Foundation/Foundation.h> #import <JavaScriptCore/JavaScriptCore.h> @protocol JSFooExports <JSExport> - (instancetype)initWithName:(NSString *)name; @end @interface JSFoo : NSObject <JSFooExports> - (instancetype)initWithName:(NSString *)name; @end @implementation JSFoo - (instancetype)initWithName:(NSString *)name { self = [super init]; if (!self) return nil; NSLog(@">>> initWithName: %@", name); return self; } @end int main() { JSContext *ctx = [[JSContext alloc] init]; ctx[@"JSFoo"] = [JSFoo class]; [ctx evaluateScript:@"JSFoo()"]; return 0; } Steps to Reproduce: 1. Compile + Run test => RELEASE_ASSERT crash immediately Crash: Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BREAKPOINT (SIGTRAP) Exception Codes: 0x0000000000000002, 0x0000000000000000 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010bdf0e6e JSC::ObjCCallbackFunctionImpl::call(JSContext*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 654 (ObjCCallbackFunction.mm:557) 1 com.apple.JavaScriptCore 0x000000010bdf0926 JSC::objCCallbackFunctionCallAsFunction(OpaqueJSContext const*, OpaqueJSValue*, OpaqueJSValue*, unsigned long, OpaqueJSValue const* const*, OpaqueJSValue const**) + 262 (ObjCCallbackFunction.mm:462) 2 com.apple.JavaScriptCore 0x000000010bdf1b7d long long JSC::APICallbackFunction::call<JSC::ObjCCallbackFunction>(JSC::ExecState*) + 573 (APICallbackFunction.h:61) 3 com.apple.JavaScriptCore 0x000000010bdb15b3 JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*) + 595 (LLIntSlowPaths.cpp:1203) 4 com.apple.JavaScriptCore 0x000000010bdb87ce llint_entry + 24803 5 com.apple.JavaScriptCore 0x000000010bdb2508 vmEntryToJavaScript + 299 6 com.apple.JavaScriptCore 0x000000010bc0c2ee JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 158 (JITCode.cpp:81) 7 com.apple.JavaScriptCore 0x000000010bb65cb5 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) + 17125 (Interpreter.cpp:961) 8 com.apple.JavaScriptCore 0x000000010b7d0ea5 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) + 469 (Completion.cpp:107) 9 com.apple.JavaScriptCore 0x000000010bc61d98 JSEvaluateScript + 456 (NakedPtr.h:54) 10 com.apple.JavaScriptCore 0x000000010bc73639 -[JSContext evaluateScript:withSourceURL:] + 105 (JSContext.mm:102) 11 a.out 0x000000010b56ee9b main + 171 12 libdyld.dylib 0x00007fff918095ad start + 1 ...
JavaScript Classes disallow calling the constructor without `new`. > class Foo {}; Foo() < Exception: TypeError: Cannot call a class constructor Maybe the ObjC bridge should do the same.
JS exception seems like much better behavior than RELEASE_ASSERT.
Created attachment 282842 [details] [PATCH] Proposed Fix
Attachment 282842 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/tests/testapi.mm:1352: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
The new testapi output looks like: > testapi[41761:43777520] TEST: "ObjC Constructor without 'new' pre": PASSED > testapi[41761:43777520] TypeError: Cannot call a class constructor without |new| > testapi[41761:43777520] TEST: "ObjC Constructor without 'new' post": PASSED I went ahead and updated the JavaScript error message for ES6 classes. Other browsers hinted that this was missing 'new', and I think we should do the same: Safari Before: TypeError: Cannot call a class constructor Safari After: TypeError: Cannot call a class constructor without |new| Chrome: TypeError: Class constructor Foo cannot be invoked without 'new'(…) Firefox: TypeError: Cannot call a class constructor without |new|
Comment on attachment 282842 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=282842&action=review r=me > Source/JavaScriptCore/API/tests/testapi.mm:1337 > + Please remove this empty line.
Comment on attachment 282842 [details] [PATCH] Proposed Fix Clearing flags on attachment: 282842 Committed r202846: <http://trac.webkit.org/changeset/202846>
All reviewed patches have been landed. Closing bug.