Changeset 201834 in webkit
- Timestamp:
- Jun 8, 2016 2:49:32 PM (8 years ago)
- Location:
- trunk/Source
- Files:
-
- 6 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/JavaScriptCore/ChangeLog
r201830 r201834 1 2016-06-08 Gavin Barraclough <barraclough@apple.com> 2 3 Remove removeDirect 4 https://bugs.webkit.org/show_bug.cgi?id=158516 5 6 Reviewed by Ryosuke Niwa. 7 8 removeDirect is typically used as a subroutine of deleteProperty, but is also available to 9 call directly. Having this functionality factored out to a separate routine is a bad idea 10 on a couple of fronts: 11 12 - for the main use within deleteProperty there is redundancy (presence of the property 13 was being checked twice) and inconsistency (the two functions returned different results 14 in the case of a nonexistent property; the result from removeDirect was never observed). 15 16 - all uses of removeDirect are in practical terms incorrect. removeDirect had the 17 advantage of ignoring the configurable (DontDelete) attributes, but this is achievable 18 using the DeletePropertyMode setting - and the disadvantage of failing delete static 19 table properties. Last uses were one that was removed in bug #158295 (where failure to 20 delete static properties was a problem), and as addressed in this patch removeDirect is 21 being used to implement runtime enabled features. This only works because we currently 22 force reification of all properties on the DOM prototype objects, so in effect there are 23 no static properties. In order to make the code robust such that runtime enabled 24 features would still work even if we were not reifying static properties (a change we 25 may want to make) we should be calling deleteProperty in this case too. 26 27 * runtime/JSObject.cpp: 28 (JSC::JSObject::deleteProperty): 29 - incorporated removeDirect functionality, added comments & ASSERT. 30 (JSC::JSObject::removeDirect): Deleted. 31 - removed. 32 * runtime/JSObject.h: 33 - removed removeDirect. 34 1 35 2016-06-08 Mark Lam <mark.lam@apple.com> 2 36 -
trunk/Source/JavaScriptCore/runtime/JSObject.cpp
r201766 r201834 1497 1497 return thisObject->methodTable(vm)->deletePropertyByIndex(thisObject, exec, index.value()); 1498 1498 1499 unsigned attributes; 1500 1499 1501 if (!thisObject->staticFunctionsReified()) { 1500 1502 if (auto* entry = thisObject->findPropertyHashEntry(propertyName)) { 1501 if (entry->attributes() & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) 1503 // If the static table contains a non-configurable (DontDelete) property then we can return early; 1504 // if there is a property in the storage array it too must be non-configurable (the language does 1505 // not allow repacement of a non-configurable property with a configurable one). 1506 if (entry->attributes() & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) { 1507 ASSERT(!isValidOffset(thisObject->structure(vm)->get(vm, propertyName, attributes)) || attributes & DontDelete); 1502 1508 return false; 1509 } 1503 1510 thisObject->reifyAllStaticProperties(exec); 1504 1511 } 1505 1512 } 1506 1513 1507 unsigned attributes; 1508 if (isValidOffset(thisObject->structure(vm)->get(vm, propertyName, attributes))) { 1514 Structure* structure = thisObject->structure(vm); 1515 1516 bool propertyIsPresent = isValidOffset(structure->get(vm, propertyName, attributes)); 1517 if (propertyIsPresent) { 1509 1518 if (attributes & DontDelete && vm.deletePropertyMode() != VM::DeletePropertyMode::IgnoreConfigurable) 1510 1519 return false; 1511 thisObject->removeDirect(vm, propertyName); 1520 1521 PropertyOffset offset; 1522 if (structure->isUncacheableDictionary()) 1523 offset = structure->removePropertyWithoutTransition(vm, propertyName); 1524 else 1525 thisObject->setStructure(vm, Structure::removePropertyTransition(vm, structure, propertyName, offset)); 1526 1527 if (offset != invalidOffset) 1528 thisObject->putDirectUndefined(offset); 1512 1529 } 1513 1530 … … 1977 1994 1978 1995 structure(vm)->setStaticFunctionsReified(true); 1979 }1980 1981 bool JSObject::removeDirect(VM& vm, PropertyName propertyName)1982 {1983 Structure* structure = this->structure(vm);1984 if (!isValidOffset(structure->get(vm, propertyName)))1985 return false;1986 1987 PropertyOffset offset;1988 if (structure->isUncacheableDictionary()) {1989 offset = structure->removePropertyWithoutTransition(vm, propertyName);1990 if (offset == invalidOffset)1991 return false;1992 putDirectUndefined(offset);1993 return true;1994 }1995 1996 setStructure(vm, Structure::removePropertyTransition(vm, structure, propertyName, offset));1997 if (offset == invalidOffset)1998 return false;1999 putDirectUndefined(offset);2000 return true;2001 1996 } 2002 1997 -
trunk/Source/JavaScriptCore/runtime/JSObject.h
r201712 r201834 623 623 void transitionTo(VM&, Structure*); 624 624 625 JS_EXPORT_PRIVATE bool removeDirect(VM&, PropertyName); // Return true if anything is removed.626 625 bool hasCustomProperties() { return structure()->didTransition(); } 627 626 bool hasGetterSetterProperties() { return structure()->hasGetterSetterProperties(); } -
trunk/Source/WebCore/ChangeLog
r201832 r201834 1 2016-06-08 Gavin Barraclough <barraclough@apple.com> 2 3 Remove removeDirect 4 https://bugs.webkit.org/show_bug.cgi?id=158516 5 6 Reviewed by Ryosuke Niwa. 7 8 removeDirect is typically used as a subroutine of deleteProperty, but is also available to 9 call directly. Having this functionality factored out to a separate routine is a bad idea 10 on a couple of fronts: 11 12 - for the main use within deleteProperty there is redundancy (presence of the property 13 was being checked twice) and inconsistency (the two functions returned different results 14 in the case of a nonexistent property; the result from removeDirect was never observed). 15 16 - all uses of removeDirect are in practical terms incorrect. removeDirect had the 17 advantage of ignoring the configurable (DontDelete) attributes, but this is achievable 18 using the DeletePropertyMode setting - and the disadvantage of failing delete static 19 table properties. Last uses were one that was removed in bug #158295 (where failure to 20 delete static properties was a problem), and as addressed in this patch removeDirect is 21 being used to implement runtime enabled features. This only works because we currently 22 force reification of all properties on the DOM prototype objects, so in effect there are 23 no static properties. In order to make the code robust such that runtime enabled 24 features would still work even if we were not reifying static properties (a change we 25 may want to make) we should be calling deleteProperty in this case too. 26 27 * bindings/scripts/CodeGeneratorJS.pm: 28 (GenerateImplementation): 29 - changed to call deleteProperty instead of removeDirect. 30 * bindings/scripts/test/JS/JSTestObj.cpp: 31 (WebCore::JSTestObjPrototype::finishCreation): 32 - updated bindings test results. 33 1 34 2016-06-08 Nan Wang <n_wang@apple.com> 2 35 -
trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
r201808 r201834 2283 2283 push(@implContent, " if (!${enable_function}()) {\n"); 2284 2284 push(@implContent, " Identifier propertyName = Identifier::fromString(&vm, reinterpret_cast<const LChar*>(\"$name\"), strlen(\"$name\"));\n"); 2285 push(@implContent, " removeDirect(vm, propertyName);\n"); 2285 push(@implContent, " VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable);\n"); 2286 push(@implContent, " JSObject::deleteProperty(this, globalObject()->globalExec(), propertyName);\n"); 2286 2287 push(@implContent, " }\n"); 2287 2288 push(@implContent, "#endif\n") if $conditionalString; -
trunk/Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
r201719 r201834 1311 1311 if (!RuntimeEnabledFeatures::sharedFeatures().testFeatureEnabled()) { 1312 1312 Identifier propertyName = Identifier::fromString(&vm, reinterpret_cast<const LChar*>("enabledAtRuntimeOperation"), strlen("enabledAtRuntimeOperation")); 1313 removeDirect(vm, propertyName); 1313 VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable); 1314 JSObject::deleteProperty(this, globalObject()->globalExec(), propertyName); 1314 1315 } 1315 1316 #endif … … 1317 1318 if (!RuntimeEnabledFeatures::sharedFeatures().testFeatureEnabled()) { 1318 1319 Identifier propertyName = Identifier::fromString(&vm, reinterpret_cast<const LChar*>("enabledAtRuntimeAttribute"), strlen("enabledAtRuntimeAttribute")); 1319 removeDirect(vm, propertyName); 1320 VM::DeletePropertyModeScope scope(vm, VM::DeletePropertyMode::IgnoreConfigurable); 1321 JSObject::deleteProperty(this, globalObject()->globalExec(), propertyName); 1320 1322 } 1321 1323 #endif
Note: See TracChangeset
for help on using the changeset viewer.