Source/JavaScriptCore/ChangeLog

 12015-11-19 Chris Dumez <cdumez@apple.com>
 2
 3 Caching of properties on objects that have named property getters is sometimes incorrect
 4 https://bugs.webkit.org/show_bug.cgi?id=151453
 5 <rdar://problem/23049343>
 6
 7 Reviewed by NOBODY (OOPS!).
 8
 9 Add new GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag to be
 10 used by objects that have a non-'OverrideBuiltins' named property getter.
 11 This flag prevents caching of properties that are missing as a named
 12 property with this name may later become available.
 13
 14 Objects with an 'OverrideBuiltins' named property getter will keep using
 15 the GetOwnPropertySlotIsImpure TypeInfo flag, which prevents all property
 16 caching since named properties can override own properties or properties
 17 on the prototype.
 18
 19 * bytecode/ComplexGetStatus.cpp:
 20 (JSC::ComplexGetStatus::computeFor):
 21 * bytecode/PropertyCondition.cpp:
 22 (JSC::PropertyCondition::isStillValid):
 23 * jit/Repatch.cpp:
 24 (JSC::tryCacheGetByID):
 25 (JSC::tryRepatchIn):
 26 * jsc.cpp:
 27 * runtime/JSTypeInfo.h:
 28 (JSC::TypeInfo::getOwnPropertySlotIsImpure):
 29 (JSC::TypeInfo::getOwnPropertySlotIsImpureForPropertyAbsence):
 30 (JSC::TypeInfo::prohibitsPropertyCaching): Deleted.
 31 * runtime/Structure.h:
 32
1332015-11-19 Csaba Osztrogonác <ossy@webkit.org>
234
335 Unreviewed CLOOP buildfix after r192624.

Source/WebCore/ChangeLog

 12015-11-19 Chris Dumez <cdumez@apple.com>
 2
 3 Caching of properties on objects that have named property getters is sometimes incorrect
 4 https://bugs.webkit.org/show_bug.cgi?id=151453
 5 <rdar://problem/23049343>
 6
 7 Reviewed by NOBODY (OOPS!).
 8
 9 In r188590, we dropped the JSC::GetOwnPropertySlotIsImpure TypeInfo flag for
 10 interfaces that have a non-'OverrideBuiltins' named property getter in order
 11 to allow caching of properties returns by GetOwnPropertySlot(). We assumed
 12 this was safe as it was no longer possible for named properties to override
 13 own properties (or properties on the prototype).
 14
 15 However, there is an issue when we cache the non-existence of a property.
 16 Even though at one point the property did not exist, a named property with
 17 this name may later become available. In such case, caching would cause us
 18 to wrongly report a property as missing.
 19
 20 To address the problem, this patch introduces a new
 21 GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag and uses it for
 22 interfaces that have a non-'OverrideBuiltins' named property getter. This
 23 will cause us to not cache the fact that a property is missing on such
 24 objects, while maintaining the performance win from r188590 in the common
 25 case.
 26
 27 Test: fast/dom/NamedNodeMap-named-getter-caching.html
 28
 29 * bindings/scripts/CodeGeneratorJS.pm:
 30 (GenerateHeader):
 31 * bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
 32 * bindings/scripts/test/JS/JSTestEventTarget.h:
 33 * bindings/scripts/test/JS/JSTestOverrideBuiltins.h:
 34
1352015-11-19 Csaba Osztrogonác <ossy@webkit.org>
236
337 [EFL] Fix -Winconsistent-missing-override clang warnings

Source/JavaScriptCore/bytecode/ComplexGetStatus.cpp

@@ComplexGetStatus ComplexGetStatus::computeFor(
3434 Structure* headStructure, const ObjectPropertyConditionSet& conditionSet, UniquedStringImpl* uid)
3535{
3636 // FIXME: We should assert that we never see a structure that
37  // hasImpureGetOwnPropertySlot() but for which we don't
 37 // getOwnPropertySlotIsImpure() but for which we don't
3838 // newImpurePropertyFiresWatchpoints(). We're not at a point where we can do
3939 // that, yet.
4040 // https://bugs.webkit.org/show_bug.cgi?id=131810

Source/JavaScriptCore/bytecode/PropertyCondition.cpp

@@bool PropertyCondition::isStillValid(Structure* structure, JSObject* base) const
213213 // "shadow" an existing JS property on the same object. Hence it affects both presence and
214214 // absence. It doesn't affect AbsenceOfSetter because impure properties aren't ever setters.
215215 switch (m_kind) {
216  case Presence:
217216 case Absence:
 217 if (structure->typeInfo().getOwnPropertySlotIsImpure() || structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
 218 return false;
 219 break;
 220 case Presence:
218221 case Equivalence:
219  if (structure->typeInfo().hasImpureGetOwnPropertySlot())
 222 if (structure->typeInfo().getOwnPropertySlotIsImpure())
220223 return false;
221224 break;
222225 default:

Source/JavaScriptCore/jit/Repatch.cpp

@@static InlineCacheAction tryCacheGetByID(ExecState* exec, JSValue baseValue, con
283283 if (structure->typeInfo().prohibitsPropertyCaching() || structure->isDictionary())
284284 return GiveUpOnCache;
285285
 286 if (slot.isUnset() && structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
 287 return GiveUpOnCache;
 288
286289 if (slot.isUnset()) {
287290 conditionSet = generateConditionsForPropertyMiss(
288291 vm, codeBlock, exec, structure, propertyName.impl());

@@static InlineCacheAction tryRepatchIn(
485488 if (forceICFailure(exec))
486489 return GiveUpOnCache;
487490
488  if (!base->structure()->propertyAccessesAreCacheable())
 491 if (!base->structure()->propertyAccessesAreCacheable() || (!wasFound && !base->structure()->propertyAccessesAreCacheableForAbsence()))
489492 return GiveUpOnCache;
490493
491494 if (wasFound) {

Source/JavaScriptCore/jsc.cpp

@@public:
267267
268268 DECLARE_INFO;
269269 typedef JSNonFinalObject Base;
270  static const unsigned StructureFlags = Base::StructureFlags | JSC::HasImpureGetOwnPropertySlot | JSC::OverridesGetOwnPropertySlot;
 270 static const unsigned StructureFlags = Base::StructureFlags | JSC::GetOwnPropertySlotIsImpure | JSC::OverridesGetOwnPropertySlot;
271271
272272 static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
273273 {

Source/JavaScriptCore/runtime/JSTypeInfo.h

@@static const unsigned StructureIsImmortal = 1 << 7;
4747
4848static const unsigned OverridesGetPropertyNames = 1 << 8;
4949static const unsigned ProhibitsPropertyCaching = 1 << 9;
50 static const unsigned HasImpureGetOwnPropertySlot = 1 << 10;
 50static const unsigned GetOwnPropertySlotIsImpure = 1 << 10;
5151static const unsigned NewImpurePropertyFiresWatchpoints = 1 << 11;
5252static const unsigned IsEnvironmentRecord = 1 << 12;
 53static const unsigned GetOwnPropertySlotIsImpureForPropertyAbsence = 1 << 13;
5354
5455class TypeInfo {
5556public:

@@public:
9192 bool structureIsImmortal() const { return isSetOnFlags1(StructureIsImmortal); }
9293 bool overridesGetPropertyNames() const { return isSetOnFlags2(OverridesGetPropertyNames); }
9394 bool prohibitsPropertyCaching() const { return isSetOnFlags2(ProhibitsPropertyCaching); }
94  bool hasImpureGetOwnPropertySlot() const { return isSetOnFlags2(HasImpureGetOwnPropertySlot); }
 95 bool getOwnPropertySlotIsImpure() const { return isSetOnFlags2(GetOwnPropertySlotIsImpure); }
 96 bool getOwnPropertySlotIsImpureForPropertyAbsence() const { return isSetOnFlags2(GetOwnPropertySlotIsImpureForPropertyAbsence); }
9597 bool newImpurePropertyFiresWatchpoints() const { return isSetOnFlags2(NewImpurePropertyFiresWatchpoints); }
9698 bool isEnvironmentRecord() const { return isSetOnFlags2(IsEnvironmentRecord); }
9799

Source/JavaScriptCore/runtime/Structure.h

@@public:
204204 {
205205 return dictionaryKind() != UncachedDictionaryKind
206206 && !typeInfo().prohibitsPropertyCaching()
207  && !(typeInfo().hasImpureGetOwnPropertySlot() && !typeInfo().newImpurePropertyFiresWatchpoints());
 207 && !(typeInfo().getOwnPropertySlotIsImpure() && !typeInfo().newImpurePropertyFiresWatchpoints());
 208 }
 209
 210 bool propertyAccessesAreCacheableForAbsence()
 211 {
 212 return !typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence();
208213 }
209214
210215 bool needImpurePropertyWatchpoint()
211216 {
212217 return propertyAccessesAreCacheable()
213  && typeInfo().hasImpureGetOwnPropertySlot()
 218 && typeInfo().getOwnPropertySlotIsImpure()
214219 && typeInfo().newImpurePropertyFiresWatchpoints();
215220 }
216221

@@public:
218223 // DFG from inlining property accesses since structures don't transition when a new impure property appears.
219224 bool takesSlowPathInDFGForImpureProperty()
220225 {
221  return typeInfo().hasImpureGetOwnPropertySlot();
 226 return typeInfo().getOwnPropertySlotIsImpure();
222227 }
223228
224229 // Type accessors.

Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@sub GenerateHeader
982982 my $namedGetterFunction = GetNamedGetterFunction($interface);
983983 my $indexedGetterFunction = GetIndexedGetterFunction($interface);
984984
985  my $hasImpureNamedGetter = $interface->extendedAttributes->{"OverrideBuiltins"}
986  && ($namedGetterFunction || $interface->extendedAttributes->{"CustomNamedGetter"});
 985 my $hasNamedGetter = $namedGetterFunction
 986 || $interface->extendedAttributes->{"CustomNamedGetter"};
987987
988988 my $hasComplexGetter =
989989 $indexedGetterFunction
990990 || $interface->extendedAttributes->{"JSCustomGetOwnPropertySlotAndDescriptor"}
991991 || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"}
992  || $namedGetterFunction
993  || $interface->extendedAttributes->{"CustomNamedGetter"};
 992 || $hasNamedGetter;
994993
995994 my $hasGetter = InstanceOverridesGetOwnPropertySlot($interface);
996995
997  if ($hasImpureNamedGetter) {
998  $structureFlags{"JSC::HasImpureGetOwnPropertySlot"} = 1;
 996 if ($hasNamedGetter) {
 997 if ($interface->extendedAttributes->{"OverrideBuiltins"}) {
 998 $structureFlags{"JSC::GetOwnPropertySlotIsImpure"} = 1;
 999 } else {
 1000 $structureFlags{"JSC::GetOwnPropertySlotIsImpureForPropertyAbsence"} = 1;
 1001 }
9991002 }
10001003 if ($interface->extendedAttributes->{"NewImpurePropertyFiresWatchpoints"}) {
10011004 $structureFlags{"JSC::NewImpurePropertyFiresWatchpoints"} = 1;

Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h

@@public:
5353
5454 static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
5555public:
56  static const unsigned StructureFlags = JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | Base::StructureFlags;
 56 static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpureForPropertyAbsence | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | Base::StructureFlags;
5757protected:
5858 JSTestCustomNamedGetter(JSC::Structure*, JSDOMGlobalObject&, Ref<TestCustomNamedGetter>&&);
5959

Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h

@@public:
5757 static void visitChildren(JSCell*, JSC::SlotVisitor&);
5858
5959public:
60  static const unsigned StructureFlags = JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::MasqueradesAsUndefined | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
 60 static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpureForPropertyAbsence | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::MasqueradesAsUndefined | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
6161protected:
6262 JSTestEventTarget(JSC::Structure*, JSDOMGlobalObject&, Ref<TestEventTarget>&&);
6363

Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.h

@@public:
5454 static void getOwnPropertyNames(JSC::JSObject*, JSC::ExecState*, JSC::PropertyNameArray&, JSC::EnumerationMode = JSC::EnumerationMode());
5555 static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
5656public:
57  static const unsigned StructureFlags = JSC::HasImpureGetOwnPropertySlot | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
 57 static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpure | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
5858protected:
5959 JSTestOverrideBuiltins(JSC::Structure*, JSDOMGlobalObject&, Ref<TestOverrideBuiltins>&&);
6060

LayoutTests/ChangeLog

 12015-11-19 Chris Dumez <cdumez@apple.com>
 2
 3 Caching of properties on objects that have named property getters is sometimes incorrect
 4 https://bugs.webkit.org/show_bug.cgi?id=151453
 5 <rdar://problem/23049343>
 6
 7 Reviewed by NOBODY (OOPS!).
 8
 9 Add layout test to make sure caching does not cause NamedNodeMap's
 10 named property getter to sometimes wrongly report a property as
 11 missing.
 12
 13 * fast/dom/NamedNodeMap-named-getter-caching-expected.txt: Added.
 14 * fast/dom/NamedNodeMap-named-getter-caching.html: Added.
 15
1162015-11-19 David Kilzer <ddkilzer@apple.com>
217
318 Skip more mediastream tests that crash when painting a mock video source.

LayoutTests/fast/dom/NamedNodeMap-named-getter-caching-expected.txt

 1Tests caching of result of NamedNodeMap named property getter
 2
 3On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 4
 5
 6PASS bodyAttributes.class is undefined
 7PASS lastIterationHasRightValue is true
 8PASS successfullyParsed is true
 9
 10TEST COMPLETE
 11

LayoutTests/fast/dom/NamedNodeMap-named-getter-caching.html

 1<!DOCTYPE html>
 2<html>
 3<body>
 4<script src="../../resources/js-test-pre.js"></script>
 5<script>
 6description("Tests caching of result of NamedNodeMap named property getter");
 7
 8var bodyAttributes = document.body.attributes;
 9shouldBe("bodyAttributes.class", "undefined");
 10
 11var lastIterationHasRightValue = false;
 12for (var i = 0; i < 1000; i++) {
 13 if (i == 999)
 14 document.body.setAttribute('class', 'test');
 15 if (bodyAttributes.class != undefined && i == 999)
 16 lastIterationHasRightValue = true;
 17}
 18
 19shouldBeTrue("lastIterationHasRightValue");
 20
 21</script>
 22<script src="../../resources/js-test-post.js"></script>
 23</body>
 24</html>