We don't need special handle OwnKeys for CEO wrapper.

akroshg · akroshg · commit d5407a9c0e13 · 2019-02-27T16:37:22.000-08:00
The code below will make sure to dispatch to CEO in proper way
diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -1237,16 +1237,6 @@ using namespace Js;
         {
             return proxy->PropertyKeysTrap(JavascriptProxy::KeysTrapKind::GetOwnPropertyNamesKind, scriptContext);
         }
-#ifdef _CHAKRACOREBUILD
-        else
-        {
-            CustomExternalWrapperObject * wrapper = JavascriptOperators::TryFromVar<CustomExternalWrapperObject>(instance);
-            if (wrapper)
-            {
-                return wrapper->PropertyKeysTrap(CustomExternalWrapperObject::KeysTrapKind::GetOwnPropertyNamesKind, scriptContext);
-            }
-        }
-#endif
 
         return JavascriptObject::CreateOwnStringPropertiesHelper(object, scriptContext);
     }
@@ -1262,16 +1252,6 @@ using namespace Js;
         {
             return proxy->PropertyKeysTrap(JavascriptProxy::KeysTrapKind::GetOwnPropertySymbolKind, scriptContext);
         }
-#ifdef _CHAKRACOREBUILD
-        else
-        {
-            CustomExternalWrapperObject * wrapper = JavascriptOperators::TryFromVar<CustomExternalWrapperObject>(instance);
-            if (wrapper)
-            {
-                return wrapper->PropertyKeysTrap(CustomExternalWrapperObject::KeysTrapKind::GetOwnPropertySymbolKind, scriptContext);
-            }
-        }
-#endif
 
         return JavascriptObject::CreateOwnSymbolPropertiesHelper(object, scriptContext);
     }
@@ -1286,16 +1266,6 @@ using namespace Js;
         {
             return proxy->PropertyKeysTrap(JavascriptProxy::KeysTrapKind::KeysKind, scriptContext);
         }
-#ifdef _CHAKRACOREBUILD
-        else
-        {
-            CustomExternalWrapperObject * wrapper = JavascriptOperators::TryFromVar<CustomExternalWrapperObject>(instance);
-            if (wrapper)
-            {
-                return wrapper->PropertyKeysTrap(CustomExternalWrapperObject::KeysTrapKind::KeysKind, scriptContext);
-            }
-        }
-#endif
 
         return JavascriptObject::CreateOwnStringSymbolPropertiesHelper(object, scriptContext);
     }
@@ -1331,43 +1301,6 @@ using namespace Js;
             }
             return proxyResultToReturn;
         }
-#ifdef _CHAKRACOREBUILD
-        else
-        {
-            CustomExternalWrapperObject * wrapper = JavascriptOperators::TryFromVar<CustomExternalWrapperObject>(object);
-            if (wrapper)
-            {
-                JavascriptArray* wrapperResult = wrapper->PropertyKeysTrap(CustomExternalWrapperObject::KeysTrapKind::GetOwnEnumerablePropertyNamesKind, scriptContext);
-                JavascriptArray* wrapperResultToReturn = scriptContext->GetLibrary()->CreateArray(0);
-                if (wrapperResult != nullptr)
-                {
-                    // filter enumerable keys
-                    uint32 resultLength = wrapperResult->GetLength();
-                    Var element;
-                    const Js::PropertyRecord *propertyRecord = nullptr;
-                    uint32 index = 0;
-                    for (uint32 i = 0; i < resultLength; i++)
-                    {
-                        element = wrapperResult->DirectGetItem(i);
-
-                        Assert(!VarIs<JavascriptSymbol>(element));
-
-                        PropertyDescriptor propertyDescriptor;
-                        JavascriptConversion::ToPropertyKey(element, scriptContext, &propertyRecord, nullptr);
-                        if (JavascriptOperators::GetOwnPropertyDescriptor(object, propertyRecord->GetPropertyId(), scriptContext, &propertyDescriptor))
-                        {
-                            if (propertyDescriptor.IsEnumerable())
-                            {
-                                wrapperResultToReturn->DirectSetItemAt(index++, CrossSite::MarshalVar(scriptContext, element));
-                            }
-                        }
-                    }
-                }
-
-                return wrapperResultToReturn;
-            }
-        }
-#endif
 
         return JavascriptObject::CreateOwnEnumerableStringPropertiesHelper(object, scriptContext);
     }
@@ -1379,16 +1312,6 @@ using namespace Js;
         {
             return proxy->PropertyKeysTrap(JavascriptProxy::KeysTrapKind::KeysKind, scriptContext);
         }
-#ifdef _CHAKRACOREBUILD
-        else
-        {
-            CustomExternalWrapperObject * wrapper = JavascriptOperators::TryFromVar<CustomExternalWrapperObject>(object);
-            if (wrapper)
-            {
-                return wrapper->PropertyKeysTrap(CustomExternalWrapperObject::KeysTrapKind::EnumerableKeysKind, scriptContext);
-            }
-        }
-#endif
 
         return JavascriptObject::CreateOwnEnumerableStringSymbolPropertiesHelper(object, scriptContext);
     }
diff --git a/lib/Runtime/Library/CustomExternalWrapperObject.cpp b/lib/Runtime/Library/CustomExternalWrapperObject.cpp
@@ -1184,192 +1184,6 @@ BOOL CustomExternalWrapperObject::DeleteProperty(Js::JavascriptString *propertyN
     return TRUE;
 }
 
-Js::JavascriptArray * CustomExternalWrapperObject::PropertyKeysTrap(KeysTrapKind keysTrapKind, Js::ScriptContext * requestContext)
-{
-    PROBE_STACK(GetScriptContext(), Js::Constants::MinStackDefault);
-
-    // Reject implicit call
-    ThreadContext* threadContext = requestContext->GetThreadContext();
-    if (threadContext->IsDisableImplicitCall())
-    {
-        threadContext->AddImplicitCallFlags(Js::ImplicitCall_External);
-        return nullptr;
-    }
-
-    if (!this->EnsureInitialized(requestContext))
-    {
-        return nullptr;
-    }
-
-    Js::RecyclableObject * targetObj = this;
-    CustomExternalWrapperType * type = this->GetExternalType();
-    Js::JavascriptFunction* ownKeysMethod = nullptr;
-    if (type->GetJsGetterSetterInterceptor()->ownKeysTrap != nullptr)
-    {
-        ownKeysMethod = Js::VarTo<JavascriptFunction>(type->GetJsGetterSetterInterceptor()->ownKeysTrap);
-    }
-
-    Assert(!GetScriptContext()->IsHeapEnumInProgress());
-
-    if (nullptr == ownKeysMethod)
-    {
-        switch (keysTrapKind)
-        {
-        case KeysTrapKind::GetOwnPropertyNamesKind:
-            return JavascriptObject::CreateOwnStringPropertiesHelper(this, requestContext);
-        case KeysTrapKind::GetOwnPropertySymbolKind:
-            return JavascriptObject::CreateOwnSymbolPropertiesHelper(this, requestContext);
-        case KeysTrapKind::KeysKind:
-            return JavascriptObject::CreateOwnStringSymbolPropertiesHelper(this, requestContext);
-        case KeysTrapKind::GetOwnEnumerablePropertyNamesKind:
-            return JavascriptObject::CreateOwnEnumerableStringPropertiesHelper(this, requestContext);
-        case KeysTrapKind::EnumerableKeysKind:
-            return JavascriptObject::CreateOwnEnumerableStringSymbolPropertiesHelper(this, requestContext);
-        default:
-            Assume(UNREACHED);
-        }
-    }
-
-    Js::Var ownKeysResult = threadContext->ExecuteImplicitCall(ownKeysMethod, Js::ImplicitCall_Accessor, [=]()->Js::Var
-    {
-        return CALL_FUNCTION(threadContext, ownKeysMethod, Js::CallInfo(Js::CallFlags_Value, 1), targetObj);
-    });
-
-    if (!Js::JavascriptOperators::IsObject(ownKeysResult))
-    {
-        Js::JavascriptError::ThrowTypeError(requestContext, JSERR_InconsistentTrapResult, _u("ownKeys"));
-    }
-
-    Js::RecyclableObject* trapResultArray = Js::VarTo<RecyclableObject>(ownKeysResult);
-    Js::JavascriptArray* trapResult = requestContext->GetLibrary()->CreateArray(0);
-    bool isConfigurableKeyMissingFromTrapResult = false;
-    bool isNonconfigurableKeyMissingFromTrapResult = false;
-    bool isKeyMissingFromTrapResult = false;
-    bool isKeyMissingFromTargetResult = false;
-    bool isAnyNonconfigurableKeyPresent = false;
-    Js::Var element;
-    Js::PropertyId propertyId;
-    const Js::PropertyRecord* propertyRecord = nullptr;
-    BOOL isTargetExtensible = FALSE;
-
-    BEGIN_TEMP_ALLOCATOR(tempAllocator, requestContext, _u("Runtime"))
-    {
-        // Dictionary containing intersection of keys present in targetKeys and trapResult
-        Js::Var lenValue = Js::JavascriptOperators::OP_GetLength(trapResultArray, requestContext);
-        uint32 len = (uint32)Js::JavascriptConversion::ToLength(lenValue, requestContext);
-        JsUtil::BaseDictionary<Js::PropertyId, bool, Memory::ArenaAllocator> targetToTrapResultMap(tempAllocator, len);
-
-        // Trap result to return.
-        // Note : This will not necessarily have all elements present in trapResultArray. E.g. If trap was called from GetOwnPropertySymbols()
-        // trapResult will only contain symbol elements from trapResultArray.
-        switch (keysTrapKind)
-        {
-        case GetOwnPropertyNamesKind:
-        case KeysTrapKind::GetOwnEnumerablePropertyNamesKind:
-            GetOwnPropertyKeysHelper(requestContext, trapResultArray, len, trapResult, targetToTrapResultMap,
-                [&](const Js::PropertyRecord *propertyRecord)->bool
-            {
-                return !propertyRecord->IsSymbol();
-            });
-            break;
-        case GetOwnPropertySymbolKind:
-            GetOwnPropertyKeysHelper(requestContext, trapResultArray, len, trapResult, targetToTrapResultMap,
-                [&](const Js::PropertyRecord *propertyRecord)->bool
-            {
-                return propertyRecord->IsSymbol();
-            });
-            break;
-        case KeysKind:
-        case EnumerableKeysKind:
-            GetOwnPropertyKeysHelper(requestContext, trapResultArray, len, trapResult, targetToTrapResultMap,
-                [&](const Js::PropertyRecord *propertyRecord)->bool
-            {
-                return true;
-            });
-            break;
-        }
-
-        isTargetExtensible = targetObj->IsExtensible();
-        Js::JavascriptArray * targetKeys = Js::JavascriptOperators::GetOwnPropertyKeys(targetObj, requestContext);
-
-        for (uint32 i = 0; i < targetKeys->GetLength(); i++)
-        {
-            element = targetKeys->DirectGetItem(i);
-            AssertMsg(Js::VarIs<JavascriptSymbol>(element) || Js::VarIs<JavascriptString>(element), "Invariant check during ownKeys wrapper trap should make sure we only get property key here. (symbol or string primitives)");
-            Js::JavascriptConversion::ToPropertyKey(element, requestContext, &propertyRecord, nullptr);
-            propertyId = propertyRecord->GetPropertyId();
-
-            if (propertyId == Js::Constants::NoProperty)
-                continue;
-
-            // If not present in intersection means either the property is not present in targetKeys or
-            // we have already visited the property in targetKeys
-            if (targetToTrapResultMap.ContainsKey(propertyId))
-            {
-                isKeyMissingFromTrapResult = false;
-                targetToTrapResultMap.Remove(propertyId);
-            }
-            else
-            {
-                isKeyMissingFromTrapResult = true;
-            }
-
-            Js::PropertyDescriptor targetKeyPropertyDescriptor;
-            if (Js::JavascriptOperators::GetOwnPropertyDescriptor(targetObj, propertyId, requestContext, &targetKeyPropertyDescriptor) && !targetKeyPropertyDescriptor.IsConfigurable())
-            {
-                isAnyNonconfigurableKeyPresent = true;
-                if (isKeyMissingFromTrapResult)
-                {
-                    isNonconfigurableKeyMissingFromTrapResult = true;
-                }
-            }
-            else
-            {
-                if (isKeyMissingFromTrapResult)
-                {
-                    isConfigurableKeyMissingFromTrapResult = true;
-                }
-            }
-        }
-        // Keys that were not found in targetKeys will continue to remain in the map
-        isKeyMissingFromTargetResult = targetToTrapResultMap.Count() != 0;
-    }
-    END_TEMP_ALLOCATOR(tempAllocator, requestContext)
-
-
-    // 19.
-    if (isTargetExtensible && !isAnyNonconfigurableKeyPresent)
-    {
-        return trapResult;
-    }
-
-    // 21.
-    if (isNonconfigurableKeyMissingFromTrapResult)
-    {
-        Js::JavascriptError::ThrowTypeError(requestContext, JSERR_InconsistentTrapResult, _u("ownKeys"));
-    }
-
-    // 22.
-    if (isTargetExtensible)
-    {
-        return trapResult;
-    }
-
-    // 23.
-    if (isConfigurableKeyMissingFromTrapResult)
-    {
-        Js::JavascriptError::ThrowTypeError(requestContext, JSERR_InconsistentTrapResult, _u("ownKeys"));
-    }
-
-    // 24.
-    if (isKeyMissingFromTargetResult)
-    {
-        Js::JavascriptError::ThrowTypeError(requestContext, JSERR_InconsistentTrapResult, _u("ownKeys"));
-    }
-
-    return trapResult;
-}
-
 #if ENABLE_TTD
 TTD::NSSnapObjects::SnapObjectType CustomExternalWrapperObject::GetSnapTag_TTD() const
 {
diff --git a/lib/Runtime/Library/CustomExternalWrapperObject.h b/lib/Runtime/Library/CustomExternalWrapperObject.h
@@ -113,8 +113,6 @@ namespace Js
         BOOL SetPropertyTrap(Var receiver, SetPropertyTrapKind setPropertyTrapKind, GetPropertyNameFunc getPropertyName, Var newValue, ScriptContext * requestContext, PropertyOperationFlags propertyOperationFlags, BOOL skipPrototypeCheck, Fn fn);
         BOOL SetPropertyTrap(Var receiver, SetPropertyTrapKind setPropertyTrapKind, JavascriptString * propertyString, Var newValue, ScriptContext * requestContext, PropertyOperationFlags propertyOperationFlags);
 
-        JavascriptArray * PropertyKeysTrap(KeysTrapKind keysTrapKind, ScriptContext * requestContext);
-
         void * GetSlotData() const;
         void SetSlotData(void * data);
         int GetInlineSlotSize() const;
