[inspector] better stacks for promises

src/inspector/v8-debugger.cc

Index: src/inspector/v8-debugger.cc
diff --git a/src/inspector/v8-debugger.cc b/src/inspector/v8-debugger.cc
index 3acb329dae1c17369177da985803ae5877125e1a..458e7111814d2aef1ca21808a3f21fc30fe100fd 100644
--- a/src/inspector/v8-debugger.cc
+++ b/src/inspector/v8-debugger.cc
@@ -913,10 +913,6 @@ void V8Debugger::asyncTaskCanceledForStack(void* task) {
 
 void V8Debugger::asyncTaskStartedForStack(void* task) {
   if (!m_maxAsyncCallStackDepth) return;
-  m_currentTasks.push_back(task);
-  auto parentIt = m_parentTask.find(task);
-  AsyncTaskToStackTrace::iterator stackIt = m_asyncTaskStacks.find(
-      parentIt == m_parentTask.end() ? task : parentIt->second);
   // Needs to support following order of events:
   // - asyncTaskScheduled
   //   <-- attached here -->
@@ -924,15 +920,21 @@ void V8Debugger::asyncTaskStartedForStack(void* task) {
   // - asyncTaskCanceled <-- canceled before finished
   //   <-- async stack requested here -->
   // - asyncTaskFinished
-  std::weak_ptr<AsyncStackTrace> asyncParent;
-  if (stackIt != m_asyncTaskStacks.end()) asyncParent = stackIt->second;
+  m_currentTasks.push_back(task);
+  auto parentIt = m_parentTask.find(task);
+  AsyncTaskToStackTrace::iterator stackIt = m_asyncTaskStacks.find(
+      parentIt == m_parentTask.end() ? task : parentIt->second);
+  if (stackIt != m_asyncTaskStacks.end()) {
+    m_currentAsyncParent.push_back(stackIt->second.lock());
+  } else {
+    m_currentAsyncParent.emplace_back();
+  }
   auto itCreation = m_asyncTaskCreationStacks.find(task);
-  if (asyncParent.lock() && itCreation != m_asyncTaskCreationStacks.end()) {

[kozy, 2017/04/27 18:35:31]

this change is essential of this CL.

+  if (itCreation != m_asyncTaskCreationStacks.end()) {
     m_currentAsyncCreation.push_back(itCreation->second.lock());
   } else {
     m_currentAsyncCreation.emplace_back();
   }
-  m_currentAsyncParent.push_back(asyncParent.lock());
 }
 
 void V8Debugger::asyncTaskFinishedForStack(void* task) {
@@ -1039,9 +1041,37 @@ void V8Debugger::collectOldAsyncStacksIfNeeded() {
       ++it;
     }
   }
+  // We use parent information to provide correct parent stack for promises,
+  // Promise.resolve().then(...).then(...)
+  // created task#1 - stack#1
+  // scheduled task#1 - stack#2
+  // created task#2 -> task#1 - stack#3
+  // created task#3 -> task#2 - stack#4
+  // ...
+  // started task#2 - use stack(parent(task#2)) == stack(task#1) == stack#2
+  // ...
+  // scheduled task#2 - stack#5

[dgozman, 2017/04/27 17:26:34]

scheduled after started?

[kozy, 2017/04/27 18:35:31]

yes, it's how our promise instrumentation works and why on started we get
scheduled stack from parent.

+  // finished task#2
+  // started task#3 - use stack(parent(task#3)) == stack(task#2) == stack#5
+  // ...
+  // scheduled task#3 - stack#6
+  // finished task#3
+  // Let's consider what parent link is obsolete when we collect stacks:
+  // stack#1 - (task#2 -> task#1, task#3 -> task#2)
+  // stack#2 - (task#3 -> task#2)
+  // stack#3 - (task#3 -> task#2)
+  // stack#4 - (task#3 -> task#2)
+  // stack#5 - ()
+  // stack#6 - ()
+  // But in case when we start collecting stacks after getting stack#4 but
+  // before stack#5 is added we should preserve parent link since original task
+  // can be scheduled at any time in future and this link will be required to
+  // get correct parent stack later. It means that we should collect parent link
+  // in case when there is no creation or schedule stack for parent.
   for (auto it = m_parentTask.begin(); it != m_parentTask.end();) {
     if (m_asyncTaskCreationStacks.find(it->second) ==
-        m_asyncTaskCreationStacks.end()) {
+            m_asyncTaskCreationStacks.end() &&
+        m_asyncTaskStacks.find(it->second) == m_asyncTaskStacks.end()) {
       it = m_parentTask.erase(it);
     } else {
       ++it;