Title: [219035] trunk
Revision
219035
Author
[email protected]
Date
2017-06-30 19:56:04 -0700 (Fri, 30 Jun 2017)

Log Message

Web Inspector: AsyncStackTrace nodes can be corrupted when truncating
https://bugs.webkit.org/show_bug.cgi?id=173840
<rdar://problem/30840820>

Reviewed by Joseph Pecoraro.

Source/_javascript_Core:

When truncating an asynchronous stack trace, the parent chain is traversed
until a locked node is found. The path from this node to the root is shared
by more than one stack trace, and cannot be safely modified. Starting at
the first locked node, the path is cloned and becomes a new stack trace tree.

However, the clone operation initialized each new AsyncStackTrace node with
the original node's parent. This would increment the child count of the original
node. When cloning nodes, new nodes should not have their parent set until the
next node up the parent chain is cloned.

* inspector/AsyncStackTrace.cpp:
(Inspector::AsyncStackTrace::truncate):

LayoutTests:

Add a test for truncating a branching asynchronous stack trace.

* inspector/debugger/truncate-async-stack-trace-expected.txt: Added.
* inspector/debugger/truncate-async-stack-trace.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (219034 => 219035)


--- trunk/LayoutTests/ChangeLog	2017-07-01 02:25:59 UTC (rev 219034)
+++ trunk/LayoutTests/ChangeLog	2017-07-01 02:56:04 UTC (rev 219035)
@@ -1,3 +1,16 @@
+2017-06-30  Matt Baker  <[email protected]>
+
+        Web Inspector: AsyncStackTrace nodes can be corrupted when truncating
+        https://bugs.webkit.org/show_bug.cgi?id=173840
+        <rdar://problem/30840820>
+
+        Reviewed by Joseph Pecoraro.
+
+        Add a test for truncating a branching asynchronous stack trace.
+
+        * inspector/debugger/truncate-async-stack-trace-expected.txt: Added.
+        * inspector/debugger/truncate-async-stack-trace.html: Added.
+
 2017-06-30  Alex Christensen  <[email protected]>
 
         Rebase test after r219024

Added: trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace-expected.txt (0 => 219035)


--- trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace-expected.txt	2017-07-01 02:56:04 UTC (rev 219035)
@@ -0,0 +1,7 @@
+Tests for truncated async stack traces.
+
+
+== Running test suite: AsyncStackTrace.truncate
+-- Running test case: TruncateBranchingStackTrace
+PASS: Paused in branching stack trace.
+

Added: trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace.html (0 => 219035)


--- trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/debugger/truncate-async-stack-trace.html	2017-07-01 02:56:04 UTC (rev 219035)
@@ -0,0 +1,49 @@
+<!doctype html>
+<html>
+<head>
+<script src=""
+<script>
+function testBranchingTruncate(repeatCount) {
+    let count = 0;
+    function repeat() {
+        if (count++ === repeatCount) {
+            debugger;
+            TestPage.dispatchEventToFrontend("AfterTestFunction");
+            return;
+        }
+
+        setInterval(() => {}, 1000);
+        setTimeout(repeat, 0);
+    }
+    setTimeout(repeat, 0);
+}
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("AsyncStackTrace.truncate");
+
+    suite.addTestCase({
+        name: "TruncateBranchingStackTrace",
+        description: "Check that branching stack traces can be truncated.",
+        test(resolve, reject) {
+            WebInspector.debuggerManager.awaitEvent(WebInspector.DebuggerManager.Event.Paused)
+            .then((event) => {
+                InspectorTest.pass("Paused in branching stack trace.");
+                WebInspector.debuggerManager.resume();
+            });
+
+            InspectorTest.awaitEvent("AfterTestFunction").then(resolve, reject);
+
+            let repeatCount = WebInspector.debuggerManager.asyncStackTraceDepth;
+            InspectorTest.evaluateInPage(`testBranchingTruncate(${repeatCount})`);
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>Tests for truncated async stack traces.</p>
+</body>
+</html>

Modified: trunk/Source/_javascript_Core/ChangeLog (219034 => 219035)


--- trunk/Source/_javascript_Core/ChangeLog	2017-07-01 02:25:59 UTC (rev 219034)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-07-01 02:56:04 UTC (rev 219035)
@@ -1,3 +1,24 @@
+2017-06-30  Matt Baker  <[email protected]>
+
+        Web Inspector: AsyncStackTrace nodes can be corrupted when truncating
+        https://bugs.webkit.org/show_bug.cgi?id=173840
+        <rdar://problem/30840820>
+
+        Reviewed by Joseph Pecoraro.
+
+        When truncating an asynchronous stack trace, the parent chain is traversed
+        until a locked node is found. The path from this node to the root is shared
+        by more than one stack trace, and cannot be safely modified. Starting at
+        the first locked node, the path is cloned and becomes a new stack trace tree.
+
+        However, the clone operation initialized each new AsyncStackTrace node with
+        the original node's parent. This would increment the child count of the original
+        node. When cloning nodes, new nodes should not have their parent set until the
+        next node up the parent chain is cloned.
+
+        * inspector/AsyncStackTrace.cpp:
+        (Inspector::AsyncStackTrace::truncate):
+
 2017-06-30  Michael Saboff  <[email protected]>
 
         RegExp's  anchored with .* with \g flag can return wrong match start for strings with multiple matches

Modified: trunk/Source/_javascript_Core/inspector/AsyncStackTrace.cpp (219034 => 219035)


--- trunk/Source/_javascript_Core/inspector/AsyncStackTrace.cpp	2017-07-01 02:25:59 UTC (rev 219034)
+++ trunk/Source/_javascript_Core/inspector/AsyncStackTrace.cpp	2017-07-01 02:56:04 UTC (rev 219035)
@@ -163,22 +163,25 @@
     // cloning the locked portion of the trace (the path from the locked node
     // to the new root). The subtree rooted at the last unlocked ancestor is
     // then appended to the new tree.
-    auto* currentNode = lastUnlockedAncestor;
-    while (currentNode->m_parent) {
-        auto& parentNode = currentNode->m_parent;
-        currentNode->m_parent = AsyncStackTrace::create(parentNode->m_callStack.copyRef(), true, parentNode->m_parent);
-        currentNode = currentNode->m_parent.get();
+    auto* previousNode = lastUnlockedAncestor;
 
-        if (parentNode.get() == newStackTraceRoot)
+    // The subtree being truncated must be removed from it's parent before
+    // updating its parent pointer chain.
+    auto* sourceNode = lastUnlockedAncestor->m_parent.get();
+    lastUnlockedAncestor->remove();
+
+    while (sourceNode) {
+        previousNode->m_parent = AsyncStackTrace::create(sourceNode->m_callStack.copyRef(), true, nullptr);
+        previousNode->m_parent->m_childCount = 1;
+        previousNode = previousNode->m_parent.get();
+
+        if (sourceNode == newStackTraceRoot)
             break;
+
+        sourceNode = sourceNode->m_parent.get();
     }
 
-    currentNode->m_truncated = true;
-    currentNode->remove();
-
-    // Decrement the child count of the first locked ancestor after removing its subtree.
-    auto& firstLockedAncestor = lastUnlockedAncestor->m_parent;
-    firstLockedAncestor->m_childCount--;
+    previousNode->m_truncated = true;
 }
 
 void AsyncStackTrace::remove()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to