Fix potential control flow deadlock

If a task function returns a promise, but schedules no sub-tasks, wait
for the promise to resolve as there may be tasks scheduled in the
callback chain.

Fixes issue 8094

Author: jleyba

javascript/webdriver/promise.js

@@ -1772,7 +1772,7 @@ promise.ControlFlow.prototype.getNextTask_ = function() {
   var frame = this.activeFrame_;
   var firstChild = frame.getFirstChild();
   if (!firstChild) {
-    if (!frame.pendingCallback) {
+    if (!frame.pendingCallback && !frame.isBlocked_) {
       this.resolveFrame_(frame);
     }
     return null;
@@ -1894,13 +1894,14 @@ promise.ControlFlow.prototype.runInFrame_ = function(
       return;
     }
 
-    // If the executed function was a task and the result is a promise, wait
-    // for the promise to resolve. If there is nothing scheduled, go ahead and
-    // discard the frame. Otherwise, we wait for the frame to be closed out
-    // by the event loop.
+    // If the executed function returned a promise, wait for it to resolve. If
+    // there is nothing scheduled in the frame, go ahead and discard it.
+    // Otherwise, we wait for the frame to be closed out by the event loop.
     var shortCircuitTask;
-    if (opt_isTask && promise.isPromise(result)) {
+    if (promise.isPromise(result)) {
+      newFrame.isBlocked_ = true;
       var onResolve = function() {
+        newFrame.isBlocked_ = false;
         shortCircuitTask = new promise.MicroTask_(function() {
           if (isCloseable(newFrame)) {
             removeNewFrame();
@@ -1919,6 +1920,9 @@ promise.ControlFlow.prototype.runInFrame_ = function(
 
     newFrame.once(promise.Frame_.CLOSE_EVENT, function() {
       shortCircuitTask && shortCircuitTask.cancel();
+      if (isCloseable(newFrame)) {
+        removeNewFrame();
+      }
       callback(result);
     }).once(promise.Frame_.ERROR_EVENT, function(reason) {
       shortCircuitTask && shortCircuitTask.cancel();
@@ -2128,6 +2132,13 @@ promise.Frame_ = goog.defineClass(webdriver.EventEmitter, {
      */
     this.isLocked_ = false;
 
+    /**
+     * Whether this frame's completion is blocked on the resolution of a promise
+     * returned by its main function.
+     * @private
+     */
+    this.isBlocked_ = false;
+
     /**
      * Whether this frame represents a pending callback attached to a
      * {@link promise.Promise}.

javascript/webdriver/test/promise_flow_test.js

@@ -338,6 +338,38 @@ function testFraming_lotsOfNesting() {
 }
 
 
+function testFrame_callbackReturnsPromiseThatDependsOnATask_1() {
+  schedule('a').then(function() {
+    schedule('b');
+    return webdriver.promise.delayed(5).then(function() {
+      return schedule('c');
+    });
+  });
+  schedule('d');
+
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c', 'd');
+  });
+}
+
+
+function testFrame_callbackReturnsPromiseThatDependsOnATask_2() {
+  schedule('a').then(function() {
+    schedule('b');
+    return webdriver.promise.delayed(5).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return schedule('c'); });
+  });
+  schedule('d');
+
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c', 'd');
+  });
+}
+
+
 function testFraming_eachCallbackWaitsForAllScheduledTasksToComplete() {
   schedule('a').
       then(function() {
@@ -1293,8 +1325,67 @@ function testSubtasks_taskReturnsPromiseThatDependsOnSubtask_2() {
 }
 
 
+function testSubtasks_taskReturnsPromiseThatDependsOnSubtask_3() {
+  scheduleAction('a', function() {
+    return webdriver.promise.delayed(10).then(function() {
+      return schedule('b');
+    });
+  });
+  schedule('c');
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c');
+  });
+}
 
 
+function testSubtasks_taskReturnsPromiseThatDependsOnSubtask_4() {
+  scheduleAction('a', function() {
+    return webdriver.promise.delayed(5).then(function() {
+      return webdriver.promise.delayed(5).then(function() {
+        return schedule('b');
+      });
+    });
+  });
+  schedule('c');
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c');
+  });
+}
+
+
+function testSubtasks_taskReturnsPromiseThatDependsOnSubtask_5() {
+  scheduleAction('a', function() {
+    return webdriver.promise.delayed(5).then(function() {
+      return webdriver.promise.delayed(5).then(function() {
+        return webdriver.promise.delayed(5).then(function() {
+          return webdriver.promise.delayed(5).then(function() {
+            return schedule('b');
+          });
+        });
+      });
+    });
+  });
+  schedule('c');
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c');
+  });
+}
+
+
+function testSubtasks_taskReturnsPromiseThatDependsOnSubtask_6() {
+  scheduleAction('a', function() {
+    return webdriver.promise.delayed(5).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return webdriver.promise.delayed(5) }).
+        then(function() { return schedule('b'); });
+  });
+  schedule('c');
+  return waitForIdle().then(function() {
+    assertFlowHistory('a', 'b', 'c');
+  });
+}
+
 function testSubtasks_subTaskFails_1() {
   schedule('a');
   scheduleAction('sub-tasks', function() {