Fix progress not passed through from cast promises. Closes #88

petkaantonov · Feb 4, 2014 · 4ce08c9 · 4ce08c9
1 parent 4d4a650
commit 4ce08c9
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 17 deletions.
diff --git a/src/progress.js b/src/progress.js
@@ -93,7 +93,8 @@ module.exports = function(Promise, isPromiseArrayProxy) {
                 if (typeof handler === "function") {
                     handler.call(receiver, progressValue, promise);
                 }
-                else if (Promise.is(receiver) && receiver._isProxied()) {
+                else if (typeof receiver._isProxied === "function" &&
+                        receiver._isProxied()) {
                     receiver._progressUnchecked(progressValue);
                 }
                 else if (isPromiseArrayProxy(receiver, promise)) {

diff --git a/src/promise.js b/src/promise.js
@@ -422,8 +422,9 @@ function Promise$_then(
     if (debugging && !haveInternalData) {
         var haveSameContext = this._peekContext() === this._traceParent;
         ret._traceParent = haveSameContext ? this._traceParent : this;
-        ret._setTrace(typeof caller === "function" ?
-            caller : this._then, this);
+        ret._setTrace(typeof caller === "function"
+                ? caller
+                : this._then, this);
     }
 
     if (!haveInternalData && this._isBound()) {
@@ -759,7 +760,6 @@ function Promise$_settlePromiseFromHandler(
 Promise.prototype._follow =
 function Promise$_follow(promise) {
     ASSERT(arguments.length === 1);
-    ASSERT(isPromise(promise));
     ASSERT(this._isFollowingOrFulfilledOrRejected() === false);
     ASSERT(promise !== this);
     this._setFollowing();
@@ -923,7 +923,8 @@ Promise.prototype._settlePromiseAt = function Promise$_settlePromiseAt(index) {
         //optimization when .then listeners on a promise are
         //just respective fate sealers on some other promise
         if (receiver !== void 0) {
-            if (receiver instanceof Promise && receiver._isProxied()) {
+            if (typeof receiver._isProxied === "function" &&
+                receiver._isProxied()) {
                 //Must be smuggled data if proxied
                 ASSERT(!isPromise(promise));
                 receiver._unsetProxied();
@@ -1182,7 +1183,7 @@ if (!CapturedTrace.isSupported()) {
 Promise._makeSelfResolutionError = makeSelfResolutionError;
 require("./finally.js")(Promise, NEXT_FILTER);
 require("./direct_resolve.js")(Promise);
-require("./thenables.js")(Promise);
+require("./thenables.js")(Promise, INTERNAL);
 Promise.RangeError = RangeError;
 Promise.CancellationError = CancellationError;
 Promise.TimeoutError = TimeoutError;

diff --git a/src/thenables.js b/src/thenables.js
@@ -1,10 +1,9 @@
 "use strict";
-module.exports = function(Promise) {
+module.exports = function(Promise, INTERNAL) {
     var ASSERT = require("./assert.js");
     var util = require("./util.js");
     var errorObj = util.errorObj;
     var isObject = util.isObject;
-    var tryCatch2 = util.tryCatch2;
 
     function getThen(obj) {
         try {
@@ -38,21 +37,47 @@ module.exports = function(Promise) {
         return obj;
     }
 
+    function isAnyBluebirdPromise(obj) {
+        try {
+            return typeof obj._resolveFromSyncValue === "function";
+        }
+        catch(ignore) {
+            return false;
+        }
+    }
+
     function Promise$_doThenable(x, then, caller, originalPromise) {
         ASSERT(typeof then === "function");
         ASSERT(arguments.length === 4);
-        var resolver = Promise.defer(caller);
+        //Make casting from another bluebird fast
+        if (isAnyBluebirdPromise(x)) {
+            var ret = new Promise(INTERNAL);
+            ret._follow(x);
+            ret._setTrace(caller, void 0);
+            return ret;
+        }
+        return Promise$_doThenableSlowCase(x, then, caller, originalPromise);
+    }
 
+    function Promise$_doThenableSlowCase(x, then, caller, originalPromise) {
+        var resolver = Promise.defer(caller);
         var called = false;
-        var ret = tryCatch2(then, x,
-            Promise$_resolveFromThenable, Promise$_rejectFromThenable);
-
-        if (ret === errorObj && !called) {
-            called = true;
-            if (originalPromise !== void 0) {
-                originalPromise._attachExtraTrace(ret.e);
+        try {
+            then.call(
+                x,
+                Promise$_resolveFromThenable,
+                Promise$_rejectFromThenable,
+                Promise$_progressFromThenable
+            );
+        }
+        catch(e) {
+            if (!called) {
+                called = true;
+                if (originalPromise !== void 0) {
+                    originalPromise._attachExtraTrace(e);
+                }
+                resolver.promise._reject(e);
             }
-            resolver.promise._reject(ret.e);
         }
         return resolver.promise;
 
@@ -81,6 +106,14 @@ module.exports = function(Promise) {
             resolver.promise._attachExtraTrace(r);
             resolver.promise._reject(r);
         }
+
+        function Promise$_progressFromThenable(v) {
+            if (called) return;
+            var promise = resolver.promise;
+            if (typeof promise._progress === "function") {
+                promise._progress(v);
+            }
+        }
     }
 
     Promise._cast = Promise$_Cast;

diff --git a/test/mocha/bluebird-multiple-instances.js b/test/mocha/bluebird-multiple-instances.js
@@ -69,6 +69,30 @@ if( isNodeJS ) {
             }, 13);
         });
 
+        specify("Should use fast cast", function(done) {
+            var a = Promise1.pending();
+            var b = Promise2.cast(a.promise);
+            assert(b._isProxied());
+            done();
+        });
+
+        specify("Should pass through progress with fast cast", function(done){
+            var a = Promise1.pending();
+            var b = Promise2.cast(a.promise);
+            var test = 0;
+            b.then(function() {
+                test++;
+            }, null, function() {
+                test++;
+            });
+
+            a.progress();
+            a.resolve();
+            setTimeout(function(){
+                assert.equal(test, 2);
+                done();
+            }, 20);
+        });
     });
 
 }
diff --git a/test/mocha/q_progress.js b/test/mocha/q_progress.js
@@ -444,4 +444,33 @@ describe("progress", function () {
             done();
         }, 13);
     });
+
+    specify("GH-88", function(done) {
+        var thenable = {
+            then: function(f, r, p) {
+                setTimeout(function(){
+                    var l = 10;
+                    while(l--) {
+                        p(4);
+                    }
+                    setTimeout(function(){
+                        f(3);
+                    }, 13);
+                }, 13);
+            }
+        };
+
+        var promise = Promise.cast(thenable);
+        var count = 0;
+        promise.progressed(function(v){
+            count++;
+            assert.equal(v, 4);
+        });
+        promise.then(function(v) {
+            assert.equal(count, 10);
+            assert.equal(v, 3);
+            done();
+        });
+
+    });
 });