<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[192693] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/192693">192693</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2015-11-20 12:45:12 -0800 (Fri, 20 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Caching of properties on objects that have named property getters is sometimes incorrect
https://bugs.webkit.org/show_bug.cgi?id=151453
<rdar://problem/23049343>
Reviewed by Gavin Barraclough.
Source/JavaScriptCore:
Add new GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag to be
used by objects that have a non-'OverrideBuiltins' named property getter.
This flag prevents caching of properties that are missing as a named
property with this name may later become available.
Objects with an 'OverrideBuiltins' named property getter will keep using
the GetOwnPropertySlotIsImpure TypeInfo flag, which prevents all property
caching since named properties can override own properties or properties
on the prototype.
* bytecode/ComplexGetStatus.cpp:
(JSC::ComplexGetStatus::computeFor):
* bytecode/PropertyCondition.cpp:
(JSC::PropertyCondition::isStillValid):
* jit/Repatch.cpp:
(JSC::tryCacheGetByID):
(JSC::tryRepatchIn):
* jsc.cpp:
* runtime/JSTypeInfo.h:
(JSC::TypeInfo::getOwnPropertySlotIsImpure):
(JSC::TypeInfo::getOwnPropertySlotIsImpureForPropertyAbsence):
(JSC::TypeInfo::prohibitsPropertyCaching): Deleted.
* runtime/Structure.h:
Source/WebCore:
In <a href="http://trac.webkit.org/projects/webkit/changeset/188590">r188590</a>, we dropped the JSC::GetOwnPropertySlotIsImpure TypeInfo flag for
interfaces that have a non-'OverrideBuiltins' named property getter in order
to allow caching of properties returns by GetOwnPropertySlot(). We assumed
this was safe as it was no longer possible for named properties to override
own properties (or properties on the prototype).
However, there is an issue when we cache the non-existence of a property.
Even though at one point the property did not exist, a named property with
this name may later become available. In such case, caching would cause us
to wrongly report a property as missing.
To address the problem, this patch introduces a new
GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag and uses it for
interfaces that have a non-'OverrideBuiltins' named property getter. This
will cause us to not cache the fact that a property is missing on such
objects, while maintaining the performance win from <a href="http://trac.webkit.org/projects/webkit/changeset/188590">r188590</a> in the common
case.
Test: fast/dom/NamedNodeMap-named-getter-caching.html
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
* bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
* bindings/scripts/test/JS/JSTestEventTarget.h:
* bindings/scripts/test/JS/JSTestOverrideBuiltins.h:
LayoutTests:
Add layout test to make sure caching does not cause NamedNodeMap's
named property getter to sometimes wrongly report a property as
missing.
* fast/dom/NamedNodeMap-named-getter-caching-expected.txt: Added.
* fast/dom/NamedNodeMap-named-getter-caching.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeComplexGetStatuscpp">trunk/Source/JavaScriptCore/bytecode/ComplexGetStatus.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodePropertyConditioncpp">trunk/Source/JavaScriptCore/bytecode/PropertyCondition.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejitRepatchcpp">trunk/Source/JavaScriptCore/jit/Repatch.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorejsccpp">trunk/Source/JavaScriptCore/jsc.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSTypeInfoh">trunk/Source/JavaScriptCore/runtime/JSTypeInfo.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeStructureh">trunk/Source/JavaScriptCore/runtime/Structure.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm">trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptstestJSJSTestCustomNamedGetterh">trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptstestJSJSTestEventTargeth">trunk/Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h</a></li>
<li><a href="#trunkSourceWebCorebindingsscriptstestJSJSTestOverrideBuiltinsh">trunk/Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastdomNamedNodeMapnamedgettercachingexpectedtxt">trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastdomNamedNodeMapnamedgettercachinghtml">trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/LayoutTests/ChangeLog 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2015-11-20 Chris Dumez <cdumez@apple.com>
+
+ Caching of properties on objects that have named property getters is sometimes incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=151453
+ <rdar://problem/23049343>
+
+ Reviewed by Gavin Barraclough.
+
+ Add layout test to make sure caching does not cause NamedNodeMap's
+ named property getter to sometimes wrongly report a property as
+ missing.
+
+ * fast/dom/NamedNodeMap-named-getter-caching-expected.txt: Added.
+ * fast/dom/NamedNodeMap-named-getter-caching.html: Added.
+
</ins><span class="cx"> 2015-11-20 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Simple line layout: Add text-indent support.
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomNamedNodeMapnamedgettercachingexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching-expected.txt (0 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching-expected.txt 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+Tests caching of result of NamedNodeMap named property getter
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS bodyAttributes.class is undefined
+PASS lastIterationHasRightValue is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomNamedNodeMapnamedgettercachinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching.html (0 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching.html (rev 0)
+++ trunk/LayoutTests/fast/dom/NamedNodeMap-named-getter-caching.html 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -0,0 +1,28 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Tests caching of result of NamedNodeMap named property getter");
+
+var bodyAttributes = document.body.attributes;
+shouldBe("bodyAttributes.class", "undefined");
+
+var lastIterationHasRightValue = false;
+for (var i = 0; i < 1000; i++) {
+ if (i == 999)
+ document.body.setAttribute('class', 'test');
+ if (bodyAttributes.class != undefined) {
+ if (i == 999)
+ lastIterationHasRightValue = true;
+ else
+ testFailed("The body element should NOT have a 'class' attribute set");
+ }
+}
+
+shouldBeTrue("lastIterationHasRightValue");
+
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2015-11-20 Chris Dumez <cdumez@apple.com>
+
+ Caching of properties on objects that have named property getters is sometimes incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=151453
+ <rdar://problem/23049343>
+
+ Reviewed by Gavin Barraclough.
+
+ Add new GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag to be
+ used by objects that have a non-'OverrideBuiltins' named property getter.
+ This flag prevents caching of properties that are missing as a named
+ property with this name may later become available.
+
+ Objects with an 'OverrideBuiltins' named property getter will keep using
+ the GetOwnPropertySlotIsImpure TypeInfo flag, which prevents all property
+ caching since named properties can override own properties or properties
+ on the prototype.
+
+ * bytecode/ComplexGetStatus.cpp:
+ (JSC::ComplexGetStatus::computeFor):
+ * bytecode/PropertyCondition.cpp:
+ (JSC::PropertyCondition::isStillValid):
+ * jit/Repatch.cpp:
+ (JSC::tryCacheGetByID):
+ (JSC::tryRepatchIn):
+ * jsc.cpp:
+ * runtime/JSTypeInfo.h:
+ (JSC::TypeInfo::getOwnPropertySlotIsImpure):
+ (JSC::TypeInfo::getOwnPropertySlotIsImpureForPropertyAbsence):
+ (JSC::TypeInfo::prohibitsPropertyCaching): Deleted.
+ * runtime/Structure.h:
+
</ins><span class="cx"> 2015-11-19 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> REGRESSION(r189433) Web Inspector: JSContext inspection exceptions should include native call frames by default
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeComplexGetStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ComplexGetStatus.cpp (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ComplexGetStatus.cpp 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/bytecode/ComplexGetStatus.cpp 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -34,7 +34,7 @@
</span><span class="cx"> Structure* headStructure, const ObjectPropertyConditionSet& conditionSet, UniquedStringImpl* uid)
</span><span class="cx"> {
</span><span class="cx"> // FIXME: We should assert that we never see a structure that
</span><del>- // hasImpureGetOwnPropertySlot() but for which we don't
</del><ins>+ // getOwnPropertySlotIsImpure() but for which we don't
</ins><span class="cx"> // newImpurePropertyFiresWatchpoints(). We're not at a point where we can do
</span><span class="cx"> // that, yet.
</span><span class="cx"> // https://bugs.webkit.org/show_bug.cgi?id=131810
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodePropertyConditioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/PropertyCondition.cpp (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/PropertyCondition.cpp 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/bytecode/PropertyCondition.cpp 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -213,10 +213,13 @@
</span><span class="cx"> // "shadow" an existing JS property on the same object. Hence it affects both presence and
</span><span class="cx"> // absence. It doesn't affect AbsenceOfSetter because impure properties aren't ever setters.
</span><span class="cx"> switch (m_kind) {
</span><ins>+ case Absence:
+ if (structure->typeInfo().getOwnPropertySlotIsImpure() || structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
+ return false;
+ break;
</ins><span class="cx"> case Presence:
</span><del>- case Absence:
</del><span class="cx"> case Equivalence:
</span><del>- if (structure->typeInfo().hasImpureGetOwnPropertySlot())
</del><ins>+ if (structure->typeInfo().getOwnPropertySlotIsImpure())
</ins><span class="cx"> return false;
</span><span class="cx"> break;
</span><span class="cx"> default:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejitRepatchcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jit/Repatch.cpp (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jit/Repatch.cpp 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/jit/Repatch.cpp 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -283,6 +283,9 @@
</span><span class="cx"> if (structure->typeInfo().prohibitsPropertyCaching() || structure->isDictionary())
</span><span class="cx"> return GiveUpOnCache;
</span><span class="cx">
</span><ins>+ if (slot.isUnset() && structure->typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence())
+ return GiveUpOnCache;
+
</ins><span class="cx"> if (slot.isUnset()) {
</span><span class="cx"> conditionSet = generateConditionsForPropertyMiss(
</span><span class="cx"> vm, codeBlock, exec, structure, propertyName.impl());
</span><span class="lines">@@ -485,7 +488,7 @@
</span><span class="cx"> if (forceICFailure(exec))
</span><span class="cx"> return GiveUpOnCache;
</span><span class="cx">
</span><del>- if (!base->structure()->propertyAccessesAreCacheable())
</del><ins>+ if (!base->structure()->propertyAccessesAreCacheable() || (!wasFound && !base->structure()->propertyAccessesAreCacheableForAbsence()))
</ins><span class="cx"> return GiveUpOnCache;
</span><span class="cx">
</span><span class="cx"> if (wasFound) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorejsccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/jsc.cpp (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/jsc.cpp 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/jsc.cpp 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -267,7 +267,7 @@
</span><span class="cx">
</span><span class="cx"> DECLARE_INFO;
</span><span class="cx"> typedef JSNonFinalObject Base;
</span><del>- static const unsigned StructureFlags = Base::StructureFlags | JSC::HasImpureGetOwnPropertySlot | JSC::OverridesGetOwnPropertySlot;
</del><ins>+ static const unsigned StructureFlags = Base::StructureFlags | JSC::GetOwnPropertySlotIsImpure | JSC::OverridesGetOwnPropertySlot;
</ins><span class="cx">
</span><span class="cx"> static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSTypeInfoh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSTypeInfo.h (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSTypeInfo.h 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/runtime/JSTypeInfo.h 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -47,9 +47,10 @@
</span><span class="cx">
</span><span class="cx"> static const unsigned OverridesGetPropertyNames = 1 << 8;
</span><span class="cx"> static const unsigned ProhibitsPropertyCaching = 1 << 9;
</span><del>-static const unsigned HasImpureGetOwnPropertySlot = 1 << 10;
</del><ins>+static const unsigned GetOwnPropertySlotIsImpure = 1 << 10;
</ins><span class="cx"> static const unsigned NewImpurePropertyFiresWatchpoints = 1 << 11;
</span><span class="cx"> static const unsigned IsEnvironmentRecord = 1 << 12;
</span><ins>+static const unsigned GetOwnPropertySlotIsImpureForPropertyAbsence = 1 << 13;
</ins><span class="cx">
</span><span class="cx"> class TypeInfo {
</span><span class="cx"> public:
</span><span class="lines">@@ -91,7 +92,8 @@
</span><span class="cx"> bool structureIsImmortal() const { return isSetOnFlags1(StructureIsImmortal); }
</span><span class="cx"> bool overridesGetPropertyNames() const { return isSetOnFlags2(OverridesGetPropertyNames); }
</span><span class="cx"> bool prohibitsPropertyCaching() const { return isSetOnFlags2(ProhibitsPropertyCaching); }
</span><del>- bool hasImpureGetOwnPropertySlot() const { return isSetOnFlags2(HasImpureGetOwnPropertySlot); }
</del><ins>+ bool getOwnPropertySlotIsImpure() const { return isSetOnFlags2(GetOwnPropertySlotIsImpure); }
+ bool getOwnPropertySlotIsImpureForPropertyAbsence() const { return isSetOnFlags2(GetOwnPropertySlotIsImpureForPropertyAbsence); }
</ins><span class="cx"> bool newImpurePropertyFiresWatchpoints() const { return isSetOnFlags2(NewImpurePropertyFiresWatchpoints); }
</span><span class="cx"> bool isEnvironmentRecord() const { return isSetOnFlags2(IsEnvironmentRecord); }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeStructureh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Structure.h (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Structure.h 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/JavaScriptCore/runtime/Structure.h 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -204,13 +204,18 @@
</span><span class="cx"> {
</span><span class="cx"> return dictionaryKind() != UncachedDictionaryKind
</span><span class="cx"> && !typeInfo().prohibitsPropertyCaching()
</span><del>- && !(typeInfo().hasImpureGetOwnPropertySlot() && !typeInfo().newImpurePropertyFiresWatchpoints());
</del><ins>+ && !(typeInfo().getOwnPropertySlotIsImpure() && !typeInfo().newImpurePropertyFiresWatchpoints());
</ins><span class="cx"> }
</span><ins>+
+ bool propertyAccessesAreCacheableForAbsence()
+ {
+ return !typeInfo().getOwnPropertySlotIsImpureForPropertyAbsence();
+ }
</ins><span class="cx">
</span><span class="cx"> bool needImpurePropertyWatchpoint()
</span><span class="cx"> {
</span><span class="cx"> return propertyAccessesAreCacheable()
</span><del>- && typeInfo().hasImpureGetOwnPropertySlot()
</del><ins>+ && typeInfo().getOwnPropertySlotIsImpure()
</ins><span class="cx"> && typeInfo().newImpurePropertyFiresWatchpoints();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -218,7 +223,7 @@
</span><span class="cx"> // DFG from inlining property accesses since structures don't transition when a new impure property appears.
</span><span class="cx"> bool takesSlowPathInDFGForImpureProperty()
</span><span class="cx"> {
</span><del>- return typeInfo().hasImpureGetOwnPropertySlot();
</del><ins>+ return typeInfo().getOwnPropertySlotIsImpure();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Type accessors.
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/WebCore/ChangeLog 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2015-11-20 Chris Dumez <cdumez@apple.com>
+
+ Caching of properties on objects that have named property getters is sometimes incorrect
+ https://bugs.webkit.org/show_bug.cgi?id=151453
+ <rdar://problem/23049343>
+
+ Reviewed by Gavin Barraclough.
+
+ In r188590, we dropped the JSC::GetOwnPropertySlotIsImpure TypeInfo flag for
+ interfaces that have a non-'OverrideBuiltins' named property getter in order
+ to allow caching of properties returns by GetOwnPropertySlot(). We assumed
+ this was safe as it was no longer possible for named properties to override
+ own properties (or properties on the prototype).
+
+ However, there is an issue when we cache the non-existence of a property.
+ Even though at one point the property did not exist, a named property with
+ this name may later become available. In such case, caching would cause us
+ to wrongly report a property as missing.
+
+ To address the problem, this patch introduces a new
+ GetOwnPropertySlotIsImpureForPropertyAbsence TypeInfo flag and uses it for
+ interfaces that have a non-'OverrideBuiltins' named property getter. This
+ will cause us to not cache the fact that a property is missing on such
+ objects, while maintaining the performance win from r188590 in the common
+ case.
+
+ Test: fast/dom/NamedNodeMap-named-getter-caching.html
+
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (GenerateHeader):
+ * bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
+ * bindings/scripts/test/JS/JSTestEventTarget.h:
+ * bindings/scripts/test/JS/JSTestOverrideBuiltins.h:
+
</ins><span class="cx"> 2015-11-19 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Back-buffer to front-buffer copy fails for some buffer formats
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptsCodeGeneratorJSpm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -982,20 +982,23 @@
</span><span class="cx"> my $namedGetterFunction = GetNamedGetterFunction($interface);
</span><span class="cx"> my $indexedGetterFunction = GetIndexedGetterFunction($interface);
</span><span class="cx">
</span><del>- my $hasImpureNamedGetter = $interface->extendedAttributes->{"OverrideBuiltins"}
- && ($namedGetterFunction || $interface->extendedAttributes->{"CustomNamedGetter"});
</del><ins>+ my $hasNamedGetter = $namedGetterFunction
+ || $interface->extendedAttributes->{"CustomNamedGetter"};
</ins><span class="cx">
</span><span class="cx"> my $hasComplexGetter =
</span><span class="cx"> $indexedGetterFunction
</span><span class="cx"> || $interface->extendedAttributes->{"JSCustomGetOwnPropertySlotAndDescriptor"}
</span><span class="cx"> || $interface->extendedAttributes->{"CustomGetOwnPropertySlot"}
</span><del>- || $namedGetterFunction
- || $interface->extendedAttributes->{"CustomNamedGetter"};
</del><ins>+ || $hasNamedGetter;
</ins><span class="cx">
</span><span class="cx"> my $hasGetter = InstanceOverridesGetOwnPropertySlot($interface);
</span><span class="cx">
</span><del>- if ($hasImpureNamedGetter) {
- $structureFlags{"JSC::HasImpureGetOwnPropertySlot"} = 1;
</del><ins>+ if ($hasNamedGetter) {
+ if ($interface->extendedAttributes->{"OverrideBuiltins"}) {
+ $structureFlags{"JSC::GetOwnPropertySlotIsImpure"} = 1;
+ } else {
+ $structureFlags{"JSC::GetOwnPropertySlotIsImpureForPropertyAbsence"} = 1;
+ }
</ins><span class="cx"> }
</span><span class="cx"> if ($interface->extendedAttributes->{"NewImpurePropertyFiresWatchpoints"}) {
</span><span class="cx"> $structureFlags{"JSC::NewImpurePropertyFiresWatchpoints"} = 1;
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptstestJSJSTestCustomNamedGetterh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -53,7 +53,7 @@
</span><span class="cx">
</span><span class="cx"> static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
</span><span class="cx"> public:
</span><del>- static const unsigned StructureFlags = JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | Base::StructureFlags;
</del><ins>+ static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpureForPropertyAbsence | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | Base::StructureFlags;
</ins><span class="cx"> protected:
</span><span class="cx"> JSTestCustomNamedGetter(JSC::Structure*, JSDOMGlobalObject&, Ref<TestCustomNamedGetter>&&);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptstestJSJSTestEventTargeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -57,7 +57,7 @@
</span><span class="cx"> static void visitChildren(JSCell*, JSC::SlotVisitor&);
</span><span class="cx">
</span><span class="cx"> public:
</span><del>- static const unsigned StructureFlags = JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::MasqueradesAsUndefined | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
</del><ins>+ static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpureForPropertyAbsence | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::MasqueradesAsUndefined | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
</ins><span class="cx"> protected:
</span><span class="cx"> JSTestEventTarget(JSC::Structure*, JSDOMGlobalObject&, Ref<TestEventTarget>&&);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsscriptstestJSJSTestOverrideBuiltinsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.h (192692 => 192693)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.h 2015-11-20 20:40:26 UTC (rev 192692)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.h 2015-11-20 20:45:12 UTC (rev 192693)
</span><span class="lines">@@ -54,7 +54,7 @@
</span><span class="cx"> static void getOwnPropertyNames(JSC::JSObject*, JSC::ExecState*, JSC::PropertyNameArray&, JSC::EnumerationMode = JSC::EnumerationMode());
</span><span class="cx"> static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
</span><span class="cx"> public:
</span><del>- static const unsigned StructureFlags = JSC::HasImpureGetOwnPropertySlot | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
</del><ins>+ static const unsigned StructureFlags = JSC::GetOwnPropertySlotIsImpure | JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | JSC::OverridesGetOwnPropertySlot | JSC::OverridesGetPropertyNames | Base::StructureFlags;
</ins><span class="cx"> protected:
</span><span class="cx"> JSTestOverrideBuiltins(JSC::Structure*, JSDOMGlobalObject&, Ref<TestOverrideBuiltins>&&);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>