2011-03-24 David Hyatt <hyatt@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Mar 2011 22:01:18 +0000 (22:01 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Mar 2011 22:01:18 +0000 (22:01 +0000)
        Reviewed by Dan Bernstein.

        Add a simplified normal flow layout path optimization for overflow recomputation
        and for positioned objects inside relative positioned containers.

        Currently there is an optimized code path for positioned objects, but as soon as
        we encounter a normal flow object in the containing block chain, we lose the
        optimization.

        This patch adds a new type of style difference called SimplifiedLayout that is
        returned when only overflow needs to be recomputed. Whenever a transform changes,
        this is the hint returned now instead of a full layout.

        In addition, when positioned objects need layout and start marking up the
        containing block chain, we now propagate the fact that the layout is simplified
        all the way up to the root, even when we encounter normal flow containing
        blocks.

        The layoutOnlyPositionedObjects function has been renamed to simplifiedLayout()
        and is now used for all of these cases (in addition to what it handled before).

        The simplified layout optimization (even in ToT) did not work correctly when
        static distances needed to be recomputed. In order to make static distance
        computations work with simplified layout, positioned objects now always compute
        their static offsets, even if they explicitly specify left/top.  That way normal
        flow layout never has to re-run when the positioned object moves.  This makes
        movement of a positioned object along a single non-auto axis much faster when the
        other axis is auto. Because this code kicked in more often for absolutely positioned
        objects whose original display was inline, I went ahead and fixed the trailing space
        issue with those objects.  This causes a bunch of layout tests to progress.

        Added fast/block/positioning/static-inline-position-dynamic.html and trailing-space-test.html.

        * animations/dynamic-stylesheet-loading-expected.txt:
        * animations/keyframe-timing-functions-expected.txt:
        * fast/block/positioning/static-inline-position-dynamic.html: Added.
        * fast/block/positioning/trailing-space-test.html: Added.
        * fast/dynamic/ancestor-to-absolute-expected.txt:
        * fast/forms/input-appearance-elementFromPoint-expected.txt:
        * fast/forms/input-hit-test-border-expected.txt:
        * platform/mac/fast/block/positioning/002-expected.png:
        * platform/mac/fast/block/positioning/003-expected.png:
        * platform/mac/fast/block/positioning/004-expected.png:
        * platform/mac/fast/block/positioning/005-expected.png:
        * platform/mac/fast/block/positioning/006-expected.png:
        * platform/mac/fast/block/positioning/007-expected.png:
        * platform/mac/fast/block/positioning/008-expected.png:
        * platform/mac/fast/block/positioning/009-expected.png:
        * platform/mac/fast/block/positioning/010-expected.png:
        * platform/mac/fast/block/positioning/011-expected.png:
        * platform/mac/fast/block/positioning/012-expected.png:
        * platform/mac/fast/block/positioning/013-expected.png:
        * platform/mac/fast/block/positioning/014-expected.png:
        * platform/mac/fast/block/positioning/015-expected.png:
        * platform/mac/fast/block/positioning/016-expected.png:
        * platform/mac/fast/block/positioning/017-expected.png:
        * platform/mac/fast/block/positioning/018-expected.png:
        * platform/mac/fast/block/positioning/019-expected.png:
        * platform/mac/fast/block/positioning/020-expected.png:
        * platform/mac/fast/block/positioning/021-expected.png:
        * platform/mac/fast/block/positioning/022-expected.png:
        * platform/mac/fast/block/positioning/023-expected.png:
        * platform/mac/fast/block/positioning/024-expected.png:
        * platform/mac/fast/block/positioning/025-expected.png:
        * platform/mac/fast/block/positioning/026-expected.png:
        * platform/mac/fast/block/positioning/027-expected.png:
        * platform/mac/fast/block/positioning/028-expected.png:
        * platform/mac/fast/block/positioning/029-expected.png:
        * platform/mac/fast/block/positioning/030-expected.png:
        * platform/mac/fast/block/positioning/031-expected.png:
        * platform/mac/fast/block/positioning/032-expected.png:
        * platform/mac/fast/block/positioning/033-expected.png:
        * platform/mac/fast/block/positioning/034-expected.png:
        * platform/mac/fast/block/positioning/035-expected.png:
        * platform/mac/fast/block/positioning/036-expected.png:
        * platform/mac/fast/block/positioning/037-expected.png:
        * platform/mac/fast/block/positioning/038-expected.png:
        * platform/mac/fast/block/positioning/039-expected.png:
        * platform/mac/fast/block/positioning/040-expected.png:
        * platform/mac/fast/block/positioning/041-expected.png:
        * platform/mac/fast/block/positioning/042-expected.png:
        * platform/mac/fast/block/positioning/043-expected.png:
        * platform/mac/fast/block/positioning/044-expected.png:
        * platform/mac/fast/block/positioning/045-expected.png:
        * platform/mac/fast/block/positioning/046-expected.png:
        * platform/mac/fast/block/positioning/047-expected.checksum:
        * platform/mac/fast/block/positioning/047-expected.png:
        * platform/mac/fast/block/positioning/048-expected.png:
        * platform/mac/fast/block/positioning/049-expected.png:
        * platform/mac/fast/block/positioning/050-expected.png:
        * platform/mac/fast/block/positioning/056-expected.txt:
        * platform/mac/fast/block/positioning/061-expected.png:
        * platform/mac/fast/block/positioning/062-expected.png:
        * platform/mac/fast/block/positioning/auto/005-expected.txt:
        * platform/mac/fast/block/positioning/auto/006-expected.txt:
        * platform/mac/fast/block/positioning/auto/007-expected.png:
        * platform/mac/fast/block/positioning/auto/vertical-lr/005-expected.txt:
        * platform/mac/fast/block/positioning/auto/vertical-lr/006-expected.txt:
        * platform/mac/fast/block/positioning/auto/vertical-lr/007-expected.checksum:
        * platform/mac/fast/block/positioning/auto/vertical-lr/007-expected.png:
        * platform/mac/fast/block/positioning/auto/vertical-rl/005-expected.txt:
        * platform/mac/fast/block/positioning/auto/vertical-rl/006-expected.txt:
        * platform/mac/fast/block/positioning/auto/vertical-rl/007-expected.checksum:
        * platform/mac/fast/block/positioning/auto/vertical-rl/007-expected.png:
        * platform/mac/fast/block/positioning/fixed-positioning-scrollbar-bug-expected.txt:
        * platform/mac/fast/block/positioning/inline-block-relposition-expected.checksum:
        * platform/mac/fast/block/positioning/inline-block-relposition-expected.png:
        * platform/mac/fast/block/positioning/inline-block-relposition-expected.txt:
        * platform/mac/fast/block/positioning/move-with-auto-width-expected.png:
        * platform/mac/fast/block/positioning/negative-rel-position-expected.png:
        * platform/mac/fast/block/positioning/relative-overconstrained-expected.png:
        * platform/mac/fast/block/positioning/relative-overflow-block-expected.txt:
        * platform/mac/fast/block/positioning/static-inline-position-dynamic-expected.checksum: Added.
        * platform/mac/fast/block/positioning/static-inline-position-dynamic-expected.png: Added.
        * platform/mac/fast/block/positioning/static-inline-position-dynamic-expected.txt: Added.
        * platform/mac/fast/block/positioning/trailing-space-test-expected.checksum: Added.
        * platform/mac/fast/block/positioning/trailing-space-test-expected.png: Added.
        * platform/mac/fast/block/positioning/trailing-space-test-expected.txt: Added.
        * platform/mac/fast/clip/001-expected.txt:
        * platform/mac/fast/clip/004-expected.txt:
        * platform/mac/fast/clip/006-expected.txt:
        * platform/mac/fast/clip/007-expected.txt:
        * platform/mac/fast/clip/008-expected.txt:
        * platform/mac/fast/clip/009-expected.txt:
        * platform/mac/fast/clip/010-expected.txt:
        * platform/mac/fast/clip/011-expected.txt:
        * platform/mac/fast/clip/012-expected.txt:
        * platform/mac/fast/clip/013-expected.txt:
        * platform/mac/fast/clip/014-expected.txt:
        * platform/mac/fast/clip/nestedTransparencyClip-expected.txt:
        * platform/mac/fast/clip/outline-overflowClip-expected.txt:
        * platform/mac/fast/forms/input-appearance-preventDefault-expected.txt:
        * platform/mac/fast/invalid/014-expected.txt:
        * platform/mac/fast/layers/layer-visibility-expected.txt:
        * platform/mac/fast/repaint/layout-state-scrolloffset-expected.txt:
        * platform/mac/fast/repaint/layout-state-scrolloffset2-expected.txt:
        * platform/mac/fast/repaint/layout-state-scrolloffset3-expected.txt:
        * platform/mac/tables/mozilla/bugs/bug51140-expected.txt:
2011-03-25  Geoffrey Garen  <ggaren@apple.com>

        Reviewed by Oliver Hunt.

        Crash when paused at a breakpoint caused by inaccurate Activation records.
        https://bugs.webkit.org/show_bug.cgi?id=57120

        * runtime/JSActivation.cpp:
        (JSC::JSActivation::symbolTableGet):
        (JSC::JSActivation::symbolTablePut):
        (JSC::JSActivation::getOwnPropertyNames):
        (JSC::JSActivation::symbolTablePutWithAttributes):

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

LayoutTests/ChangeLog
LayoutTests/inspector/debugger/debugger-activation-crash2-expected.txt [new file with mode: 0644]
LayoutTests/inspector/debugger/debugger-activation-crash2.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSActivation.cpp

index 07bd8e3..6cc7c7b 100644 (file)
 
         Reviewed by Oliver Hunt.
 
+        Crash when paused at a breakpoint caused by inaccurate Activation records.
+        https://bugs.webkit.org/show_bug.cgi?id=57120
+
+        * inspector/debugger/debugger-activation-crash2-expected.txt: Added.
+        * inspector/debugger/debugger-activation-crash2.html: Added.
+
+2011-03-25  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
         Crash in debugger beneath MarkStack::drain @ me.com, ibm.com
         https://bugs.webkit.org/show_bug.cgi?id=57080
         <rdar://problem/8525907>
diff --git a/LayoutTests/inspector/debugger/debugger-activation-crash2-expected.txt b/LayoutTests/inspector/debugger/debugger-activation-crash2-expected.txt
new file mode 100644 (file)
index 0000000..d903107
--- /dev/null
@@ -0,0 +1,12 @@
+Tests for a crash when paused at a breakpoint caused by inaccurate Activation records. Bug 57120
+
+Debugger was enabled.
+Script source was shown.
+Set timer for test function.
+Script execution paused.
+Call stack:
+    0)  (debugger-activation-crash2.html:9)
+    1) testFunction (debugger-activation-crash2.html:20)
+Script execution resumed.
+Debugger was disabled.
+
diff --git a/LayoutTests/inspector/debugger/debugger-activation-crash2.html b/LayoutTests/inspector/debugger/debugger-activation-crash2.html
new file mode 100644 (file)
index 0000000..92ce3e1
--- /dev/null
@@ -0,0 +1,54 @@
+<script src="../../http/tests/inspector/inspector-test.js"></script>
+<script src="../../http/tests/inspector/debugger-test.js"></script>
+
+<script>
+var closures = [];
+function makeClosure() {
+    var v1, v2, v3, v4, v5, v6, v7, v8, v9, v10; // Make a lot of potentially captured variables.
+    return function () { 
+        var x = v1; // But only capture one in optimizing compiles.
+        return x;
+    };
+}
+
+for (var i = 0; i < 100; ++i) {
+    closures.push(makeClosure());
+}
+
+closures[0](); // Force compilation.
+function testFunction() {
+    closures[0](); // Force recompilation.
+    
+    // At this point, closures[0] captured 1 variable but thinks it captured 10.
+    // If so, stopping at a breakpoint should make it crash.
+}
+
+function test() {
+    InspectorTest.startDebuggerTest(step1);
+
+    function step1()
+    {
+        InspectorTest.showScriptSource("debugger-activation-crash2.html", step2);
+    }
+
+    function step2(sourceFrame)
+    {
+        InspectorTest.addResult("Script source was shown.");
+        InspectorTest.setBreakpoint(sourceFrame, 8, "", true);
+        InspectorTest.runTestFunctionAndWaitUntilPaused(step3);
+    }
+
+    function step3(callFrames)
+    {
+        InspectorTest.captureStackTrace(callFrames);
+        InspectorTest.completeDebuggerTest();
+    }
+}
+
+window.onload = runTest;
+</script>
+
+<p>
+Tests for a crash when paused at a breakpoint caused by inaccurate Activation records.
+<a href="https://bugs.webkit.org/show_bug.cgi?id=57120">Bug 57120</a>
+</p>
index 4d73e69..cd27b9f 100644 (file)
@@ -1,3 +1,16 @@
+2011-03-25  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Crash when paused at a breakpoint caused by inaccurate Activation records.
+        https://bugs.webkit.org/show_bug.cgi?id=57120
+
+        * runtime/JSActivation.cpp:
+        (JSC::JSActivation::symbolTableGet):
+        (JSC::JSActivation::symbolTablePut):
+        (JSC::JSActivation::getOwnPropertyNames):
+        (JSC::JSActivation::symbolTablePutWithAttributes):
+
 2011-03-24  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.
index ca45f2b..d0c50dd 100644 (file)
@@ -76,12 +76,13 @@ void JSActivation::markChildren(MarkStack& markStack)
 inline bool JSActivation::symbolTableGet(const Identifier& propertyName, PropertySlot& slot)
 {
     SymbolTableEntry entry = symbolTable().inlineGet(propertyName.impl());
-    if (!entry.isNull()) {
-        ASSERT(entry.getIndex() < m_numCapturedVars);
-        slot.setValue(registerAt(entry.getIndex()).get());
-        return true;
-    }
-    return false;
+    if (entry.isNull())
+        return false;
+    if (entry.getIndex() >= m_numCapturedVars)
+        return false;
+
+    slot.setValue(registerAt(entry.getIndex()).get());
+    return true;
 }
 
 inline bool JSActivation::symbolTablePut(JSGlobalData& globalData, const Identifier& propertyName, JSValue value)
@@ -93,7 +94,9 @@ inline bool JSActivation::symbolTablePut(JSGlobalData& globalData, const Identif
         return false;
     if (entry.isReadOnly())
         return true;
-    ASSERT(entry.getIndex() < m_numCapturedVars);
+    if (entry.getIndex() >= m_numCapturedVars)
+        return false;
+
     registerAt(entry.getIndex()).set(globalData, this, value);
     return true;
 }
@@ -102,9 +105,11 @@ void JSActivation::getOwnPropertyNames(ExecState* exec, PropertyNameArray& prope
 {
     SymbolTable::const_iterator end = symbolTable().end();
     for (SymbolTable::const_iterator it = symbolTable().begin(); it != end; ++it) {
-        ASSERT(it->second.getIndex() < m_numCapturedVars);
-        if (!(it->second.getAttributes() & DontEnum) || (mode == IncludeDontEnumProperties))
-            propertyNames.add(Identifier(exec, it->first.get()));
+        if (it->second.getAttributes() & DontEnum && mode != IncludeDontEnumProperties)
+            continue;
+        if (it->second.getIndex() >= m_numCapturedVars)
+            continue;
+        propertyNames.add(Identifier(exec, it->first.get()));
     }
     // Skip the JSVariableObject implementation of getOwnPropertyNames
     JSObject::getOwnPropertyNames(exec, propertyNames, mode);
@@ -121,6 +126,7 @@ inline bool JSActivation::symbolTablePutWithAttributes(JSGlobalData& globalData,
     ASSERT(!entry.isNull());
     if (entry.getIndex() >= m_numCapturedVars)
         return false;
+
     entry.setAttributes(attributes);
     registerAt(entry.getIndex()).set(globalData, this, value);
     return true;