Share types with deleted properties

chakra-core · Sep 15, 2018 · 6caf4a2 · 6caf4a2
1 parent c666aa4
commit 6caf4a2
Show file tree

Hide file tree

Showing 12 changed files with 186 additions and 65 deletions.
diff --git a/lib/Common/ConfigFlagsList.h b/lib/Common/ConfigFlagsList.h
@@ -383,6 +383,7 @@ PHASE(All)
         PHASE(TypePathDynamicSize)
         PHASE(ObjectCopy)
         PHASE(ShareTypesWithAttributes)
+        PHASE(ShareTypesWithDeleted)
         PHASE(ShareAccessorTypes)
         PHASE(ShareFuncTypes)
         PHASE(ConditionalCompilation)

diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -1696,6 +1696,10 @@ using namespace Js;
     template <bool unscopables>
     BOOL JavascriptOperators::GetProperty_Internal(Var instance, RecyclableObject* propertyObject, const bool isRoot, PropertyId propertyId, Var* value, ScriptContext* requestContext, PropertyValueInfo* info)
     {
+#if ENABLE_FIXED_FIELDS && DBG
+        DynamicTypeHandler *dynamicTypeHandler = nullptr;
+#endif
+
         if (TaggedNumber::Is(instance))
         {
             PropertyValueInfo::ClearCacheInfo(info);
@@ -1718,6 +1722,9 @@ using namespace Js;
             }
             else
             {
+#if ENABLE_FIXED_FIELDS && DBG
+                dynamicTypeHandler = DynamicObject::Is(object) ? DynamicObject::FromVar(object)->GetTypeHandler() : nullptr;
+#endif
                 PropertyQueryFlags result = object->GetPropertyQuery(instance, propertyId, value, info, requestContext);
                 if (result != PropertyQueryFlags::Property_NotFound)
                 {
@@ -1737,10 +1744,10 @@ using namespace Js;
         if (foundProperty)
         {
 #if ENABLE_FIXED_FIELDS && DBG
-            if (DynamicObject::Is(object))
+            // Note: It's valid to check this for the original type handler but not for a new type handler that may have been installed
+            // by a getter that, for instance, deleted and re-added the property.
+            if (dynamicTypeHandler)
             {
-                DynamicObject* dynamicObject = (DynamicObject*)object;
-                DynamicTypeHandler* dynamicTypeHandler = dynamicObject->GetDynamicType()->GetTypeHandler();
                 Var property;
                 if (dynamicTypeHandler->CheckFixedProperty(requestContext->GetPropertyName(propertyId), &property, requestContext))
                 {
@@ -8825,23 +8832,18 @@ using namespace Js;
                         // ES5 8.12.9.9.c: Convert the property named P of object O from an accessor property to a data property.
                         // Preserve the existing values of the converted property's [[Configurable]] and [[Enumerable]] attributes
                         // and set the rest of the property's attributes to their default values.
-                        // Note: avoid using SetProperty/SetPropertyWithAttributes here because they has undesired side-effects:
-                        //       it calls previous setter and in some cases of attribute values throws.
-                        //       To walk around, call DeleteProperty and then AddProperty.
                         PropertyAttributes preserveFromObject = currentDescriptor->GetAttributes() & (PropertyConfigurable | PropertyEnumerable);
 
                         tempDescriptor.SetAttributes(preserveFromObject, PropertyConfigurable | PropertyEnumerable);
                         tempDescriptor.MergeFrom(descriptor);   // Update only fields specified in 'descriptor'.
                         Var descriptorValue = descriptor.ValueSpecified() ? descriptor.GetValue() : defaultDataValue;
 
-                        // Note: HostDispath'es implementation of DeleteProperty currently throws E_NOTIMPL.
-                        obj->DeleteProperty(propId, PropertyOperation_None);
-                        BOOL tempResult = obj->SetPropertyWithAttributes(propId, descriptorValue, tempDescriptor.GetAttributes(), NULL, PropertyOperation_Force);
-                        Assert(tempResult);
+                        BOOL result = DynamicObject::FromVar(obj)->ConvertAccessorToData(propId, descriptorValue, tempDescriptor.GetAttributes());
 
                         // At this time we already set value and attributes to desired values,
                         // thus we can skip step ES5 8.12.9.12 and simply return true.
-                        return TRUE;
+                        Assert(result);
+                        return result;
                     }
                 }
             }

diff --git a/lib/Runtime/RuntimeCommon.h b/lib/Runtime/RuntimeCommon.h
@@ -84,6 +84,7 @@ namespace Js
         ObjectSlotAttr_Int =          0x20,
         ObjectSlotAttr_Double =       0x40,
         ObjectSlotAttr_Default =      (ObjectSlotAttr_Writable|ObjectSlotAttr_Enumerable|ObjectSlotAttr_Configurable),
+        ObjectSlotAttr_DeletedDefault = (ObjectSlotAttr_Deleted|ObjectSlotAttr_Writable|ObjectSlotAttr_Configurable),
         ObjectSlotAttr_PropertyAttributesMask = (ObjectSlotAttr_Default|ObjectSlotAttr_Deleted),
         ObjectSlotAttr_All =          0xFF,
         ObjectSlotAttr_Setter =       ObjectSlotAttr_All ^ ObjectSlotAttr_Deleted,   // an impossible value indicating "setter"

diff --git a/lib/Runtime/Types/DynamicObject.cpp b/lib/Runtime/Types/DynamicObject.cpp
@@ -382,6 +382,19 @@ namespace Js
         this->type = predecessorType;
     }
 
+    BOOL DynamicObject::ConvertAccessorToData(PropertyId propId, Var value, PropertyAttributes attributes)
+    {
+        uint32 indexVal;
+        if (GetScriptContext()->IsNumericPropertyId(propId, &indexVal))
+        {
+            DeleteItem(indexVal, PropertyOperation_None);
+        }
+        GetTypeHandler()->InitProperty(this, propId, value, PropertyOperation_Force, nullptr);
+        BOOL result = SetPropertyWithAttributes(propId, value, attributes, NULL, PropertyOperation_Force);
+        Assert(result);
+        return result;
+    }
+
     DWORD DynamicObject::GetOffsetOfAuxSlots()
     {
         return offsetof(DynamicObject, auxSlots);

diff --git a/lib/Runtime/Types/DynamicObject.h b/lib/Runtime/Types/DynamicObject.h
@@ -314,6 +314,8 @@ namespace Js
 
         void ChangeTypeIf(const Type* oldType);
 
+        BOOL ConvertAccessorToData(PropertyId propId, Var value, PropertyAttributes attributes);
+
         BOOL FindNextProperty(BigPropertyIndex& index, JavascriptString** propertyString, PropertyId* propertyId, PropertyAttributes* attributes,
             DynamicType *typeToEnumerate, EnumeratorFlags flags, ScriptContext * requestContext, PropertyValueInfo * info);