Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

fix(xhr): inner onreadystatechange should not use zoneAwareListener #800

Merged
merged 1 commit into from
Jun 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/browser/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,11 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
const data = <XHROptions>task.data;
// remove existing event listener
const listener = data.target[XHR_LISTENER];
const oriAddListener = data.target[zoneSymbol('addEventListener')];
const oriRemoveListener = data.target[zoneSymbol('removeEventListener')];

if (listener) {
data.target.removeEventListener('readystatechange', listener);
oriRemoveListener.apply(data.target, ['readystatechange', listener]);
}
const newListener = data.target[XHR_LISTENER] = () => {
if (data.target.readyState === data.target.DONE) {
Expand All @@ -104,7 +107,7 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
}
}
};
data.target.addEventListener('readystatechange', newListener);
oriAddListener.apply(data.target, ['readystatechange', newListener]);

const storedTask: Task = data.target[XHR_TASK];
if (!storedTask) {
Expand Down
40 changes: 33 additions & 7 deletions test/browser/XMLHttpRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ifEnvSupports} from '../test-util';
import {ifEnvSupports, supportPatchXHROnProperty} from '../test-util';

describe('XMLHttpRequest', function() {
let testZone: Zone;
Expand All @@ -26,14 +26,14 @@ describe('XMLHttpRequest', function() {
// 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("<root>::ProxyZone::WTF::TestZone")');
expect(wtfMock.log[wtfMock.log.length - 3])
.toEqual('< Zone:invokeTask:XMLHttpRequest.send');
.toEqual('> Zone:invokeTask:XMLHttpRequest.send("<root>::ProxyZone::WTF::TestZone")');
expect(wtfMock.log[wtfMock.log.length - 2])
.toMatch(/\< Zone\:invokeTask.*addEventListener\:readystatechange/);
.toEqual('< Zone:invokeTask:XMLHttpRequest.send');
if (supportPatchXHROnProperty()) {
expect(wtfMock.log[wtfMock.log.length - 1])
.toMatch(/\> Zone\:invokeTask.*addEventListener\:load/);
}
done();
};

Expand All @@ -44,6 +44,32 @@ describe('XMLHttpRequest', function() {
}, null, null, 'unit-test');
});

it('should not trigger Zone callback of internal onreadystatechange', function(done) {
const scheduleSpy = jasmine.createSpy('schedule');
const xhrZone = Zone.current.fork({
name: 'xhr',
onScheduleTask: (delegate: ZoneDelegate, currentZone: Zone, targetZone, task: Task) => {
if (task.type === 'eventTask') {
scheduleSpy(task.source);
}
return delegate.scheduleTask(targetZone, task);
}
});

xhrZone.run(() => {
const req = new XMLHttpRequest();
req.onload = function() {
expect(Zone.current.name).toEqual('xhr');
if (supportPatchXHROnProperty()) {
expect(scheduleSpy).toHaveBeenCalled();
}
done();
};
req.open('get', '/', true);
req.send();
});
});

it('should work with onreadystatechange', function(done) {
let req: XMLHttpRequest;

Expand Down
11 changes: 11 additions & 0 deletions test/test-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,14 @@ function _runTest(test: any, block: Function, done: Function) {
});
}
}

export function supportPatchXHROnProperty() {
let desc = Object.getOwnPropertyDescriptor(XMLHttpRequest.prototype, 'onload');
if (!desc && (window as any)['XMLHttpRequestEventTarget']) {
desc = Object.getOwnPropertyDescriptor(global['XMLHttpRequestEventTarget'].prototype, 'onload');
}
if (!desc || !desc.configurable) {
return false;
}
return true;
}
6 changes: 5 additions & 1 deletion test/zone-spec/task-tracking.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import '../../lib/zone-spec/task-tracking';

import {supportPatchXHROnProperty} from '../test-util';

declare const global: any;

describe('TaskTrackingZone', function() {
Expand Down Expand Up @@ -47,7 +49,9 @@ describe('TaskTrackingZone', function() {
setTimeout(() => {
expect(taskTrackingZoneSpec.macroTasks.length).toBe(0);
expect(taskTrackingZoneSpec.microTasks.length).toBe(0);
expect(taskTrackingZoneSpec.eventTasks.length).not.toBe(0);
if (supportPatchXHROnProperty()) {
expect(taskTrackingZoneSpec.eventTasks.length).not.toBe(0);
}
taskTrackingZoneSpec.clearEvents();
expect(taskTrackingZoneSpec.eventTasks.length).toBe(0);
done();
Expand Down