From 39266654e6a006603a50ddd154c6d3a2be254946 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20H=C3=A4ni?= <haeni.sebastian@gmail.com>
Date: Fri, 30 Oct 2020 19:09:06 +0100
Subject: [PATCH] fix(http): queue jsonp <script> tag onLoad event handler in
 microtask (#39512)

Before this change, when trying to load a JSONP script that calls the JSONP callback inside a
microtask, it will fail in Internet Explorer 11 and EdgeHTML. This commit changes the onLoad cleanup
to be queued after the loaded endpoint executed any potential microtask itself. This ensures that
the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and
only then run the cleanup inside onLoad.

Fixes #39496

PR Close #39512
---
 packages/common/http/src/jsonp.ts       | 56 +++++++++++++++----------
 packages/common/http/test/jsonp_spec.ts | 10 +++++
 2 files changed, 43 insertions(+), 23 deletions(-)

diff --git a/packages/common/http/src/jsonp.ts b/packages/common/http/src/jsonp.ts
index e2e0360cc0..aefedf20c0 100644
--- a/packages/common/http/src/jsonp.ts
+++ b/packages/common/http/src/jsonp.ts
@@ -50,6 +50,11 @@ export abstract class JsonpCallbackContext {
  */
 @Injectable()
 export class JsonpClientBackend implements HttpBackend {
+  /**
+   * A resolved promise that can be used to schedule microtasks in the event handlers.
+   */
+  private readonly resolvedPromise = Promise.resolve();
+
   constructor(private callbackMap: JsonpCallbackContext, @Inject(DOCUMENT) private document: any) {}
 
   /**
@@ -140,33 +145,38 @@ export class JsonpClientBackend implements HttpBackend {
           return;
         }
 
-        // Cleanup the page.
-        cleanup();
+        // We wrap it in an extra Promise, to ensure the microtask
+        // is scheduled after the loaded endpoint has executed any potential microtask itself,
+        // which is not guaranteed in Internet Explorer and EdgeHTML. See issue #39496
+        this.resolvedPromise.then(() => {
+          // Cleanup the page.
+          cleanup();
 
-        // Check whether the response callback has run.
-        if (!finished) {
-          // It hasn't, something went wrong with the request. Return an error via
-          // the Observable error path. All JSONP errors have status 0.
-          observer.error(new HttpErrorResponse({
+          // Check whether the response callback has run.
+          if (!finished) {
+            // It hasn't, something went wrong with the request. Return an error via
+            // the Observable error path. All JSONP errors have status 0.
+            observer.error(new HttpErrorResponse({
+              url,
+              status: 0,
+              statusText: 'JSONP Error',
+              error: new Error(JSONP_ERR_NO_CALLBACK),
+            }));
+            return;
+          }
+
+          // Success. body either contains the response body or null if none was
+          // returned.
+          observer.next(new HttpResponse({
+            body,
+            status: 200,
+            statusText: 'OK',
             url,
-            status: 0,
-            statusText: 'JSONP Error',
-            error: new Error(JSONP_ERR_NO_CALLBACK),
           }));
-          return;
-        }
 
-        // Success. body either contains the response body or null if none was
-        // returned.
-        observer.next(new HttpResponse({
-          body,
-          status: 200,
-          statusText: 'OK',
-          url,
-        }));
-
-        // Complete the stream, the response is over.
-        observer.complete();
+          // Complete the stream, the response is over.
+          observer.complete();
+        });
       };
 
       // onError() is the error callback, which runs if the script returned generates
diff --git a/packages/common/http/test/jsonp_spec.ts b/packages/common/http/test/jsonp_spec.ts
index 0c5888f381..10b0e6574d 100644
--- a/packages/common/http/test/jsonp_spec.ts
+++ b/packages/common/http/test/jsonp_spec.ts
@@ -45,6 +45,16 @@ const SAMPLE_REQ = new HttpRequest<never>('JSONP', '/test');
       runOnlyCallback(home, {data: 'This is a test'});
       document.mockLoad();
     });
+    // Issue #39496
+    it('handles a request with callback call wrapped in promise', done => {
+      backend.handle(SAMPLE_REQ).subscribe(() => {
+        done();
+      });
+      Promise.resolve().then(() => {
+        runOnlyCallback(home, {data: 'This is a test'});
+      });
+      document.mockLoad();
+    });
     it('handles an error response properly', done => {
       const error = new Error('This is a test error');
       backend.handle(SAMPLE_REQ).pipe(toArray()).subscribe(undefined, (err: HttpErrorResponse) => {