From 2aab9c816bc5764fdfe16dbd3b9385c1c7c3d0b9 Mon Sep 17 00:00:00 2001
From: JiaLiPassion <JiaLi.Passion@gmail.com>
Date: Tue, 19 Jun 2018 01:57:38 +0900
Subject: [PATCH] fix(xhr): should invoke xhr task after onload is triggered
 (#1055)

---
 lib/browser/browser.ts              | 24 ++++++++++++++++++++++-
 test/browser/XMLHttpRequest.spec.ts | 30 +++++++++++++++++++++--------
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts
index 9b72338bd..d4873797a 100644
--- a/lib/browser/browser.ts
+++ b/lib/browser/browser.ts
@@ -144,7 +144,29 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType) => {
           // sometimes on some browsers XMLHttpRequest will fire onreadystatechange with
           // readyState=4 multiple times, so we need to check task state here
           if (!data.aborted && (XMLHttpRequest as any)[XHR_SCHEDULED] && task.state === SCHEDULED) {
-            task.invoke();
+            // check whether the xhr has registered onload listener
+            // if that is the case, the task should invoke after all
+            // onload listeners finish.
+            const loadTasks = target['__zone_symbol__loadfalse'];
+            if (loadTasks && loadTasks.length > 0) {
+              const oriInvoke = task.invoke;
+              task.invoke = function() {
+                // need to load the tasks again, because in other
+                // load listener, they may remove themselves
+                const loadTasks = target['__zone_symbol__loadfalse'];
+                for (let i = 0; i < loadTasks.length; i++) {
+                  if (loadTasks[i] === task) {
+                    loadTasks.splice(i, 1);
+                  }
+                }
+                if (!data.aborted && task.state === SCHEDULED) {
+                  oriInvoke.call(task);
+                }
+              };
+              loadTasks.push(task);
+            } else {
+              task.invoke();
+            }
           }
         }
       };
diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts
index 0e34ccc74..a1b59cff0 100644
--- a/test/browser/XMLHttpRequest.spec.ts
+++ b/test/browser/XMLHttpRequest.spec.ts
@@ -17,23 +17,37 @@ describe('XMLHttpRequest', function() {
 
   it('should intercept XHRs and treat them as MacroTasks', function(done) {
     let req: XMLHttpRequest;
-    const testZoneWithWtf =
-        Zone.current.fork((Zone as any)['wtfZoneSpec']).fork({name: 'TestZone'});
+    let onStable: any;
+    const testZoneWithWtf = Zone.current.fork((Zone as any)['wtfZoneSpec']).fork({
+      name: 'TestZone',
+      onHasTask: (delegate: ZoneDelegate, curr: Zone, target: Zone, hasTask: HasTaskState) => {
+        if (!hasTask.macroTask) {
+          onStable && onStable();
+        }
+      }
+    });
 
     testZoneWithWtf.run(() => {
       req = new XMLHttpRequest();
+      const logs: string[] = [];
       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 - 3])
-            .toEqual('> Zone:invokeTask:XMLHttpRequest.send("<root>::ProxyZone::WTF::TestZone")');
+        logs.push('onload');
+      };
+      onStable = function() {
         expect(wtfMock.log[wtfMock.log.length - 2])
+            .toEqual('> Zone:invokeTask:XMLHttpRequest.send("<root>::ProxyZone::WTF::TestZone")');
+        expect(wtfMock.log[wtfMock.log.length - 1])
             .toEqual('< Zone:invokeTask:XMLHttpRequest.send');
         if (supportPatchXHROnProperty()) {
-          expect(wtfMock.log[wtfMock.log.length - 1])
+          expect(wtfMock.log[wtfMock.log.length - 3])
+              .toMatch(/\< Zone\:invokeTask.*addEventListener\:load/);
+          expect(wtfMock.log[wtfMock.log.length - 4])
               .toMatch(/\> Zone\:invokeTask.*addEventListener\:load/);
         }
+        // if browser can patch onload
+        if ((req as any)['__zone_symbol__loadfalse']) {
+          expect(logs).toEqual(['onload']);
+        }
         done();
       };