[BUGFIX] Avoid race condition in DocumentService.ready()

Improve document-service responsiveness by relying on
`DOMContentLoaded` and `document.readyState` >= `interactive`.

1) Handle non-loading state as "ready" to avoid waiting for `complete`.

   `document.readyState` has three states:

    * `loading`
       The document is still loading.

    * `interactive`
       The document has finished loading and the document
       has been parsed but sub-resources such as scripts, images,
       stylesheets and frames are still loading. The state indicates
       that the DOMContentLoaded event is about to fire.

    * `complete`
       The document and all sub-resources have finished loading.
       The state indicates that the load event is about to fire.

   If DocumentService.ready was called in "interactive" state we have
   been skipping this state as we only considered `complete` to be
   the "ready" state in this case.
   This is wrong as we actually wait for the DOMContentLoaded if we
   are launched in the initial readyState (`loading`), that means
   an initial `interactive` must be understood as:

     `DOMContentLoaded` has already been fired as state is `interactive`

   We should actually avoid waiting for `complete` entirely, as document
   readyState `interactive` means "ready" in terms of document parsing.
   We're not interested in images or async scripts that are still
   loading and want event listeners to be registered as early as
   possible.

   With this improvement applied we can also drop the (now) unneeded
   `load` event-listener, as we are no longer skipping interactive
   state (this made the `load`-listener necessary previously).

   Note that the `load` workaround was previously needed for IE<=10 as
   those versions set `document.readyState` to `interactive` prior to
   the `DOMContentLoaded` event. This applied workaround becomes a race
   condition once `load` never happens and that is the case with a
   recent google chrome regression which causes iframe documents to
   never "complete": https://issues.chromium.org/issues/347724924

2) Drop the timeout logic. It is no longer needed now that we only need
   to wait for `DOMContentLoaded` – which will always fire (or already
   fired) – instead of the `load` event.

3) Avoid creation of new promises for every invocation of the
   ready-method. A promise can be reused without side effects by
   multiple consumers. This avoids creating a lot of event listeners
   (and previously timers).

4) Remove unneeded document and window references.

Releases: main, 12.4, 11.5
Resolves: #104135
Related: #104139
Change-Id: I42c86961405f8a5c346c17ea429a288a85d58f8a
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84753


Tested-by: Benjamin Franzke <ben@bnf.dev>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: core-ci <typo3@b13.com>

Build/Sources/TypeScript/core/document-service.ts

@@ -16,42 +16,18 @@
  * @exports @typo3/core/document-service
  */
 class DocumentService {
-  private readonly windowRef: Window;
-  private readonly documentRef: Document;
+  private promise: Promise<Document> = null;
 
-  /**
-   * @param {Window} windowRef
-   * @param {Document} documentRef
-   */
-  constructor(windowRef: Window = window, documentRef: Document = document) {
-    this.windowRef = windowRef;
-    this.documentRef = documentRef;
+  public ready(): Promise<Document> {
+    return this.promise ?? (this.promise = this.createPromise());
   }
 
-  ready(): Promise<Document> {
-    return new Promise<Document>((resolve: (document: Document) => void, reject: (document: Document) => void) => {
-      if (this.documentRef.readyState === 'complete') {
-        resolve(this.documentRef);
-      } else {
-        // timeout & reject after 30 seconds
-        const timer = setTimeout((): void => {
-          clearListeners();
-          reject(this.documentRef);
-        }, 30000);
-        const clearListeners = (): void => {
-          this.windowRef.removeEventListener('load', delegate);
-          this.documentRef.removeEventListener('DOMContentLoaded', delegate);
-        };
-        const delegate = (): void => {
-          clearListeners();
-          clearTimeout(timer);
-          resolve(this.documentRef);
-        };
-        this.windowRef.addEventListener('load', delegate);
-        this.documentRef.addEventListener('DOMContentLoaded', delegate);
-      }
-
-    });
+  private async createPromise(): Promise<Document> {
+    if (document.readyState !== 'loading') {
+      return document;
+    }
+    await new Promise<void>(resolve => document.addEventListener('DOMContentLoaded', () => resolve(), { once: true }));
+    return document;
   }
 }
 

typo3/sysext/core/Resources/Public/JavaScript/document-service.js

@@ -10,4 +10,4 @@
  *
  * The TYPO3 project - inspiring people to share!
  */
-class DocumentService{constructor(e=window,t=document){this.windowRef=e,this.documentRef=t}ready(){return new Promise(((e,t)=>{if("complete"===this.documentRef.readyState)e(this.documentRef);else{const n=setTimeout((()=>{o(),t(this.documentRef)}),3e4),o=()=>{this.windowRef.removeEventListener("load",d),this.documentRef.removeEventListener("DOMContentLoaded",d)},d=()=>{o(),clearTimeout(n),e(this.documentRef)};this.windowRef.addEventListener("load",d),this.documentRef.addEventListener("DOMContentLoaded",d)}}))}}const documentService=new DocumentService;export default documentService;
\ No newline at end of file
+class DocumentService{constructor(){this.promise=null}ready(){return this.promise??(this.promise=this.createPromise())}async createPromise(){return"loading"!==document.readyState||await new Promise((e=>document.addEventListener("DOMContentLoaded",(()=>e()),{once:!0}))),document}}const documentService=new DocumentService;export default documentService;
\ No newline at end of file