Merge pull request #376 from auth0/delay-iframe-removal

[SDK-1127] Delay removal of iframe to prevent Chrome hanging status bug #240

Author: adamjmcgrath

__tests__/cache.test.ts

@@ -2,6 +2,24 @@ import Cache from '../src/cache';
 
 describe('cache', () => {
   let cache: Cache;
+  let OriginalDate: Date;
+
+  beforeEach(() => {
+    OriginalDate = (<any>global).Date;
+    (<any>global).Date = class {
+      time: number;
+      constructor(time: number) {
+        this.time = time;
+      }
+      getTime() {
+        return this.time || 0;
+      }
+    };
+  });
+  afterEach(() => {
+    (<any>global).Date = OriginalDate;
+  });
+
   beforeEach(() => {
     cache = new Cache();
     jest.useFakeTimers();

__tests__/utils.test.ts

@@ -502,8 +502,9 @@ describe('utils', () => {
         }
       };
       const { iframe, url } = setup(message);
+      jest.useFakeTimers();
       await runIframe(url, origin);
-
+      jest.runAllTimers();
       expect(message.source.close).toHaveBeenCalled();
       expect(window.document.body.appendChild).toHaveBeenCalledWith(iframe);
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
@@ -524,20 +525,10 @@ describe('utils', () => {
       ].forEach(m => {
         it(`ignores invalid messages: ${JSON.stringify(m)}`, async () => {
           const { iframe, url, origin } = setup(m);
-          /**
-           * We need to run the timers after we start `runIframe` to simulate
-           * the window event listener, but we also need to use `jest.useFakeTimers`
-           * to trigger the timeout. That's why we're using a real `setTimeout`,
-           * then using fake timers then rolling back to real timers
-           */
-          setTimeout(() => {
-            jest.runAllTimers();
-          }, 10);
           jest.useFakeTimers();
-          await expect(runIframe(url, origin)).rejects.toMatchObject(
-            TIMEOUT_ERROR
-          );
-          jest.useRealTimers();
+          const promise = runIframe(url, origin);
+          jest.runAllTimers();
+          await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR);
           expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
         });
       });
@@ -553,9 +544,11 @@ describe('utils', () => {
         }
       };
       const { iframe, url } = setup(message);
+      jest.useFakeTimers();
       await expect(runIframe(url, origin)).resolves.toMatchObject(
         message.data.response
       );
+      jest.runAllTimers();
       expect(message.source.close).toHaveBeenCalled();
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
     });
@@ -570,30 +563,22 @@ describe('utils', () => {
         }
       };
       const { iframe, url } = setup(message);
+      jest.useFakeTimers();
       await expect(runIframe(url, origin)).rejects.toMatchObject(
         message.data.response
       );
+      jest.runAllTimers();
       expect(message.source.close).toHaveBeenCalled();
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
     });
     it('times out after timeoutInSeconds', async () => {
       const { iframe, url, origin } = setup('');
       const seconds = 10;
-      /**
-       * We need to run the timers after we start `runIframe` to simulate
-       * the window event listener, but we also need to use `jest.useFakeTimers`
-       * to trigger the timeout. That's why we're using a real `setTimeout`,
-       * then using fake timers then rolling back to real timers
-       */
-      setTimeout(() => {
-        jest.runTimersToTime(seconds * 1000);
-      }, 10);
       jest.useFakeTimers();
-      await expect(runIframe(url, origin, seconds)).rejects.toMatchObject(
-        TIMEOUT_ERROR
-      );
+      const promise = runIframe(url, origin, seconds);
+      jest.runTimersToTime(seconds * 1000);
+      await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR);
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
-      jest.useRealTimers();
     });
   });
   describe('getCrypto', () => {

src/constants.ts

@@ -3,6 +3,11 @@
  */
 export const DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS = 60;
 
+/**
+ * @ignore
+ */
+export const CLEANUP_IFRAME_TIMEOUT_IN_SECONDS = 2;
+
 /**
  * @ignore
  */

src/utils.ts

@@ -1,6 +1,9 @@
 import fetch from 'unfetch';
 
-import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants';
+import {
+  DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS,
+  CLEANUP_IFRAME_TIMEOUT_IN_SECONDS
+} from './constants';
 
 const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i);
 
@@ -54,7 +57,12 @@ export const runIframe = (
       e.data.response.error ? rej(e.data.response) : res(e.data.response);
       clearTimeout(timeoutSetTimeoutId);
       window.removeEventListener('message', iframeEventHandler, false);
-      window.document.body.removeChild(iframe);
+      // Delay the removal of the iframe to prevent hanging loading status
+      // in Chrome: https://github.com/auth0/auth0-spa-js/issues/240
+      setTimeout(
+        () => window.document.body.removeChild(iframe),
+        CLEANUP_IFRAME_TIMEOUT_IN_SECONDS * 1000
+      );
     };
     window.addEventListener('message', iframeEventHandler, false);
     window.document.body.appendChild(iframe);