A promise returning function is not expected to throw exception. In case of argument mismatch or other cases, the exceptions should be transmitted to the application by rejecting the promise. The only case the function should throw is if the function is called on a this value which does not have the right type. The binding generator should handle these cases properly.
Created attachment 255017 [details] Patch
Comment on attachment 255017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255017&action=review > Source/WebCore/bindings/js/JSDOMPromise.h:41 > + static void rejectPromiseWithException(JSC::ExecState*, JSDOMGlobalObject*, JSC::JSPromiseDeferred*); I think all the arguments should be references. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2799 > + push(@implContent, "static inline EncodedJSValue ${functionName}Promise(ExecState*, " . $className . "*, JSPromiseDeferred*);\n"); > + push(@implContent, "EncodedJSValue JSC_HOST_CALL ${functionName}(ExecState* exec)\n"); > + push(@implContent, "{\n"); > + > + GenerateFunctionCastedThis($interface, $interfaceName, $className, $function); > + > + push(@implContent, " JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, castedThis->globalObject());\n"); > + push(@implContent, " ${functionName}Promise(exec, castedThis, promiseDeferred);\n"); > + > + push(@implContent, " if (exec->hadException())\n"); > + push(@implContent, " DeferredWrapper::rejectPromiseWithException(exec, castedThis->globalObject(), promiseDeferred);\n"); > + push(@implContent, " ASSERT(!exec->hadException());\n"); > + push(@implContent, " return JSValue::encode(promiseDeferred->promise());\n"); > + push(@implContent, "}\n"); Normally we try to make the generated code as small as possible, using normal C++ programming for everything we can. We’ve strayed from that a bit, but it’s important to not load the total amount of code this generates. Can we use a helper function or function template to make he actual generated code as small as possible? It seems that once we call xxxPromise we do nothing more that depends on the arguments. I’d like to see this entire thing be a one line invocation of a function template that is in JSDOMPromise.h.
Created attachment 255109 [details] Patch for landing
(In reply to comment #2) > Comment on attachment 255017 [details] > Patch > > nothing more that depends on the arguments. I’d like to see this entire > thing be a one line invocation of a function template that is in > JSDOMPromise.h. Thanks for the idea. Latest patch tries to integrate some of this. Let me know if you would like further adjustment. If not, I will cq+ it tomorrow.
Comment on attachment 255109 [details] Patch for landing Clearing flags on attachment: 255109 Committed r185739: <http://trac.webkit.org/changeset/185739>
All reviewed patches have been landed. Closing bug.
Comment on attachment 255109 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=255109&action=review Looks really good. > Source/WebCore/bindings/js/JSDOMPromise.h:62 > +template<class JSClassName> The type here is the JavaScript class, not the JavaScript class name. I suggest the name WrapperClass here instead of JSClassName. Also, we almost always use typename rather than class in templates, although I’m not entirely sure why. > Source/WebCore/bindings/js/JSDOMPromise.h:63 > +inline JSC::JSValue callPromiseFunction(JSC::ExecState& state, JSClassName& jsObject, JSC::EncodedJSValue promiseFunction(JSC::ExecState*, JSClassName*, JSC::JSPromiseDeferred*)) A typical name used in bindings for what we are calling jsObject here is “wrapper”