[webkit-reviews] review denied: [Bug 51855] Extract ThreadFunctionInvocation into separate file and share between Apple Windows and Android : [Attachment 77861] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 51855: Extract ThreadFunctionInvocation into separate file and share
between Apple Windows and Android
https://bugs.webkit.org/show_bug.cgi?id=51855

Attachment 77861: Patch
https://bugs.webkit.org/attachment.cgi?id=77861&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=77861&action=review

> Source/JavaScriptCore/wtf/ThreadingPthreads.cpp:-155
> -    ThreadData* data = static_cast<ThreadData*>(arg);
> +    ThreadFunctionInvocation invocation =
*static_cast<ThreadFunctionInvocation*>(arg);
> +    delete static_cast<ThreadFunctionInvocation*>(arg);
> +
>      JavaVM* vm = JSC::Bindings::getJavaVM();
>      JNIEnv* env;
>      void* ret = 0;
>      if (vm->AttachCurrentThread(&env, 0) == JNI_OK) {
> -	   ret = data->entryPoint(data->arg);
> +	   ret = invocation.function(invocation.data);
>	   vm->DetachCurrentThread();
>      }
> -    delete data;

This is a little bizarre. How about just adopting arg into an
OwnPtr<ThreadFunctionInvocation> and using that to invoke the function?

> Source/JavaScriptCore/wtf/ThreadingPthreads.cpp:162
> +    ThreadFunctionInvocation* invocation = new
ThreadFunctionInvocation(entryPoint, data); // Deallocated in
runThreadWithRegistration() if a thread is created.
> +
> +    if (pthread_create(&threadHandle, 0, runThreadWithRegistration,
invocation)) {

It would be nicer to use adoptPtr/leakPtr here.