From fd39f97e20fe49028590b4f3f94f13a2249fdc46 Mon Sep 17 00:00:00 2001 From: Julie Ralph Date: Tue, 22 Mar 2016 17:36:54 -0700 Subject: [PATCH] feat: treat XHRs as macrotasks Now, when an XHR is sent it is treated as a pending macrotask. ZoneSpecs, such as the AsyncTestZoneSpec, rely on this to ensure that XHRs are completed. The macrotask is cancelled if the XHR is aborted, and completed when the XHR reaches state DONE. --- lib/browser/browser.ts | 72 ++++++++++++++++++++++++++++- lib/zone-spec/async-test.ts | 28 +++-------- test/async-test.spec.ts | 51 ++++++++++++++++++++ test/browser/XMLHttpRequest.spec.ts | 65 ++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 24 deletions(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index a1c8e7107..f824c65ae 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -3,7 +3,7 @@ import {eventTargetPatch} from './event-target'; import {propertyPatch} from './define-property'; import {registerElementPatch} from './register-element'; import {propertyDescriptorPatch} from './property-descriptor'; -import {patchMethod, patchPrototype, patchClass} from "./utils"; +import {patchMethod, patchPrototype, patchClass, zoneSymbol} from "./utils"; const set = 'set'; const clear = 'clear'; @@ -15,7 +15,7 @@ patchTimer(_global, set, clear, 'Interval'); patchTimer(_global, set, clear, 'Immediate'); patchTimer(_global, 'request', 'cancelMacroTask', 'AnimationFrame'); patchTimer(_global, 'mozRequest', 'mozCancel', 'AnimationFrame'); -patchTimer(_global, 'webkitRequest', 'webkitCancel', 'AnimationFrame') +patchTimer(_global, 'webkitRequest', 'webkitCancel', 'AnimationFrame'); for (var i = 0; i < blockingMethods.length; i++) { var name = blockingMethods[i]; @@ -34,6 +34,74 @@ patchClass('FileReader'); propertyPatch(); registerElementPatch(_global); +// Treat XMLHTTPRequest as a macrotask. +patchXHR(_global); + +const XHR_TASK = zoneSymbol('xhrTask'); + +interface XHROptions extends TaskData { + target: any, + args: any[], + aborted: boolean +} + +function patchXHR(window: any) { + function findPendingTask(target: any) { + var pendingTask: Task = target[XHR_TASK]; + return pendingTask; + } + + function scheduleTask(task: Task) { + var data = task.data; + data.target.addEventListener('readystatechange', () => { + if (data.target.readyState === XMLHttpRequest.DONE) { + if (!data.aborted) { + task.invoke(); + } + } + }); + var storedTask: Task = data.target[XHR_TASK]; + if (!storedTask) { + data.target[XHR_TASK] = task; + } + setNative.apply(data.target, data.args); + return task; + } + + function placeholderCallback() { + } + + function clearTask(task: Task) { + var data = task.data; + // Note - ideally, we would call data.target.removeEventListener here, but it's too late + // to prevent it from firing. So instead, we store info for the event listener. + data.aborted = true; + return clearNative.apply(data.target, data.args); + } + + var setNative = patchMethod(window.XMLHttpRequest.prototype, 'send', () => function(self: any, args: any[]) { + var zone = Zone.current; + + var options: XHROptions = { + target: self, + isPeriodic: false, + delay: null, + args: args, + aborted: false + }; + return zone.scheduleMacroTask('XMLHttpRequest.send', placeholderCallback, options, scheduleTask, clearTask); + }); + + var clearNative = patchMethod(window.XMLHttpRequest.prototype, 'abort', (delegate: Function) => function(self: any, args: any[]) { + var task: Task = findPendingTask(self); + if (task && typeof task.type == 'string') { + task.zone.cancelTask(task); + } else { + throw new Error('tried to abort an XHR which has not yet been sent'); + } + }); +} + /// GEO_LOCATION if (_global['navigator'] && _global['navigator'].geolocation) { patchPrototype(_global['navigator'].geolocation, [ diff --git a/lib/zone-spec/async-test.ts b/lib/zone-spec/async-test.ts index 1036782d8..8f5086bc1 100644 --- a/lib/zone-spec/async-test.ts +++ b/lib/zone-spec/async-test.ts @@ -16,28 +16,13 @@ _finishCallbackIfDone() { if (!(this._pendingMicroTasks || this._pendingMacroTasks)) { // We do this because we would like to catch unhandled rejected promises. - // To do this quickly when there are native promises, we must run using an unwrapped - // promise implementation. - var symbol = (Zone).__symbol__; - var NativePromise: typeof Promise = window[symbol('Promise')]; - if (NativePromise) { - NativePromise.resolve(true)[symbol('then')](() => { - if (!this._alreadyErrored) { - this.runZone.run(this._finishCallback); + this.runZone.run(() => { + setTimeout(() => { + if (!this._alreadyErrored && !(this._pendingMicroTasks || this._pendingMacroTasks)) { + this._finishCallback(); } - }); - } else { - // For implementations which do not have nativePromise, use setTimeout(0). This is slower, - // but it also works because Zones will handle errors when rejected promises have no - // listeners after one macrotask. - this.runZone.run(() => { - setTimeout(() => { - if (!this._alreadyErrored) { - this._finishCallback(); - } - }, 0); - }); - } + }, 0); + }); } } @@ -79,7 +64,6 @@ onHasTask(delegate: ZoneDelegate, current: Zone, target: Zone, hasTaskState: HasTaskState) { delegate.hasTask(target, hasTaskState); - if (hasTaskState.change == 'microTask') { this._pendingMicroTasks = hasTaskState.microTask; this._finishCallbackIfDone(); diff --git a/test/async-test.spec.ts b/test/async-test.spec.ts index a29124f41..23bd6e98a 100644 --- a/test/async-test.spec.ts +++ b/test/async-test.spec.ts @@ -173,6 +173,33 @@ describe('AsyncTestZoneSpec', function() { }); }); + it('should wait for XHRs to complete', function(done) { + var req; + var finished = false; + + var testZoneSpec = new AsyncTestZoneSpec(() => { + expect(finished).toBe(true); + done(); + }, (err) => { + done.fail('async zone called failCallback unexpectedly'); + }, 'name'); + + var atz = Zone.current.fork(testZoneSpec); + + atz.run(function() { + req = new XMLHttpRequest(); + + req.onreadystatechange = () => { + if (req.readyState === XMLHttpRequest.DONE) { + finished = true; + } + }; + + req.open('get', '/', true); + req.send(); + }); + }); + it('should fail if setInterval is used', (done) => { var finished = false; @@ -227,6 +254,30 @@ describe('AsyncTestZoneSpec', function() { }); }); + + it('should fail if an xhr fails', function(done) { + var req; + + var testZoneSpec = new AsyncTestZoneSpec(() => { + done.fail('expected failCallback to be called'); + }, (err) => { + expect(err).toEqual('bad url failure'); + done(); + }, 'name'); + + var atz = Zone.current.fork(testZoneSpec); + + atz.run(function() { + req = new XMLHttpRequest(); + req.onload = () => { + if (req.status != 200) { + throw new Error('bad url failure'); + } + } + req.open('get', '/bad-url', true); + req.send(); + }); + }); }); export var __something__; diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 71b8ca494..3ac302c5b 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -3,6 +3,34 @@ import {ifEnvSupports} from '../util'; describe('XMLHttpRequest', function () { var testZone = Zone.current.fork({name: 'test'}); + it('should intercept XHRs and treat them as MacroTasks', function(done) { + var req: any; + var testZoneWithWtf = Zone.current.fork(Zone['wtfZoneSpec']).fork({ name: 'TestZone' }); + testZoneWithWtf.run(() => { + req = new XMLHttpRequest(); + req.onload = () => { + // The last entry in the log should be the invocation for the current onload, + // which will vary depending on browser environment. The prior entries + // should be the invocation of the send macrotask. + expect(wtfMock.log[wtfMock.log.length - 5]).toMatch( + /\> Zone\:invokeTask.*addEventListener\:readystatechange/); + expect(wtfMock.log[wtfMock.log.length - 4]).toEqual( + '> Zone:invokeTask:XMLHttpRequest.send("::WTF::TestZone")'); + expect(wtfMock.log[wtfMock.log.length - 3]).toEqual( + '< Zone:invokeTask:XMLHttpRequest.send'); + expect(wtfMock.log[wtfMock.log.length - 2]).toMatch( + /\< Zone\:invokeTask.*addEventListener\:readystatechange/); + done(); + }; + + req.open('get', '/', true); + req.send(); + + var lastScheduled = wtfMock.log[wtfMock.log.length - 1]; + expect(lastScheduled).toMatch('# Zone:schedule:macroTask:XMLHttpRequest.send'); + }, null, null, 'unit-test'); + }); + it('should work with onreadystatechange', function (done) { var req; @@ -42,6 +70,43 @@ describe('XMLHttpRequest', function () { req.send(); }); + + it('should allow canceling of an XMLHttpRequest', function(done) { + var spy = jasmine.createSpy('spy'); + var req; + var pending = false; + + var trackingTestZone = Zone.current.fork({ + name: 'tracking test zone', + onHasTask: (delegate: ZoneDelegate, current: Zone, target: Zone, hasTaskState: HasTaskState) => { + if (hasTaskState.change == 'macroTask') { + pending = hasTaskState.macroTask; + } + delegate.hasTask(target, hasTaskState); + } + }); + + trackingTestZone.run(function() { + req = new XMLHttpRequest(); + req.onreadystatechange = function() { + if (req.readyState === XMLHttpRequest.DONE) { + if (req.status !== 0) { + spy(); + } + } + }; + req.open('get', '/', true); + + req.send(); + req.abort(); + }); + + setTimeout(function() { + expect(spy).not.toHaveBeenCalled(); + expect(pending).toEqual(false); + done(); + }, 0); + }); })); it('should preserve other setters', function () {