JavaScriptCore:

        Reviewed by Sam.

        - fix http://bugs.webkit.org/show_bug.cgi?id=15927
          REGRESSION(r27487): delete a.c followed by __defineGetter__("c", ...) incorrectly deletes another property
          and <rdar://problem/5586384> REGRESSION (r27487): Can't switch out of Edit HTML Source mode on Leopard Wiki

        Test: fast/js/delete-then-put.html

        * kjs/property_map.cpp:
        (KJS::PropertyMap::put): Added a missing "- 1"; code to find an empty slot was not working.
        (KJS::PropertyMap::checkConsistency): Added a missing range check that would have caught this
        problem before.

        - roll out a last-minute change to my evaluateToBoolean patch that was incorrect.

        * kjs/nodes.h: (KJS::ExprStatementNode::ExprStatementNode): Take out call to
        optimizeForUnnecessaryResult, since the result is used in some cases.

LayoutTests:

        Reviewed by Sam.

        - test for http://bugs.webkit.org/show_bug.cgi?id=15927
          delete a.c followed by __defineGetter__("c", ...) incorrectly deletes another property

        * fast/js/delete-then-put-expected.txt: Added.
        * fast/js/delete-then-put.html: Added.
        * fast/js/resources/delete-then-put.js: Added.

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index a73740a25b97e6fb7495f6f1b82653071e975423..6ccb323c5d71f3588f6f6f046229b1d2978a63e9 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,23 @@
+2007-11-10  Darin Adler  <darin@apple.com>
+
+        Reviewed by Sam.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=15927
+          REGRESSION(r27487): delete a.c followed by __defineGetter__("c", ...) incorrectly deletes another property
+          and <rdar://problem/5586384> REGRESSION (r27487): Can't switch out of Edit HTML Source mode on Leopard Wiki
+
+        Test: fast/js/delete-then-put.html
+
+        * kjs/property_map.cpp:
+        (KJS::PropertyMap::put): Added a missing "- 1"; code to find an empty slot was not working.
+        (KJS::PropertyMap::checkConsistency): Added a missing range check that would have caught this
+        problem before.
+
+        - roll out a last-minute change to my evaluateToBoolean patch that was incorrect.
+
+        * kjs/nodes.h: (KJS::ExprStatementNode::ExprStatementNode): Take out call to
+        optimizeForUnnecessaryResult, since the result is used in some cases.
+
 2007-11-10  Adam Roben  <aroben@apple.com>
 
         Windows build fix

diff --git a/JavaScriptCore/kjs/nodes.h b/JavaScriptCore/kjs/nodes.h
index 0d07148d7011ada8ca6804a5ac8426187ec5ea22..bded3908ed08a15efeca8956ca5f08fd4d96e547 100644 (file)
--- a/JavaScriptCore/kjs/nodes.h
+++ b/JavaScriptCore/kjs/nodes.h
@@ -1576,10 +1576,7 @@ namespace KJS {
 
   class ExprStatementNode : public StatementNode {
   public:
-    ExprStatementNode(ExpressionNode* e) KJS_FAST_CALL : expr(e)
-    {
-        e->optimizeForUnnecessaryResult();
-    }
+    ExprStatementNode(ExpressionNode* e) KJS_FAST_CALL : expr(e) { }
     virtual void optimizeVariableAccess(FunctionBodyNode*, DeclarationStacks::NodeStack&) KJS_FAST_CALL;
     virtual Completion execute(ExecState*) KJS_FAST_CALL;
     virtual void streamTo(SourceStream&) const KJS_FAST_CALL;

diff --git a/JavaScriptCore/kjs/property_map.cpp b/JavaScriptCore/kjs/property_map.cpp
index 9eede480e8610b8e52463d6e741ac7f10648e4bc..4bd847c2493c28817822ddd9860c763652a8e93c 100644 (file)
--- a/JavaScriptCore/kjs/property_map.cpp
+++ b/JavaScriptCore/kjs/property_map.cpp
@@ -426,7 +426,7 @@ void PropertyMap::put(const Identifier& name, JSValue* value, unsigned attribute
         // the end that we were planning on using, so search backwards for the empty
         // slot that we can use. We know it will be there because we did at least one
         // deletion in the past that left an entry empty.
-        while (m_u.table->entries()[--entryIndex].key)
+        while (m_u.table->entries()[--entryIndex - 1].key)
             ;
     }
 
@@ -819,20 +819,20 @@ void PropertyMap::checkConsistency()
             ++deletedIndexCount;
             continue;
         }
+        ASSERT(entryIndex > deletedSentinelIndex);
+        ASSERT(entryIndex - 1 <= m_u.table->keyCount + m_u.table->deletedSentinelCount);
         ++indexCount;
 
         for (unsigned b = a + 1; b != m_u.table->size; ++b)
             ASSERT(m_u.table->entryIndicies[b] != entryIndex);
-
     }
     ASSERT(indexCount == m_u.table->keyCount);
     ASSERT(deletedIndexCount == m_u.table->deletedSentinelCount);
 
     ASSERT(m_u.table->entries()[0].key == 0);
 
-    unsigned entryCount = m_u.table->keyCount + m_u.table->deletedSentinelCount;
     unsigned nonEmptyEntryCount = 0;
-    for (unsigned c = 1; c <= entryCount; ++c) {
+    for (unsigned c = 1; c <= m_u.table->keyCount + m_u.table->deletedSentinelCount; ++c) {
         UString::Rep* rep = m_u.table->entries()[c].key;
         if (!rep) {
             ASSERT(m_u.table->entries()[c].value->isUndefined());

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 61ddbf6c6139b4808910e1add0b264c6a7a149df..2bcde6b6a2c4cb8e83c53bd8a66e6d5ff6787225 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2007-11-10  Darin Adler  <darin@apple.com>
+
+        Reviewed by Sam.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=15927
+          delete a.c followed by __defineGetter__("c", ...) incorrectly deletes another property
+
+        * fast/js/delete-then-put-expected.txt: Added.
+        * fast/js/delete-then-put.html: Added.
+        * fast/js/resources/delete-then-put.js: Added.
+
 2007-11-10  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Tim Hatcher.

diff --git a/LayoutTests/fast/js/delete-then-put-expected.txt b/LayoutTests/fast/js/delete-then-put-expected.txt
new file mode 100644 (file)
index 0000000..f8a7f7f
--- /dev/null
+++ b/LayoutTests/fast/js/delete-then-put-expected.txt
@@ -0,0 +1,15 @@
+This tests for a problem with put after delete that existed at one point in the past.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS props(a) is 'a,b,c,d,e'
+delete a.c
+PASS props(a) is 'a,b,d,e'
+define getter named c
+PASS props(a) is 'a,b,d,e,c'
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

diff --git a/LayoutTests/fast/js/delete-then-put.html b/LayoutTests/fast/js/delete-then-put.html
new file mode 100644 (file)
index 0000000..77a98d4
--- /dev/null
+++ b/LayoutTests/fast/js/delete-then-put.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/delete-then-put.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>

diff --git a/LayoutTests/fast/js/resources/delete-then-put.js b/LayoutTests/fast/js/resources/delete-then-put.js
new file mode 100644 (file)
index 0000000..6913268
--- /dev/null
+++ b/LayoutTests/fast/js/resources/delete-then-put.js
@@ -0,0 +1,27 @@
+description(
+'This tests for a problem with put after delete that existed at one point in the past.'
+);
+
+function props(o)
+{
+    var s = "";
+    for (p in o) {
+        if (s.length != 0)
+            s += ",";
+        s += p;
+    }
+    return s;
+}
+
+var a = { a:1, b:2, c:3, d:4, e:5 }
+
+shouldBe("props(a)", "'a,b,c,d,e'");
+debug("delete a.c");
+delete a.c;
+shouldBe("props(a)", "'a,b,d,e'");
+debug("define getter named c");
+a.__defineGetter__("c", function() { return 3 });
+shouldBe("props(a)", "'a,b,d,e,c'");
+debug("");
+
+var successfullyParsed = true;