From 27d24ff69ad557c18c5db21ce47d3ca4ca9c23a7 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Tue, 6 Dec 2016 23:32:05 +0900 Subject: [PATCH 1/6] fix bug add event listener to xhrhttprequest multiple times #527 --- lib/browser/browser.ts | 12 ++++++++++-- test/browser/XMLHttpRequest.spec.ts | 12 ++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index 79f297dfb..85e6a1618 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -48,6 +48,7 @@ patchXHR(_global); const XHR_TASK = zoneSymbol('xhrTask'); const XHR_SYNC = zoneSymbol('xhrSync'); +const XHR_LISTENER = zoneSymbol('xhrListener'); interface XHROptions extends TaskData { target: any; @@ -63,13 +64,20 @@ function patchXHR(window: any) { function scheduleTask(task: Task) { var data = task.data; - data.target.addEventListener('readystatechange', () => { + // remove existing event listener + var listener = data.target[XHR_LISTENER]; + if (listener) { + data.target.removeEventListener('readystatechange', listener); + } + var newListener = data.target[XHR_LISTENER] = () => { if (data.target.readyState === data.target.DONE) { if (!data.aborted) { task.invoke(); } } - }); + }; + data.target.addEventListener('readystatechange', newListener); + var storedTask: Task = data.target[XHR_TASK]; if (!storedTask) { data.target[XHR_TASK] = task; diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index be1c63fd2..8cceb9d43 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -182,4 +182,16 @@ describe('XMLHttpRequest', function() { expect(XMLHttpRequest.LOADING).toEqual(3); expect(XMLHttpRequest.DONE).toEqual(4); }); + + it('should work properly when send request multiple times on single xmlRequest instance', function() { + testZone.run(function() { + var req = new XMLHttpRequest(); + req.open('get', '/', true); + req.send(); + req.onloadend = function() { + req.open('get', '/', true); + req.send(); + } + }); + }) }); From 7fa1ae0b2c01008412aa3af8d572c9cae191c340 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Wed, 7 Dec 2016 12:34:55 +0900 Subject: [PATCH 2/6] fix #287 with a better solution to prevent memory leak --- lib/browser/browser.ts | 11 ++++------- test/browser/XMLHttpRequest.spec.ts | 8 +++++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index 85e6a1618..b653735e9 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -48,7 +48,6 @@ patchXHR(_global); const XHR_TASK = zoneSymbol('xhrTask'); const XHR_SYNC = zoneSymbol('xhrSync'); -const XHR_LISTENER = zoneSymbol('xhrListener'); interface XHROptions extends TaskData { target: any; @@ -65,18 +64,16 @@ function patchXHR(window: any) { function scheduleTask(task: Task) { var data = task.data; // remove existing event listener - var listener = data.target[XHR_LISTENER]; - if (listener) { - data.target.removeEventListener('readystatechange', listener); - } - var newListener = data.target[XHR_LISTENER] = () => { + var listener = () => { if (data.target.readyState === data.target.DONE) { if (!data.aborted) { task.invoke(); } + // remove listener after xhr request is done. + data.target.removeEventListener('readystatechange', listener); } }; - data.target.addEventListener('readystatechange', newListener); + data.target.addEventListener('readystatechange', listener); var storedTask: Task = data.target[XHR_TASK]; if (!storedTask) { diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 8cceb9d43..08457abbf 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -24,12 +24,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]) + expect(wtfMock.log[wtfMock.log.length - 6]) .toMatch(/\> Zone\:invokeTask.*addEventListener\:readystatechange/); - expect(wtfMock.log[wtfMock.log.length - 4]) + expect(wtfMock.log[wtfMock.log.length - 5]) .toEqual('> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); - expect(wtfMock.log[wtfMock.log.length - 3]) + expect(wtfMock.log[wtfMock.log.length - 4]) .toEqual('< Zone:invokeTask:XMLHttpRequest.send'); + expect(wtfMock.log[wtfMock.log.length - 3]) + .toMatch(/# Zone\:cancel.*addEventListener\:readystatechange/); expect(wtfMock.log[wtfMock.log.length - 2]) .toMatch(/\< Zone\:invokeTask.*addEventListener\:readystatechange/); done(); From 54e1efd87a4d587c5ab26767dfcff02e5aed772b Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Wed, 7 Dec 2016 13:07:33 +0900 Subject: [PATCH 3/6] rollback to previous change --- lib/browser/browser.ts | 11 +++++++---- test/browser/XMLHttpRequest.spec.ts | 8 +++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index b653735e9..85e6a1618 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -48,6 +48,7 @@ patchXHR(_global); const XHR_TASK = zoneSymbol('xhrTask'); const XHR_SYNC = zoneSymbol('xhrSync'); +const XHR_LISTENER = zoneSymbol('xhrListener'); interface XHROptions extends TaskData { target: any; @@ -64,16 +65,18 @@ function patchXHR(window: any) { function scheduleTask(task: Task) { var data = task.data; // remove existing event listener - var listener = () => { + var listener = data.target[XHR_LISTENER]; + if (listener) { + data.target.removeEventListener('readystatechange', listener); + } + var newListener = data.target[XHR_LISTENER] = () => { if (data.target.readyState === data.target.DONE) { if (!data.aborted) { task.invoke(); } - // remove listener after xhr request is done. - data.target.removeEventListener('readystatechange', listener); } }; - data.target.addEventListener('readystatechange', listener); + data.target.addEventListener('readystatechange', newListener); var storedTask: Task = data.target[XHR_TASK]; if (!storedTask) { diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 08457abbf..8cceb9d43 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -24,14 +24,12 @@ 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 - 6]) - .toMatch(/\> Zone\:invokeTask.*addEventListener\:readystatechange/); expect(wtfMock.log[wtfMock.log.length - 5]) - .toEqual('> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); + .toMatch(/\> Zone\:invokeTask.*addEventListener\:readystatechange/); expect(wtfMock.log[wtfMock.log.length - 4]) - .toEqual('< Zone:invokeTask:XMLHttpRequest.send'); + .toEqual('> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); expect(wtfMock.log[wtfMock.log.length - 3]) - .toMatch(/# Zone\:cancel.*addEventListener\:readystatechange/); + .toEqual('< Zone:invokeTask:XMLHttpRequest.send'); expect(wtfMock.log[wtfMock.log.length - 2]) .toMatch(/\< Zone\:invokeTask.*addEventListener\:readystatechange/); done(); From 24d0e95144049945ec6f600b13d62bb1bd945788 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Thu, 8 Dec 2016 12:59:31 +0900 Subject: [PATCH 4/6] fix #530 for xhr request to an invalid url cause Schedule task count failed --- lib/browser/browser.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index 85e6a1618..74c34313b 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -49,6 +49,7 @@ patchXHR(_global); const XHR_TASK = zoneSymbol('xhrTask'); const XHR_SYNC = zoneSymbol('xhrSync'); const XHR_LISTENER = zoneSymbol('xhrListener'); +const XHR_SCHEDULED = zoneSymbol('xhrScheduled'); interface XHROptions extends TaskData { target: any; @@ -63,6 +64,7 @@ function patchXHR(window: any) { } function scheduleTask(task: Task) { + self[XHR_SCHEDULED] = false; var data = task.data; // remove existing event listener var listener = data.target[XHR_LISTENER]; @@ -71,7 +73,7 @@ function patchXHR(window: any) { } var newListener = data.target[XHR_LISTENER] = () => { if (data.target.readyState === data.target.DONE) { - if (!data.aborted) { + if (!data.aborted && self[XHR_SCHEDULED]) { task.invoke(); } } @@ -83,6 +85,7 @@ function patchXHR(window: any) { data.target[XHR_TASK] = task; } sendNative.apply(data.target, data.args); + self[XHR_SCHEDULED] = true; return task; } From e0a9330078a0ecc9a8228d4beeb2208469153e47 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Thu, 8 Dec 2016 13:19:02 +0900 Subject: [PATCH 5/6] add test case --- test/browser/XMLHttpRequest.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 8cceb9d43..bef4ccc9b 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -194,4 +194,12 @@ describe('XMLHttpRequest', function() { } }); }) + + it('should work properly when send request to an invalid instance', function() { + testZone.run(function() { + var req = new XMLHttpRequest(); + req.open('get', 'file:///test', true); + req.send(); + }); + }); }); From a2ddf8af54deb792fd397e8eeba690fce9e386a5 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Thu, 8 Dec 2016 13:34:26 +0900 Subject: [PATCH 6/6] remove test case --- test/browser/XMLHttpRequest.spec.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index bef4ccc9b..8cceb9d43 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -194,12 +194,4 @@ describe('XMLHttpRequest', function() { } }); }) - - it('should work properly when send request to an invalid instance', function() { - testZone.run(function() { - var req = new XMLHttpRequest(); - req.open('get', 'file:///test', true); - req.send(); - }); - }); });