fix(ios): reduced TitaniumKit build warnings & fixed TiBlob crash#14298
fix(ios): reduced TitaniumKit build warnings & fixed TiBlob crash#14298hbugdoll wants to merge 4 commits into
Conversation
| JSContext *context = JSContext.currentContext; | ||
| JSValueRef *exception; | ||
| JSObjectRef arrayBuffer = JSObjectMakeArrayBufferWithBytesNoCopy(context.JSGlobalContextRef, arrayBytes, len, jsArrayBufferFreeDeallocator, nil, exception); | ||
| JSObjectRef arrayBuffer = JSObjectMakeArrayBufferWithBytesNoCopy(context.JSGlobalContextRef, arrayBytes, len, jsArrayBufferFreeDeallocator, nil, NULL); |
There was a problem hiding this comment.
Why passing NULL here instead of initializing the exception to NULL and passing the reference like you did below?
There was a problem hiding this comment.
Because exception is not evaluated resp. used further here.
There was a problem hiding this comment.
I think it should though - any exception handling can be useful
There was a problem hiding this comment.
But how?
Handling it like in arrayBuffer via KrollPromise then it would make toArrayBuffer and arrayBuffer redundant.
Handling it with the JavaScriptCore API (I am not familiar with it), ChatGPT suggests the following:
if (exception) {
JSStringRef exceptionStr = JSValueToStringCopy(context.JSGlobalContextRef, exception, NULL);
NSString *message = (__bridge_transfer NSString *)JSStringCopyCFString(kCFAllocatorDefault, exceptionStr);
JSStringRelease(exceptionStr);
// e.g. handle message
free(arrayBytes);
return nil;
}
There was a problem hiding this comment.
Mhh, we are there even both then, if the only difference is the exception?
There was a problem hiding this comment.
Oh, there is still a difference – a different context may be used (JSContext.currentContext vs. [self currentContext]).
There was a problem hiding this comment.
I've done the following changes:
- use of
[self currentContext]instead ofJSContext.currentContextintoArrayBufferto make it thread-safe - added exception handler in
toArrayBuffer - added also
free(arrayBytes)inarrayBufferwhen the exception is raised
There was a problem hiding this comment.
What do you think about the latest commit c0dd252?
It would be cool if the iOS test would run until the end again...
Related to #14290. Edit: Also fixes #14306.
Description
Removed over 40 warnings during TitaniumKit build process:
+resourceBasedURL:baseURL:iniphone/TitaniumKit/TitaniumKit/Sources/API/ScriptModule.m-JSValueInContext:iniphone/TitaniumKit/TitaniumKit/Sources/API/KrollModule.m-applyProperties:iniphone/TitaniumKit/TitaniumKit/Sources/API/TiProxy.hJSValueRef *vs.JSValueRef _Null_unspecifiediniphone/TitaniumKit/TitaniumKit/Sources/API/TiBlob.mJSValueRef, which is already a pointer (Edit: crash relevant)JSValueRef[self currentContext]instead ofJSContext.currentContextintoArrayBufferto make it thread-safetoArrayBufferfree(arrayBytes)inarrayBufferwhen the exception is raisedCAShapeLayer *vs.CALayer *iniphone/TitaniumKit/TitaniumKit/Sources/API/TiUIView.mTiHostiniphone/TitaniumKit/TitaniumKit/Sources/API/ScriptModule.mNSUncaughtExceptionHandler *iniphone/TitaniumKit/TitaniumKit/Sources/API/TiExceptionHandler.mTiBlobTypeSystemImageiniphone/TitaniumKit/TitaniumKit/Sources/API/TiBlob.mdefaultcase