Title: [147570] trunk
Revision
147570
Author
mhahnenb...@apple.com
Date
2013-04-03 11:38:33 -0700 (Wed, 03 Apr 2013)

Log Message

get_by_pname can become confused when iterating over objects with static properties
https://bugs.webkit.org/show_bug.cgi?id=113831

Reviewed by Geoffrey Garen.

get_by_pname doesn't take static properties into account when using a JSPropertyNameIterator to directly 
access an object's backing store. One way to fix this is to not cache any properties when iterating over 
objects with static properties. This patch fixes the bug that was originally reported on swisscom.ch.

Source/_javascript_Core: 

* runtime/JSObject.cpp:
(JSC::JSObject::getOwnNonIndexPropertyNames):
* runtime/JSPropertyNameIterator.cpp:
(JSC::JSPropertyNameIterator::create):
* runtime/PropertyNameArray.h:
(JSC::PropertyNameArray::PropertyNameArray):
(JSC::PropertyNameArray::numCacheableSlots):
(JSC::PropertyNameArray::setNumCacheableSlots):
(PropertyNameArray):

LayoutTests: 

* fast/js/dom-static-property-for-in-iteration-expected.txt: Added.
* fast/js/dom-static-property-for-in-iteration.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (147569 => 147570)


--- trunk/LayoutTests/ChangeLog	2013-04-03 18:27:53 UTC (rev 147569)
+++ trunk/LayoutTests/ChangeLog	2013-04-03 18:38:33 UTC (rev 147570)
@@ -1,3 +1,17 @@
+2013-04-02  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        get_by_pname can become confused when iterating over objects with static properties
+        https://bugs.webkit.org/show_bug.cgi?id=113831
+
+        Reviewed by Geoffrey Garen.
+
+        get_by_pname doesn't take static properties into account when using a JSPropertyNameIterator to directly 
+        access an object's backing store. One way to fix this is to not cache any properties when iterating over 
+        objects with static properties. This patch fixes the bug that was originally reported on swisscom.ch.
+
+        * fast/js/dom-static-property-for-in-iteration-expected.txt: Added.
+        * fast/js/dom-static-property-for-in-iteration.html: Added.
+
 2013-04-03  Felipe Zimmerle  <fel...@zimmerle.org>
 
         CSP blocks inline style when cloning a node

Added: trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt (0 => 147570)


--- trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt	2013-04-03 18:38:33 UTC (rev 147570)
@@ -0,0 +1,113 @@
+Checks that get_by_pname doesn't get confused about which properties go where when it comes to stacic properties.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 'file://' is 'file://'
+PASS '' is ''
+PASS 'file:///Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/bar' is 'file:///Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/bar'
+PASS '' is ''
+PASS 'nerget' is 'nerget'
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '/Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/bar' is '/Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/bar'
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS '' is ''
+PASS 'file:' is 'file:'
+PASS '<a id="foo" href="" is '<a id="foo" href=""
+PASS 'true' is 'true'
+PASS '' is ''
+PASS 'foo' is 'foo'
+PASS '' is ''
+PASS '' is ''
+PASS 'true' is 'true'
+PASS 'false' is 'false'
+PASS 'nerget' is 'nerget'
+PASS '' is ''
+PASS 'nerget' is 'nerget'
+PASS 'inherit' is 'inherit'
+PASS '0' is '0'
+PASS 'true' is 'true'
+PASS 'nerget' is 'nerget'
+PASS '' is ''
+PASS '[object HTMLCollection]' is '[object HTMLCollection]'
+PASS 'false' is 'false'
+PASS '[object CSSStyleDeclaration]' is '[object CSSStyleDeclaration]'
+PASS '[object DOMStringMap]' is '[object DOMStringMap]'
+PASS '0' is '0'
+PASS '0' is '0'
+PASS '[object NamedNodeMap]' is '[object NamedNodeMap]'
+PASS 'undefined' is 'undefined'
+PASS '39' is '39'
+PASS '' is ''
+PASS '8' is '8'
+PASS '' is ''
+PASS '0' is '0'
+PASS 'null' is 'null'
+PASS '[object HTMLBodyElement]' is '[object HTMLBodyElement]'
+PASS '[object HTMLScriptElement]' is '[object HTMLScriptElement]'
+PASS 'A' is 'A'
+PASS '[object HTMLDivElement]' is '[object HTMLDivElement]'
+PASS '0' is '0'
+PASS '0' is '0'
+PASS 'null' is 'null'
+PASS '0' is '0'
+PASS '18' is '18'
+PASS '0' is '0'
+PASS '1014' is '1014'
+PASS '0' is '0'
+PASS '0' is '0'
+PASS '[object Text]' is '[object Text]'
+PASS '[object HTMLBodyElement]' is '[object HTMLBodyElement]'
+PASS '[object Text]' is '[object Text]'
+PASS 'file:///Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/dom-static-property-for-in-iteration.html' is 'file:///Volumes/Data/WebKit-svn-01/OpenSource/LayoutTests/fast/js/dom-static-property-for-in-iteration.html'
+PASS '[object Text]' is '[object Text]'
+PASS 'null' is 'null'
+PASS 'nerget' is 'nerget'
+PASS '1' is '1'
+PASS 'A' is 'A'
+PASS 'null' is 'null'
+PASS '[object NodeList]' is '[object NodeList]'
+PASS '[object Text]' is '[object Text]'
+PASS '[object HTMLDocument]' is '[object HTMLDocument]'
+PASS 'http://www.w3.org/1999/xhtml' is 'http://www.w3.org/1999/xhtml'
+PASS 'a' is 'a'
+PASS '[object HTMLBodyElement]' is '[object HTMLBodyElement]'
+PASS '1' is '1'
+PASS '2' is '2'
+PASS '3' is '3'
+PASS '4' is '4'
+PASS '5' is '5'
+PASS '6' is '6'
+PASS '1' is '1'
+PASS '12' is '12'
+PASS '4' is '4'
+PASS '1' is '1'
+PASS '1' is '1'
+PASS '6' is '6'
+PASS '3' is '3'
+PASS '5' is '5'
+PASS '32' is '32'
+PASS '11' is '11'
+PASS '7' is '7'
+PASS '2' is '2'
+PASS '10' is '10'
+PASS '8' is '8'
+PASS '4' is '4'
+PASS '2' is '2'
+PASS '16' is '16'
+PASS '9' is '9'
+PASS '8' is '8'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+nerget

Added: trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration.html (0 => 147570)


--- trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration.html	                        (rev 0)
+++ trunk/LayoutTests/fast/js/dom-static-property-for-in-iteration.html	2013-04-03 18:38:33 UTC (rev 147570)
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Test</title>
+<script src=""
+</head>
+<body>
+    <a id="foo" href=""
+    <script>
+        description(
+        "Checks that get_by_pname doesn't get confused about which properties go where when it comes to stacic properties."
+        );
+
+        function f(a) {
+            for (var i in a) {
+                var actual = a[i];
+                var expected = a["" + i];
+                // Function toString causes eval to choke.
+                if (typeof expected === "function")
+                    continue;
+                shouldBe("'" + a[i] + "'", "'" + expected + "'");
+            }
+        }
+        var g = document.getElementById("foo");
+        g.foo="1";
+        g.bar="2";
+        g.wibble="3";
+        g.hick="4";
+        g.hock="5";
+        g.snood="6";
+        f(g);
+    </script>
+    <script src=""
+</body>
+</html>

Modified: trunk/Source/_javascript_Core/ChangeLog (147569 => 147570)


--- trunk/Source/_javascript_Core/ChangeLog	2013-04-03 18:27:53 UTC (rev 147569)
+++ trunk/Source/_javascript_Core/ChangeLog	2013-04-03 18:38:33 UTC (rev 147570)
@@ -1,3 +1,24 @@
+2013-04-02  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        get_by_pname can become confused when iterating over objects with static properties
+        https://bugs.webkit.org/show_bug.cgi?id=113831
+
+        Reviewed by Geoffrey Garen.
+
+        get_by_pname doesn't take static properties into account when using a JSPropertyNameIterator to directly 
+        access an object's backing store. One way to fix this is to not cache any properties when iterating over 
+        objects with static properties. This patch fixes the bug that was originally reported on swisscom.ch.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::getOwnNonIndexPropertyNames):
+        * runtime/JSPropertyNameIterator.cpp:
+        (JSC::JSPropertyNameIterator::create):
+        * runtime/PropertyNameArray.h:
+        (JSC::PropertyNameArray::PropertyNameArray):
+        (JSC::PropertyNameArray::numCacheableSlots):
+        (JSC::PropertyNameArray::setNumCacheableSlots):
+        (PropertyNameArray):
+
 2013-04-02  Geoffrey Garen  <gga...@apple.com>
 
         DFG should compile a little sooner

Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (147569 => 147570)


--- trunk/Source/_javascript_Core/runtime/JSObject.cpp	2013-04-03 18:27:53 UTC (rev 147569)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp	2013-04-03 18:38:33 UTC (rev 147570)
@@ -1532,7 +1532,10 @@
 void JSObject::getOwnNonIndexPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNames, EnumerationMode mode)
 {
     getClassPropertyNames(exec, object->classInfo(), propertyNames, mode, object->staticFunctionsReified());
+    size_t preStructurePropertyNamesCount = propertyNames.size();
     object->structure()->getPropertyNamesFromStructure(exec->globalData(), propertyNames, mode);
+    size_t numCacheableSlots = preStructurePropertyNamesCount ? 0 : propertyNames.size();
+    propertyNames.setNumCacheableSlots(numCacheableSlots);
 }
 
 double JSObject::toNumber(ExecState* exec) const

Modified: trunk/Source/_javascript_Core/runtime/JSPropertyNameIterator.cpp (147569 => 147570)


--- trunk/Source/_javascript_Core/runtime/JSPropertyNameIterator.cpp	2013-04-03 18:27:53 UTC (rev 147569)
+++ trunk/Source/_javascript_Core/runtime/JSPropertyNameIterator.cpp	2013-04-03 18:38:33 UTC (rev 147570)
@@ -54,7 +54,7 @@
     size_t numCacheableSlots = 0;
     if (!o->structure()->hasNonEnumerableProperties() && !o->structure()->hasGetterSetterProperties()
         && !o->structure()->isUncacheableDictionary() && !o->structure()->typeInfo().overridesGetPropertyNames())
-        numCacheableSlots = o->structure()->totalStorageSize();
+        numCacheableSlots = propertyNames.numCacheableSlots();
     
     JSPropertyNameIterator* jsPropertyNameIterator = new (NotNull, allocateCell<JSPropertyNameIterator>(*exec->heap())) JSPropertyNameIterator(exec, propertyNames.data(), numCacheableSlots);
     jsPropertyNameIterator->finishCreation(exec, propertyNames.data(), o);

Modified: trunk/Source/_javascript_Core/runtime/PropertyNameArray.h (147569 => 147570)


--- trunk/Source/_javascript_Core/runtime/PropertyNameArray.h	2013-04-03 18:27:53 UTC (rev 147569)
+++ trunk/Source/_javascript_Core/runtime/PropertyNameArray.h	2013-04-03 18:38:33 UTC (rev 147570)
@@ -55,12 +55,14 @@
         PropertyNameArray(JSGlobalData* globalData)
             : m_data(PropertyNameArrayData::create())
             , m_globalData(globalData)
+            , m_numCacheableSlots(0)
         {
         }
 
         PropertyNameArray(ExecState* exec)
             : m_data(PropertyNameArrayData::create())
             , m_globalData(&exec->globalData())
+            , m_numCacheableSlots(0)
         {
         }
 
@@ -83,12 +85,16 @@
         const_iterator begin() const { return m_data->propertyNameVector().begin(); }
         const_iterator end() const { return m_data->propertyNameVector().end(); }
 
+        size_t numCacheableSlots() const { return m_numCacheableSlots; }
+        void setNumCacheableSlots(size_t numCacheableSlots) { m_numCacheableSlots = numCacheableSlots; }
+
     private:
         typedef HashSet<StringImpl*, PtrHash<StringImpl*> > IdentifierSet;
 
         RefPtr<PropertyNameArrayData> m_data;
         IdentifierSet m_set;
         JSGlobalData* m_globalData;
+        size_t m_numCacheableSlots;
     };
 
 } // namespace JSC
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to