fix(runtime): key wrapper state by the Proxy target in SetValue/DeleteValue

GetValue resolves through proxies while SetValue and DeleteValue addressed
the object they were handed, so __releaseNativeCounterpart(proxy) deleted
the target's wrapper and then cleared the slot on the Proxy, leaving the
target's internal field dangling. All three now resolve to the same target.

ResolveProxyReceiver accepts a function target only when it carries an
ObjCClass wrapper; a proxied plain function is a TypeError instead of a
static call on the metadata class.

Author: edusperoni

NativeScript/runtime/Helpers.h

@@ -280,8 +280,9 @@ v8::Local<v8::Value> UnwrapProxyOrThrow(v8::Isolate* isolate, v8::Local<v8::Valu
 v8::Local<v8::Context> GetCreationContextOrCurrent(v8::Isolate* isolate,
                                                    const v8::Local<v8::Object>& obj);
 
+// Set/Get/DeleteValue all resolve through proxies: wrapper state is keyed by
+// the Proxy target, so a proxied wrapper yields its target's wrapper.
 void SetValue(v8::Isolate* isolate, const v8::Local<v8::Object>& obj, BaseDataWrapper* value);
-// Resolves through proxies: a proxied wrapper yields its target's wrapper.
 BaseDataWrapper* GetValue(v8::Isolate* isolate, const v8::Local<v8::Value>& val);
 
 // What happens when JS touches a wrapper whose native counterpart has already

NativeScript/runtime/Helpers.mm

@@ -265,11 +265,19 @@ throw NativeScriptException(
   return false;
 }
 
-void tns::SetValue(Isolate* isolate, const Local<Object>& obj, BaseDataWrapper* value) {
-  if (obj.IsEmpty() || obj->IsNullOrUndefined()) {
+void tns::SetValue(Isolate* isolate, const Local<Object>& val, BaseDataWrapper* value) {
+  if (val.IsEmpty() || val->IsNullOrUndefined()) {
     return;
   }
 
+  // Wrapper state lives on the Proxy target so that it is found by the same
+  // identity GetValue resolves to.
+  Local<Value> target = tns::UnwrapProxy(val);
+  if (target.IsEmpty()) {
+    return;
+  }
+  Local<Object> obj = target.As<Object>();
+
   Local<External> ext = External::New(isolate, value, v8::kExternalPointerTypeTagDefault);
 
   if (obj->InternalFieldCount() > 0) {
@@ -531,11 +539,12 @@ void WriteDebugLine(tns::LogCategory category, const char* message) {
 }
 
 void tns::DeleteValue(Isolate* isolate, const Local<Value>& val) {
-  if (val.IsEmpty() || val->IsNullOrUndefined() || !val->IsObject()) {
+  Local<Value> target = tns::UnwrapProxy(val);
+  if (target.IsEmpty() || target->IsNullOrUndefined() || !target->IsObject()) {
     return;
   }
 
-  Local<Object> obj = val.As<Object>();
+  Local<Object> obj = target.As<Object>();
   if (obj->InternalFieldCount() > 0) {
     obj->SetInternalField(0, v8::Undefined(isolate));
     return;

NativeScript/runtime/MetadataBuilder.mm

@@ -34,11 +34,14 @@ bool ResolveProxyReceiver(Isolate* isolate, Local<Object>& receiver, const char*
   const char* reason = nullptr;
   if (target.IsEmpty()) {
     reason = "on a revoked Proxy";
-  } else if (!(allowClass && target->IsFunction())) {
-    BaseDataWrapper* wrapper =
-        target.As<Object>()->InternalFieldCount() > 0 ? tns::GetValue(isolate, target) : nullptr;
-    if (wrapper == nullptr || (wrapper->Type() != WrapperType::ObjCObject &&
-                               wrapper->Type() != WrapperType::ObjCAllocObject)) {
+  } else {
+    BaseDataWrapper* wrapper = tns::GetValue(isolate, target);
+    bool isClass = allowClass && target->IsFunction() && wrapper != nullptr &&
+                   wrapper->Type() == WrapperType::ObjCClass;
+    bool isInstance = target.As<Object>()->InternalFieldCount() > 0 && wrapper != nullptr &&
+                      (wrapper->Type() == WrapperType::ObjCObject ||
+                       wrapper->Type() == WrapperType::ObjCAllocObject);
+    if (!isClass && !isInstance) {
       reason = "on a Proxy whose target is not a native object";
     }
   }

NativeScript/runtime/ObjectManager.mm

@@ -349,7 +349,9 @@ void DisposeHandle(v8::Isolate* isolate,
     return;
   }
 
-  Local<Value> value = info[0];
+  // The lookup, the Instances key and the final SetValue must all address the
+  // same object, so a Proxy is resolved once here.
+  Local<Value> value = tns::UnwrapProxy(info[0]);
   BaseDataWrapper* wrapper = tns::GetValue(isolate, value);
 
   if (wrapper == nullptr) {

TestRunner/app/tests/ProxyReceiverTests.js

@@ -140,6 +140,23 @@ describe(module.id, function () {
         expect(function () {
             findAccessor(NSOperation.prototype, "name").set.call(proxy, "x");
         }).toThrowError(TypeError, /not a native object/);
+        expect(function () {
+            NSString.stringWithString.call(new Proxy(function () {}, {}), "x");
+        }).toThrowError(TypeError, /not a native object/);
+    });
+
+    it("releases the native counterpart of the proxy target", function () {
+        var target = NSMutableString.alloc().init();
+        var proxy = new Proxy(target, {});
+
+        __releaseNativeCounterpart(proxy);
+
+        expect(function () {
+            __releaseNativeCounterpart(target);
+        }).toThrowError(/not a native wrapper/);
+        expect(function () {
+            __releaseNativeCounterpart(proxy);
+        }).toThrowError(/not a native wrapper/);
     });
 
     it("consults proxy traps for the lookup and calls the target natively", function () {