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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@31225 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 5346c5c..8c31e3a 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -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
diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index e02d26f..40609b5 100644
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -229,6 +229,7 @@
__ZNK3KJS14JSGlobalObject14isDynamicScopeEv
__ZNK3KJS16JSVariableObject16isVariableObjectEv
__ZNK3KJS16JSVariableObject16saveLocalStorageERNS_15SavedPropertiesE
+__ZNK3KJS16JSVariableObject21getPropertyAttributesERKNS_10IdentifierERj
__ZNK3KJS19InternalFunctionImp14implementsCallEv
__ZNK3KJS19InternalFunctionImp21implementsHasInstanceEv
__ZNK3KJS4List8getSliceEiRS0_
@@ -255,6 +256,7 @@
__ZNK3KJS8JSObject12defaultValueEPNS_9ExecStateENS_6JSTypeE
__ZNK3KJS8JSObject14implementsCallEv
__ZNK3KJS8JSObject19implementsConstructEv
+__ZNK3KJS8JSObject21getPropertyAttributesERKNS_10IdentifierERj
__ZNK3KJS8JSObject21implementsHasInstanceEv
__ZNK3KJS8JSObject3getEPNS_9ExecStateERKNS_10IdentifierE
__ZNK3KJS8JSObject3getEPNS_9ExecStateEj
diff --git a/JavaScriptCore/kjs/JSVariableObject.cpp b/JavaScriptCore/kjs/JSVariableObject.cpp
index 55779a8..16cfa60 100644
--- a/JavaScriptCore/kjs/JSVariableObject.cpp
+++ b/JavaScriptCore/kjs/JSVariableObject.cpp
@@ -84,13 +84,24 @@
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();
diff --git a/JavaScriptCore/kjs/JSVariableObject.h b/JavaScriptCore/kjs/JSVariableObject.h
index 947b81e..b00448a 100644
--- a/JavaScriptCore/kjs/JSVariableObject.h
+++ b/JavaScriptCore/kjs/JSVariableObject.h
@@ -37,8 +37,8 @@
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 @@
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
diff --git a/JavaScriptCore/kjs/object.h b/JavaScriptCore/kjs/object.h
index bd45a4e..8227a7c 100644
--- a/JavaScriptCore/kjs/object.h
+++ b/JavaScriptCore/kjs/object.h
@@ -403,7 +403,7 @@
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; }
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 7d2c74c..b6eac30 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -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.
diff --git a/LayoutTests/fast/js/propertyIsEnumerable-expected.txt b/LayoutTests/fast/js/propertyIsEnumerable-expected.txt
index 08db3d5..2e59e50 100644
--- a/LayoutTests/fast/js/propertyIsEnumerable-expected.txt
+++ b/LayoutTests/fast/js/propertyIsEnumerable-expected.txt
@@ -6,6 +6,11 @@
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
diff --git a/LayoutTests/fast/js/resources/propertyIsEnumerable.js b/LayoutTests/fast/js/resources/propertyIsEnumerable.js
index 7d92cae..a3cd032 100644
--- a/LayoutTests/fast/js/resources/propertyIsEnumerable.js
+++ b/LayoutTests/fast/js/resources/propertyIsEnumerable.js
@@ -5,8 +5,17 @@
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;
