Bug 17015: REGRESSION (r29414-29428): www.fox.com "shows" menu fails to render
Bug 17164: REGRESSION: JavaScript pop-up menu appears at wrong location when hovering image at http://news.chinatimes.com/
Reviewed by Oliver Hunt
<http://bugs.webkit.org/show_bug.cgi?id=16889>
<rdar://problem/5696255>
<http://bugs.webkit.org/show_bug.cgi?id=17015>
<http://bugs.webkit.org/show_bug.cgi?id=17164>
<rdar://problem/5720947>
The ActivationImp tear-off (r29425) introduced a problem with ReadModify
nodes that first resolve a slot, call valueForReadModifyNode(), and then
store a value in the previously resolved slot. Since valueForReadModifyNode()
may cause a tear-off, the slot needs to be resolved again, but this was
not happening with the existing code.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@29997
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2008-02-04 Cameron Zwarich <cwzwarich@uwaterloo.ca>
+
+ Reviewed by Oliver Hunt.
+
+ Fixes Bug 16889: REGRESSION (r29425): Canvas-based graphing calculator fails to run
+ Bug 17015: REGRESSION (r29414-29428): www.fox.com "shows" menu fails to render
+ Bug 17164: REGRESSION: JavaScript pop-up menu appears at wrong location when hovering image at http://news.chinatimes.com/
+
+ <http://bugs.webkit.org/show_bug.cgi?id=16889>
+ <rdar://problem/5696255>
+
+ <http://bugs.webkit.org/show_bug.cgi?id=17015>
+
+ <http://bugs.webkit.org/show_bug.cgi?id=17164>
+ <rdar://problem/5720947>
+
+ The ActivationImp tear-off (r29425) introduced a problem with ReadModify
+ nodes that first resolve a slot, call valueForReadModifyNode(), and then
+ store a value in the previously resolved slot. Since valueForReadModifyNode()
+ may cause a tear-off, the slot needs to be resolved again, but this was
+ not happening with the existing code.
+
+ * kjs/nodes.cpp:
+ (KJS::ReadModifyLocalVarNode::evaluate):
+ (KJS::ReadModifyResolveNode::evaluate):
+
2008-02-04 Cameron McCormack <cam@mcc.id.au>
Reviewed by Geoff Garen.
PropertySlot slot;
do {
if ((*iter)->getPropertySlot(exec, m_ident, slot)) {
- // See the comment in PostIncResolveNode::evaluate().
-
+ // If m_ident is 'arguments', the base->getPropertySlot() may cause
+ // base (which must be an ActivationImp in such this case) to be torn
+ // off from the activation stack, in which case we need to get it again
+ // from the ScopeChainIterator.
+
JSObject* base = *iter;
JSValue* v = slot.getValue(exec, base, m_ident)->toJSNumber(exec);
base->put(exec, m_ident, jsNumber(v->toNumber(exec) + 1));
JSValue* ReadModifyLocalVarNode::evaluate(ExecState* exec)
{
ASSERT(exec->variableObject() == exec->scopeChain().top());
- JSValue** slot = &exec->localStorage()[m_index].value;
ASSERT(m_operator != OpEqual);
- JSValue* v = valueForReadModifyAssignment(exec, *slot, m_right.get(), m_operator);
+ JSValue* v = valueForReadModifyAssignment(exec, exec->localStorage()[m_index].value, m_right.get(), m_operator);
KJS_CHECKEXCEPTIONVALUE
- *slot = v;
+
+ // We can't store a pointer into localStorage() and use it throughout the function
+ // body, because valueForReadModifyAssignment() might cause an ActivationImp tear-off,
+ // changing the value of localStorage().
+
+ exec->localStorage()[m_index].value = v;
return v;
}
v = valueForReadModifyAssignment(exec, v1, m_right.get(), m_operator);
KJS_CHECKEXCEPTIONVALUE
-
- base->put(exec, m_ident, v);
+
+ // Since valueForReadModifyAssignment() might cause an ActivationImp tear-off,
+ // we need to get the base from the ScopeChainIterator again.
+
+ (*iter)->put(exec, m_ident, v);
return v;
}
+2008-02-04 Cameron Zwarich <cwzwarich@uwaterloo.ca>
+
+ Reviewed by Oliver Hunt.
+
+ Adds layout tests for the collective fix for the following bugs:
+
+ Bug 16889: REGRESSION (r29425): Canvas-based graphing calculator fails to run
+ Bug 17015: REGRESSION (r29414-29428): www.fox.com "shows" menu fails to render
+ Bug 17164: REGRESSION: JavaScript pop-up menu appears at wrong location when hovering image at http://news.chinatimes.com/
+
+ <http://bugs.webkit.org/show_bug.cgi?id=16889>
+ <rdar://problem/5696255>
+
+ <http://bugs.webkit.org/show_bug.cgi?id=17015>
+
+ <http://bugs.webkit.org/show_bug.cgi?id=17164>
+ <rdar://problem/5720947>
+
+ * fast/js/read-modify-eval-expected.txt: Added.
+ * fast/js/read-modify-eval.html: Added.
+ * fast/js/resources/read-modify-eval.js: Added.
+
2008-02-04 Oliver Hunt <oliver@apple.com>
Reviewed by Steve F.
--- /dev/null
+Tests whether eval() works inside statements that read and modify a value.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS multTest(); is true
+PASS divTest(); is true
+PASS addTest(); is true
+PASS subTest(); is true
+PASS lshiftTest(); is true
+PASS rshiftTest(); is true
+PASS urshiftTest(); is true
+PASS andTest(); is true
+PASS xorTest(); is true
+PASS orTest(); is true
+PASS modTest(); is true
+PASS preIncTest(); is true
+PASS preDecTest(); is true
+PASS postIncTest(); is true
+PASS postDecTest(); is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- /dev/null
+<!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/read-modify-eval.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
--- /dev/null
+description(
+'Tests whether eval() works inside statements that read and modify a value.'
+);
+
+function multTest()
+{
+ var x = 1;
+ x *= eval('2');
+ return x == 2;
+}
+
+function divTest()
+{
+ var x = 2;
+ x /= eval('2');
+ return x == 1;
+}
+
+function addTest()
+{
+ var x = 0;
+ x += eval('1');
+ return x == 1;
+}
+
+function subTest()
+{
+ var x = 0;
+ x -= eval('1');
+ return x == -1;
+}
+
+function lshiftTest()
+{
+ var x = 1;
+ x <<= eval('1');
+ return x == 2;
+}
+
+function rshiftTest()
+{
+ var x = 1;
+ x >>= eval('1');
+ return x == 0;
+}
+
+function urshiftTest()
+{
+ var x = 1;
+ x >>>= eval('1');
+ return x == 0;
+}
+
+function andTest()
+{
+ var x = 1;
+ x &= eval('1');
+ return x == 1;
+}
+
+function xorTest()
+{
+ var x = 0;
+ x ^= eval('1');
+ return x == 1;
+}
+
+function orTest()
+{
+ var x = 0;
+ x |= eval('1');
+ return x == 1;
+}
+
+function modTest()
+{
+ var x = 4;
+ x %= eval('3');
+ return x == 1;
+}
+
+function preIncTest()
+{
+ var x = { value: 0 };
+ ++eval('x').value;
+ return x.value == 1;
+}
+
+function preDecTest()
+{
+ var x = { value: 0 };
+ --eval('x').value;
+ return x.value == -1;
+}
+
+function postIncTest()
+{
+ var x = { value: 0 };
+ eval('x').value++;
+ return x.value == 1;
+}
+
+function postDecTest()
+{
+ var x = { value: 0 };
+ eval('x').value--;
+ return x.value == -1;
+}
+
+shouldBeTrue('multTest();');
+shouldBeTrue('divTest();');
+shouldBeTrue('addTest();');
+shouldBeTrue('subTest();');
+shouldBeTrue('lshiftTest();');
+shouldBeTrue('rshiftTest();');
+shouldBeTrue('urshiftTest();');
+shouldBeTrue('andTest();');
+shouldBeTrue('xorTest();');
+shouldBeTrue('orTest();');
+shouldBeTrue('modTest();');
+
+shouldBeTrue('preIncTest();');
+shouldBeTrue('preDecTest();');
+shouldBeTrue('postIncTest();');
+shouldBeTrue('postDecTest();');
+
+successfullyParsed = true;