Skip to content

Commit 21c0893

Browse files
committed
Simplify promise cancellation logic.
1 parent 2550328 commit 21c0893

File tree

5 files changed

+96
-130
lines changed

5 files changed

+96
-130
lines changed

Diff for: javascript/node/selenium-webdriver/test/http/util_test.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,12 @@ describe('selenium-webdriver/http/util', function() {
125125
var isReady = util.waitForServer(baseUrl, 200).
126126
then(function() { done('Did not expect to succeed'); }).
127127
then(null, function(e) {
128-
assert.equal(err, e);
128+
assert.equal('cancelled!', e.message);
129129
}).
130130
then(function() { done(); }, done);
131131

132132
setTimeout(function() {
133-
isReady.cancel(err);
133+
isReady.cancel('cancelled!');
134134
}, 50);
135135
});
136136
});
@@ -165,16 +165,15 @@ describe('selenium-webdriver/http/util', function() {
165165

166166
it('can cancel wait', function(done) {
167167
responseCode = 404;
168-
var err = Error('cancelled!');
169168
var isReady = util.waitForUrl(baseUrl, 200).
170169
then(function() { done('Did not expect to succeed'); }).
171170
then(null, function(e) {
172-
assert.equal(err, e);
171+
assert.equal('cancelled!', e.message);
173172
}).
174173
then(function() { done(); }, done);
175174

176175
setTimeout(function() {
177-
isReady.cancel(err);
176+
isReady.cancel('cancelled!');
178177
}, 50);
179178
});
180179
});

Diff for: javascript/safari-driver/extension/server.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ safaridriver.extension.Server.prototype.disposeInternal = function() {
174174

175175
if (this.webSocket_) {
176176
if (this.ready_.isPending()) {
177-
this.ready_.cancel(Error('Server has been disposed'));
177+
this.ready_.cancel('Server has been disposed');
178178
}
179179
this.disposeWebSocket_();
180180
}

Diff for: javascript/webdriver/promise.js

+57-58
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ goog.require('webdriver.stacktrace.Snapshot');
6161

6262

6363

64+
/**
65+
* Error used when the computation of a promise is cancelled.
66+
*
67+
* @param {string=} opt_msg The cancellation message.
68+
* @constructor
69+
* @extends {goog.debug.Error}
70+
* @final
71+
*/
72+
webdriver.promise.CancellationError = function(opt_msg) {
73+
goog.debug.Error.call(this, opt_msg);
74+
75+
/** @override */
76+
this.name = 'CancellationError';
77+
};
78+
goog.inherits(webdriver.promise.CancellationError, goog.debug.Error);
79+
80+
81+
6482
/**
6583
* Thenable is a promise-like object with a {@code then} method which may be
6684
* used to schedule callbacks on a promised value.
@@ -75,9 +93,7 @@ webdriver.promise.Thenable = function() {};
7593
* Cancels the computation of this promise's value, rejecting the promise in the
7694
* process. This method is a no-op if the promise has alreayd been resolved.
7795
*
78-
* @param {*=} opt_reason The reason this promise is being cancelled. If not an
79-
* {@code Error}, one will be created using the value's string
80-
* representation.
96+
* @param {string=} opt_reason The reason this promise is being cancelled.
8197
*/
8298
webdriver.promise.Thenable.prototype.cancel = function(opt_reason) {};
8399

@@ -297,16 +313,14 @@ webdriver.promise.Promise.prototype.thenFinally = function(callback) {
297313
* the Deferred's canceller function (if provided). The canceller may return a
298314
* truth-y value to override the reason provided for rejection.
299315
*
300-
* @param {Function=} opt_canceller Function to call when cancelling the
301-
* computation of this instance's value.
302316
* @param {webdriver.promise.ControlFlow=} opt_flow The control flow
303317
* this instance was created under. This should only be provided during
304318
* unit tests.
305319
* @constructor
306320
* @extends {webdriver.promise.Promise.<T>}
307321
* @template T
308322
*/
309-
webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
323+
webdriver.promise.Deferred = function(opt_flow) {
310324
/* NOTE: This class's implementation diverges from the prototypical style
311325
* used in the rest of the atoms library. This was done intentionally to
312326
* protect the internal Deferred state from consumers, as outlined by
@@ -316,6 +330,12 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
316330

317331
var flow = opt_flow || webdriver.promise.controlFlow();
318332

333+
/**
334+
* The deferred this instance is chained from, if any.
335+
* @private {webdriver.promise.Deferred.<?>}
336+
*/
337+
this.parent_ = null;
338+
319339
/**
320340
* The listeners registered with this Deferred. Each element in the list will
321341
* be a 3-tuple of the callback function, errback function, and the
@@ -422,7 +442,8 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
422442
notify(listeners.shift());
423443
}
424444

425-
if (!handled && state == webdriver.promise.Deferred.State_.REJECTED) {
445+
if (!handled && state == webdriver.promise.Deferred.State_.REJECTED &&
446+
!(value instanceof webdriver.promise.CancellationError)) {
426447
flow.pendingRejections_ += 1;
427448
pendingRejectionKey = flow.timer.setTimeout(function() {
428449
pendingRejectionKey = null;
@@ -457,6 +478,8 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
457478
*/
458479
var promise = new webdriver.promise.Promise();
459480

481+
var self = this;
482+
460483
/**
461484
* Registers a callback on this Deferred.
462485
*
@@ -482,7 +505,9 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
482505
pendingRejectionKey = null;
483506
}
484507

485-
var deferred = new webdriver.promise.Deferred(cancel, flow);
508+
var deferred = new webdriver.promise.Deferred(flow);
509+
deferred.parent_ = self;
510+
486511
var listener = {
487512
callback: opt_callback,
488513
errback: opt_errback,
@@ -500,8 +525,6 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
500525
return deferred.promise;
501526
}
502527

503-
var self = this;
504-
505528
/**
506529
* Resolves this promise with the given value. If the value is itself a
507530
* promise and not a reference to this deferred, this instance will wait for
@@ -525,18 +548,17 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
525548
/**
526549
* Attempts to cancel the computation of this instance's value. This attempt
527550
* will silently fail if this instance has already resolved.
528-
* @param {*=} opt_reason The reason for cancelling this promise.
529-
*/
551+
* @param {string=} opt_reason The reason for cancelling this promise. */
530552
function cancel(opt_reason) {
531553
if (!isPending()) {
532554
return;
533555
}
534556

535-
if (opt_canceller) {
536-
opt_reason = opt_canceller(opt_reason) || opt_reason;
557+
if (self.parent_) {
558+
self.parent_.cancel(opt_reason);
559+
} else {
560+
reject(new webdriver.promise.CancellationError(opt_reason));
537561
}
538-
539-
reject(opt_reason);
540562
}
541563

542564
this.promise = promise;
@@ -630,24 +652,22 @@ webdriver.promise.isPromise = function(value) {
630652
*/
631653
webdriver.promise.delayed = function(ms) {
632654
var timer = webdriver.promise.controlFlow().timer;
633-
var key;
634-
var deferred = new webdriver.promise.Deferred(function() {
655+
var deferred = new webdriver.promise.Deferred();
656+
var key = timer.setTimeout(deferred.fulfill, ms);
657+
return deferred.thenCatch(function(e) {
635658
timer.clearTimeout(key);
659+
throw e;
636660
});
637-
key = timer.setTimeout(deferred.fulfill, ms);
638-
return deferred.promise;
639661
};
640662

641663

642664
/**
643665
* Creates a new deferred object.
644-
* @param {Function=} opt_canceller Function to call when cancelling the
645-
* computation of this instance's value.
646666
* @return {!webdriver.promise.Deferred.<T>} The new deferred object.
647667
* @template T
648668
*/
649-
webdriver.promise.defer = function(opt_canceller) {
650-
return new webdriver.promise.Deferred(opt_canceller);
669+
webdriver.promise.defer = function() {
670+
return new webdriver.promise.Deferred();
651671
};
652672

653673

@@ -694,9 +714,7 @@ webdriver.promise.rejected = function(opt_reason) {
694714
* result of the provided function's callback.
695715
*/
696716
webdriver.promise.checkedNodeCall = function(fn, var_args) {
697-
var deferred = new webdriver.promise.Deferred(function() {
698-
throw Error('This Deferred may not be cancelled');
699-
});
717+
var deferred = new webdriver.promise.Deferred();
700718
try {
701719
var args = goog.array.slice(arguments, 1);
702720
args.push(function(error, value) {
@@ -1661,13 +1679,12 @@ webdriver.promise.ControlFlow.prototype.runInNewFrame_ = function(
16611679
errback(e);
16621680
};
16631681
} catch (ex) {
1664-
removeNewFrame(new webdriver.promise.CanceledTaskError_(ex));
1682+
removeNewFrame(ex);
16651683
errback(ex);
16661684
}
16671685

16681686
/**
1669-
* @param {webdriver.promise.CanceledTaskError_=} opt_err If provided, the
1670-
* error that triggered the removal of this frame.
1687+
* @param {*=} opt_err If provided, the reason that the frame was removed.
16711688
*/
16721689
function removeNewFrame(opt_err) {
16731690
var parent = newFrame.getParent();
@@ -1676,7 +1693,8 @@ webdriver.promise.ControlFlow.prototype.runInNewFrame_ = function(
16761693
}
16771694

16781695
if (opt_err) {
1679-
newFrame.cancelRemainingTasks(opt_err);
1696+
newFrame.cancelRemainingTasks(
1697+
'Tasks cancelled due to uncaught error: ' + opt_err);
16801698
}
16811699
self.activeFrame_ = oldFrame;
16821700
}
@@ -1858,7 +1876,8 @@ webdriver.promise.Frame_.prototype.getRoot = function() {
18581876
* @param {*} error The error that triggered this abortion.
18591877
*/
18601878
webdriver.promise.Frame_.prototype.abort = function(error) {
1861-
this.cancelRemainingTasks(new webdriver.promise.CanceledTaskError_(error));
1879+
this.cancelRemainingTasks(
1880+
'Task discarded due to a previous task failure: ' + error);
18621881

18631882
var frame = this;
18641883
frame.flow_.pendingRejections_ += 1;
@@ -1915,23 +1934,22 @@ webdriver.promise.Frame_.prototype.notify_ = function(fn, opt_error) {
19151934
* });
19161935
* });
19171936
* // flow failed: Error: boom
1918-
* // task failed! CanceledTaskError: Task discarded due to a previous
1937+
* // task failed! CancelledTaskError: Task discarded due to a previous
19191938
* // task failure: Error: boom
19201939
*
1921-
* @param {!webdriver.promise.CanceledTaskError_} error The cancellation
1922-
* error.
1940+
* @param {string} reason The cancellation reason.
19231941
*/
1924-
webdriver.promise.Frame_.prototype.cancelRemainingTasks = function(error) {
1942+
webdriver.promise.Frame_.prototype.cancelRemainingTasks = function(reason) {
19251943
goog.array.forEach(this.children_, function(child) {
19261944
if (child instanceof webdriver.promise.Frame_) {
1927-
child.cancelRemainingTasks(error);
1945+
child.cancelRemainingTasks(reason);
19281946
} else {
19291947
// None of the previously registered listeners should be notified that
19301948
// the task is being canceled, however, we need at least one errback
19311949
// to prevent the cancellation from bubbling up.
19321950
child.removeAll();
19331951
child.thenCatch(goog.nullFunction);
1934-
child.cancel(error);
1952+
child.cancel(reason);
19351953
}
19361954
});
19371955
};
@@ -2043,7 +2061,7 @@ webdriver.promise.Frame_.prototype.toString = function() {
20432061
* @private
20442062
*/
20452063
webdriver.promise.Task_ = function(flow, fn, description, snapshot) {
2046-
webdriver.promise.Deferred.call(this, null, flow);
2064+
webdriver.promise.Deferred.call(this, flow);
20472065

20482066
/**
20492067
* @type {function(): (T|!webdriver.promise.Promise.<T>)}
@@ -2080,25 +2098,6 @@ webdriver.promise.Task_.prototype.toString = function() {
20802098

20812099

20822100

2083-
/**
2084-
* Special error used to signal when a task is canceled because a previous
2085-
* task in the same frame failed.
2086-
* @param {*} err The error that caused the task cancellation.
2087-
* @constructor
2088-
* @extends {goog.debug.Error}
2089-
* @private
2090-
*/
2091-
webdriver.promise.CanceledTaskError_ = function(err) {
2092-
goog.base(this, 'Task discarded due to a previous task failure: ' + err);
2093-
};
2094-
goog.inherits(webdriver.promise.CanceledTaskError_, goog.debug.Error);
2095-
2096-
2097-
/** @override */
2098-
webdriver.promise.CanceledTaskError_.prototype.name = 'CanceledTaskError';
2099-
2100-
2101-
21022101
/**
21032102
* The default flow to use if no others are active.
21042103
* @private {!webdriver.promise.ControlFlow}

Diff for: javascript/webdriver/test/promise_flow_test.js

+12-4
Original file line numberDiff line numberDiff line change
@@ -1797,8 +1797,12 @@ function testEventLoopWaitsOnPendingPromiseRejections_multipleRejections() {
17971797
}
17981798

17991799
function testCancelsPromiseReturnedByCallbackIfFrameFails_promiseCallback() {
1800-
var chainPair = callbackPair(null, assertIsStubError);
1801-
var deferredPair = callbackPair(null, assertIsStubError);
1800+
var isCancellationError = function(e) {
1801+
assertEquals('CancellationError: Error: ouch', e.toString());
1802+
};
1803+
1804+
var chainPair = callbackPair(null, isCancellationError);
1805+
var deferredPair = callbackPair(null, isCancellationError);
18021806

18031807
var d = new webdriver.promise.Deferred();
18041808
d.then(deferredPair.callback, deferredPair.errback);
@@ -1818,8 +1822,12 @@ function testCancelsPromiseReturnedByCallbackIfFrameFails_promiseCallback() {
18181822
}
18191823

18201824
function testCancelsPromiseReturnedByCallbackIfFrameFails_taskCallback() {
1821-
var chainPair = callbackPair(null, assertIsStubError);
1822-
var deferredPair = callbackPair(null, assertIsStubError);
1825+
var isCancellationError = function(e) {
1826+
assertEquals('CancellationError: Error: ouch', e.toString());
1827+
};
1828+
1829+
var chainPair = callbackPair(null, isCancellationError);
1830+
var deferredPair = callbackPair(null, isCancellationError);
18231831

18241832
var d = new webdriver.promise.Deferred();
18251833
d.then(deferredPair.callback, deferredPair.errback);

0 commit comments

Comments
 (0)