review comments

Author: MikeHolman

lib/Jsrt/Core/JsrtCore.cpp

@@ -735,10 +735,10 @@ CHAKRA_API JsCloneObject(_In_ JsValueRef source, _Out_ JsValueRef* newObject)
     VALIDATE_JSREF(source);
 
     return ContextAPINoScriptWrapper([&](Js::ScriptContext* scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode {
-        Js::JavascriptProxy* proxy = Js::JavascriptOperators::TryFromVar<Js::JavascriptProxy>(source);
-        if (proxy != nullptr)
+
+        while (Js::VarIs<Js::JavascriptProxy>(source))
         {
-            source = proxy->GetTarget();
+            source = Js::UnsafeVarTo<Js::JavascriptProxy>(source)->GetTarget();
         }
 
         // We can currently only clone certain types of dynamic objects

lib/Jsrt/Jsrt.cpp

@@ -2723,9 +2723,9 @@ CHAKRA_API JsHasExternalData(_In_ JsValueRef object, _Out_ bool *value)
 
     BEGIN_JSRT_NO_EXCEPTION
     {
-        if (Js::VarIs<Js::JavascriptProxy>(object))
+        while (Js::VarIs<Js::JavascriptProxy>(object))
         {
-            object = Js::VarTo<Js::JavascriptProxy>(object);
+            object = Js::UnsafeVarTo<Js::JavascriptProxy>(object);
         }
         *value = (Js::VarIs<JsrtExternalObject>(object)
 #ifdef _CHAKRACOREBUILD
@@ -2743,18 +2743,18 @@ CHAKRA_API JsGetExternalData(_In_ JsValueRef object, _Out_ void **data)
 
     BEGIN_JSRT_NO_EXCEPTION
     {
-        if (Js::VarIs<Js::JavascriptProxy>(object))
+        while (Js::VarIs<Js::JavascriptProxy>(object))
         {
-            object = Js::VarTo<Js::JavascriptProxy>(object)->GetTarget();
+            object = Js::UnsafeVarTo<Js::JavascriptProxy>(object)->GetTarget();
         }
         if (Js::VarIs<JsrtExternalObject>(object))
         {
-            *data = Js::VarTo<JsrtExternalObject>(object)->GetSlotData();
+            *data = Js::UnsafeVarTo<JsrtExternalObject>(object)->GetSlotData();
         }
 #ifdef _CHAKRACOREBUILD
         else if (Js::VarIs<Js::CustomExternalWrapperObject>(object))
         {
-            *data = Js::VarTo<Js::CustomExternalWrapperObject>(object)->GetSlotData();
+            *data = Js::UnsafeVarTo<Js::CustomExternalWrapperObject>(object)->GetSlotData();
         }
 #endif
         else
@@ -2772,18 +2772,18 @@ CHAKRA_API JsSetExternalData(_In_ JsValueRef object, _In_opt_ void *data)
 
     BEGIN_JSRT_NO_EXCEPTION
     {
-        if (Js::VarIs<Js::JavascriptProxy>(object))
+        while (Js::VarIs<Js::JavascriptProxy>(object))
         {
-            object = Js::VarTo<Js::JavascriptProxy>(object)->GetTarget();
+            object = Js::UnsafeVarTo<Js::JavascriptProxy>(object)->GetTarget();
         }
         if (Js::VarIs<JsrtExternalObject>(object))
         {
-            Js::VarTo<JsrtExternalObject>(object)->SetSlotData(data);
+            Js::UnsafeVarTo<JsrtExternalObject>(object)->SetSlotData(data);
         }
 #ifdef _CHAKRACOREBUILD
         else if (Js::VarIs<Js::CustomExternalWrapperObject>(object))
         {
-            Js::VarTo<Js::CustomExternalWrapperObject>(object)->SetSlotData(data);
+            Js::UnsafeVarTo<Js::CustomExternalWrapperObject>(object)->SetSlotData(data);
         }
 #endif
         else

lib/Jsrt/JsrtExternalObject.cpp

@@ -157,8 +157,7 @@ JsrtExternalObject* JsrtExternalObject::Create(void *data, uint inlineSlotSize,
     return externalObject;
 }
 
-JsrtExternalObject*
-JsrtExternalObject::Copy(bool deepCopy)
+JsrtExternalObject* JsrtExternalObject::Copy(bool deepCopy)
 {
     Recycler* recycler = this->GetRecycler();
     JsrtExternalType* type = this->GetExternalType();

lib/Runtime/Library/CustomExternalWrapperObject.cpp

@@ -140,8 +140,7 @@ BOOL CustomExternalWrapperObject::EnsureInitialized(ScriptContext* requestContex
     return TRUE;
 }
 
-CustomExternalWrapperObject*
-CustomExternalWrapperObject::Copy(bool deepCopy)
+CustomExternalWrapperObject* CustomExternalWrapperObject::Copy(bool deepCopy)
 {
     Recycler* recycler = this->GetRecycler();
     CustomExternalWrapperType* externalType = this->GetExternalType();