Changeset 31225

Timestamp:

03/21/08 19:36:34 (8 months ago)

Author:

oliver@apple.com

Message:

Global properties that use LocalStorage are not correctly listed as enumerable.

Reviewed by Geoff Garen

The problem was caused by JSObject::getPropertyAttributes not being aware
of the JSVariableObject SymbolTable. The fix is to make getPropertyAttributes
virtual and override in JSVariableObject. This does not produce any performance
regression.

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/JavaScriptCore.exp (modified) (2 diffs)
• JavaScriptCore/kjs/JSVariableObject.cpp (modified) (1 diff)
• JavaScriptCore/kjs/JSVariableObject.h (modified) (2 diffs)
• JavaScriptCore/kjs/object.h (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/propertyIsEnumerable-expected.txt (modified) (1 diff)
• LayoutTests/fast/js/resources/propertyIsEnumerable.js (modified) (1 diff)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-03-21  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Global properties that use LocalStorage are not correctly listed as enumerable.
+
+        The problem was caused by JSObject::getPropertyAttributes not being aware
+        of the JSVariableObject SymbolTable.  The fix is to make getPropertyAttributes
+        virtual and override in JSVariableObject.  This does not produce any performance
+        regression.
+
+        * JavaScriptCore.exp:
+        * kjs/JSVariableObject.cpp:
+        (KJS::JSVariableObject::getPropertyNames):
+        (KJS::JSVariableObject::getPropertyAttributes):
+        * kjs/JSVariableObject.h:
+        * kjs/object.h:
+
 2008-03-21  Arkadiusz Miskiewicz  <arekm@maven.pl>
 

trunk/JavaScriptCore/JavaScriptCore.exp

@@ -230 +230 @@
 __ZNK3KJS16JSVariableObject16isVariableObjectEv
 __ZNK3KJS16JSVariableObject16saveLocalStorageERNS_15SavedPropertiesE
+__ZNK3KJS16JSVariableObject21getPropertyAttributesERKNS_10IdentifierERj
 __ZNK3KJS19InternalFunctionImp14implementsCallEv
 __ZNK3KJS19InternalFunctionImp21implementsHasInstanceEv
@@ -256 +257 @@
 __ZNK3KJS8JSObject14implementsCallEv
 __ZNK3KJS8JSObject19implementsConstructEv
+__ZNK3KJS8JSObject21getPropertyAttributesERKNS_10IdentifierERj
 __ZNK3KJS8JSObject21implementsHasInstanceEv
 __ZNK3KJS8JSObject3getEPNS_9ExecStateERKNS_10IdentifierE

trunk/JavaScriptCore/kjs/JSVariableObject.cpp

@@ -85 +85 @@
 void JSVariableObject::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames)
 {
-    SymbolTable::const_iterator::Keys end = symbolTable().end().keys();
-    for (SymbolTable::const_iterator::Keys it = symbolTable().begin().keys(); it != end; ++it)
-        propertyNames.add(Identifier(it->get()));
+    SymbolTable::const_iterator end = symbolTable().end();
+    for (SymbolTable::const_iterator it = symbolTable().begin(); it != end; ++it)
+        if ((localStorage()[it->second].attributes & DontEnum) == 0)
+            propertyNames.add(Identifier(it->first.get()));
+    
+    JSObject::getPropertyNames(exec, propertyNames);
+}
 
-    JSObject::getPropertyNames(exec, propertyNames);
+bool JSVariableObject::getPropertyAttributes(const Identifier& propertyName, unsigned& attributes) const
+{
+    size_t index = symbolTable().get(propertyName.ustring().rep());
+    if (index != missingSymbolMarker()) {
+        attributes = localStorage()[index].attributes;
+        return true;
+    }
+    return JSObject::getPropertyAttributes(propertyName, attributes);
 }
 

trunk/JavaScriptCore/kjs/JSVariableObject.h

@@ -38 +38 @@
     class JSVariableObject : public JSObject {
     public:
-        SymbolTable& symbolTable() { return *d->symbolTable; }
-        LocalStorage& localStorage() { return d->localStorage; }
+        SymbolTable& symbolTable() const { return *d->symbolTable; }
+        LocalStorage& localStorage() const { return d->localStorage; }
         
         void saveLocalStorage(SavedProperties&) const;
@@ -53 +53 @@
         virtual bool isVariableObject() const;
         virtual bool isDynamicScope() const = 0;
+
+        virtual bool getPropertyAttributes(const Identifier& propertyName, unsigned& attributes) const;
 
     protected:

trunk/JavaScriptCore/kjs/object.h

@@ -404 +404 @@
     virtual JSObject *toObject(ExecState *exec) const;
     
-    bool getPropertyAttributes(const Identifier& propertyName, unsigned& attributes) const;
+    virtual bool getPropertyAttributes(const Identifier& propertyName, unsigned& attributes) const;
     
     // WebCore uses this to make document.all and style.filter undetectable

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-03-21  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Geoff Garen.
+
+        Ensure we correctly report optimised variables as being
+        enumerable.
+
+        * fast/js/propertyIsEnumerable-expected.txt:
+        * fast/js/resources/propertyIsEnumerable.js:
+
 2008-03-21  Sam Weinig  <sam@webkit.org>
 

trunk/LayoutTests/fast/js/propertyIsEnumerable-expected.txt

@@ -7 +7 @@
 PASS a.propertyIsEnumerable ('foo') is true
 PASS a.propertyIsEnumerable ('non-existant') is false
+PASS global.propertyIsEnumerable ('aVarDecl') is true
+PASS global.propertyIsEnumerable ('aFunctionDecl') is true
+PASS global.propertyIsEnumerable ('Math') is false
+PASS global.propertyIsEnumerable ('NaN') is false
+PASS global.propertyIsEnumerable ('undefined') is false
 PASS successfullyParsed is true
 

trunk/LayoutTests/fast/js/resources/propertyIsEnumerable.js

@@ -6 +6 @@
 a.foo='bar'
 
+var aVarDecl;
+function aFunctionDecl(){}
+var global = this;
 shouldBeFalse("a.propertyIsEnumerable('length')");
 shouldBeTrue("a.propertyIsEnumerable ('foo')");
 shouldBeFalse("a.propertyIsEnumerable ('non-existant')");
 
+shouldBeTrue("global.propertyIsEnumerable ('aVarDecl')");
+shouldBeTrue("global.propertyIsEnumerable ('aFunctionDecl')");
+shouldBeFalse("global.propertyIsEnumerable ('Math')");
+shouldBeFalse("global.propertyIsEnumerable ('NaN')");
+shouldBeFalse("global.propertyIsEnumerable ('undefined')");
+
 var successfullyParsed = true;