fix(notebooks): fix race condition in view registration (#13106)

Fixes a race condition that occurred when event handlers for `onDidRegisterNotebookSerializer`
were registered *after* said event was fired. This prevented the correct
rendering of a notebook the first time one was opened during the lifetime of a page.

Signed-off-by: Beniamino Ventura <[email protected]>

packages/notebook/src/browser/service/notebook-service.ts

@@ -14,7 +14,7 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { Disposable, DisposableCollection, Emitter, URI } from '@theia/core';
+import { Disposable, DisposableCollection, DisposableGroup, Emitter, URI } from '@theia/core';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import { BinaryBuffer } from '@theia/core/lib/common/buffer';
 import { NotebookData, TransientOptions } from '../../common';
@@ -61,7 +61,8 @@ export class NotebookService implements Disposable {
     protected readonly notebookModels = new Map<string, NotebookModel>();
 
     protected readonly didRegisterNotebookSerializerEmitter = new Emitter<string>();
-    readonly onDidRegisterNotebookSerializer = this.didRegisterNotebookSerializerEmitter.event;
+    protected readonly didRegisterNotebookSerializer: Disposable;
+    protected readonly replayViewTypes: Set<string> = new Set();
 
     protected readonly didRemoveViewTypeEmitter = new Emitter<string>();
     readonly onDidRemoveViewType = this.didRemoveViewTypeEmitter.event;
@@ -74,6 +75,31 @@ export class NotebookService implements Disposable {
     protected readonly didRemoveNotebookDocumentEmitter = new Emitter<NotebookModel>();
     readonly onDidRemoveNotebookDocument = this.didRemoveNotebookDocumentEmitter.event;
 
+    constructor() {
+        /*
+         * Handle registrations of notebook serializers that happen *before* any
+         * actual handler is added.
+         */
+        this.didRegisterNotebookSerializer = this.didRegisterNotebookSerializerEmitter.event((viewType: string) => this.replayViewTypes.add(viewType));
+    }
+
+    onDidRegisterNotebookSerializer(listener: (viewType: string) => unknown, thisArg?: unknown, disposables?: DisposableGroup): Disposable {
+        /*
+         * Replay events for the first handler that is added. Note that this
+         * will only happen once, so there is still the potential for *some*
+         * handlers to be registered after the event they're supposed to handle.
+         */
+        if (this.replayViewTypes.size !== 0) {
+            for (const viewType of this.replayViewTypes) {
+                setTimeout(listener.bind(thisArg, viewType));
+            }
+            this.replayViewTypes.clear();
+        }
+        this.didRegisterNotebookSerializer.dispose();
+
+        return this.didRegisterNotebookSerializerEmitter.event(listener, thisArg, disposables);
+    }
+
     dispose(): void {
         this.disposables.dispose();
     }
@@ -139,17 +165,24 @@ export class NotebookService implements Disposable {
         const deferred = new Deferred<NotebookProviderInfo | undefined>();
         // 20 seconds of timeout
         const timeoutDuration = 20_000;
-        const disposable = this.onDidRegisterNotebookSerializer(viewType => {
+
+        // Must declare these variables where they can be captured by the closure
+        let disposable: Disposable;
+        // eslint-disable-next-line
+        let timeout: ReturnType<typeof setTimeout>;
+
+        // eslint-disable-next-line
+        disposable = this.onDidRegisterNotebookSerializer(viewType => {
             if (viewType === type) {
                 clearTimeout(timeout);
                 disposable.dispose();
                 deferred.resolve(this.notebookProviders.get(type));
             }
         });
-        const timeout = setTimeout(() => {
+        timeout = setTimeout(() => {
             clearTimeout(timeout);
             disposable.dispose();
-            deferred.reject();
+            deferred.reject(new Error(`Timed out while waiting for notebook serializer for type ${type} to be registered`));
         }, timeoutDuration);
         return deferred.promise;
     }