Global properties that use LocalStorage are not correctly listed as enumerable.
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Mar 2008 02:36:34 +0000 (02:36 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Mar 2008 02:36:34 +0000 (02:36 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31225 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/JavaScriptCore.exp
JavaScriptCore/kjs/JSVariableObject.cpp
JavaScriptCore/kjs/JSVariableObject.h
JavaScriptCore/kjs/object.h
LayoutTests/ChangeLog
LayoutTests/fast/js/propertyIsEnumerable-expected.txt
LayoutTests/fast/js/resources/propertyIsEnumerable.js

index 5346c5c..8c31e3a 100644 (file)
@@ -1,3 +1,21 @@
+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>
 
         Webkit does not build on linux powerpc
index e02d26f..40609b5 100644 (file)
@@ -229,6 +229,7 @@ __ZNK3KJS14JSGlobalObject12saveBuiltinsERNS_13SavedBuiltinsE
 __ZNK3KJS14JSGlobalObject14isDynamicScopeEv
 __ZNK3KJS16JSVariableObject16isVariableObjectEv
 __ZNK3KJS16JSVariableObject16saveLocalStorageERNS_15SavedPropertiesE
+__ZNK3KJS16JSVariableObject21getPropertyAttributesERKNS_10IdentifierERj
 __ZNK3KJS19InternalFunctionImp14implementsCallEv
 __ZNK3KJS19InternalFunctionImp21implementsHasInstanceEv
 __ZNK3KJS4List8getSliceEiRS0_
@@ -255,6 +256,7 @@ __ZNK3KJS8JSObject11hasPropertyEPNS_9ExecStateEj
 __ZNK3KJS8JSObject12defaultValueEPNS_9ExecStateENS_6JSTypeE
 __ZNK3KJS8JSObject14implementsCallEv
 __ZNK3KJS8JSObject19implementsConstructEv
+__ZNK3KJS8JSObject21getPropertyAttributesERKNS_10IdentifierERj
 __ZNK3KJS8JSObject21implementsHasInstanceEv
 __ZNK3KJS8JSObject3getEPNS_9ExecStateERKNS_10IdentifierE
 __ZNK3KJS8JSObject3getEPNS_9ExecStateEj
index 55779a8..16cfa60 100644 (file)
@@ -84,13 +84,24 @@ bool JSVariableObject::deleteProperty(ExecState* exec, const Identifier& propert
 
 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);
 }
 
+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);
+}
+
 void JSVariableObject::mark()
 {
     JSObject::mark();
index 947b81e..b00448a 100644 (file)
@@ -37,8 +37,8 @@ namespace KJS {
 
     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;
         void restoreLocalStorage(const SavedProperties&);
@@ -53,6 +53,8 @@ namespace KJS {
         virtual bool isVariableObject() const;
         virtual bool isDynamicScope() const = 0;
 
+        virtual bool getPropertyAttributes(const Identifier& propertyName, unsigned& attributes) const;
+
     protected:
         // Subclasses of JSVariableObject can subclass this struct to add data
         // without increasing their own size (since there's a hard limit on the
index bd45a4e..8227a7c 100644 (file)
@@ -403,7 +403,7 @@ namespace KJS {
     virtual UString toString(ExecState *exec) const;
     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
     virtual bool masqueradeAsUndefined() const { return false; }
index 7d2c74c..b6eac30 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Reviewed by Oliver Hunt.
index 08db3d5..2e59e50 100644 (file)
@@ -6,6 +6,11 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 PASS a.propertyIsEnumerable('length') is false
 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
 
 TEST COMPLETE
index 7d92cae..a3cd032 100644 (file)
@@ -5,8 +5,17 @@ description(
 a = new Array();
 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;