Fix memory leak in m-frame loading progress (#178) (#182)

mml-io · Jul 2, 2024 · 13c5ff1 · 13c5ff1
1 parent b63e712
commit 13c5ff1
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 18 deletions.
diff --git a/packages/mml-web/src/elements/Model.ts b/packages/mml-web/src/elements/Model.ts
@@ -435,6 +435,7 @@ export class Model extends TransformableElement {
       Model.disposeOfGroup(this.loadedState.group);
       this.loadedState = null;
     }
+    this.latestSrcModelPromise = null;
     this.srcLoadingInstanceManager.dispose();
     this.animLoadingInstanceManager.dispose();
     this.clearDebugVisualisation();

diff --git a/packages/mml-web/src/utils/CollideableHelper.ts b/packages/mml-web/src/utils/CollideableHelper.ts
@@ -119,6 +119,8 @@ export class CollideableHelper {
 
     scene.removeCollider?.(this.collider, this.element);
 
+    this.collider = null;
+
     this.scene = null;
   }
 

diff --git a/packages/mml-web/src/utils/frame/StaticHTMLFrameInstance.ts b/packages/mml-web/src/utils/frame/StaticHTMLFrameInstance.ts
@@ -13,7 +13,8 @@ export class StaticHTMLFrameInstance {
   private readonly remoteDocumentWrapper: RemoteDocumentWrapper;
   private readonly targetForWrapper: MElement;
   private readonly scene: IMMLScene;
-  private loadingProgressManager: LoadingProgressManager;
+  private loadingProgressManagerForFrameContent: LoadingProgressManager;
+  private parentLoadingProgressManager?: LoadingProgressManager | null;
 
   static parser = new DOMParser();
 
@@ -22,23 +23,24 @@ export class StaticHTMLFrameInstance {
     this.src = src;
     this.scene = scene;
     this.container = new THREE.Group();
+    this.parentLoadingProgressManager = scene.getLoadingProgressManager?.();
 
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const windowTarget = targetElement.ownerDocument.defaultView!;
 
-    this.loadingProgressManager = new LoadingProgressManager();
-    this.loadingProgressManager.addProgressCallback(() => {
-      scene.getLoadingProgressManager?.()?.updateDocumentProgress(this);
+    this.loadingProgressManagerForFrameContent = new LoadingProgressManager();
+    this.loadingProgressManagerForFrameContent.addProgressCallback(() => {
+      this.parentLoadingProgressManager?.updateDocumentProgress(this);
     });
 
     scene
       .getLoadingProgressManager?.()
-      ?.addLoadingDocument(this, this.src, this.loadingProgressManager);
+      ?.addLoadingDocument(this, this.src, this.loadingProgressManagerForFrameContent);
 
     const wrappedScene: IMMLScene = createWrappedScene(
       this.scene,
       this.container,
-      this.loadingProgressManager,
+      this.loadingProgressManagerForFrameContent,
     );
 
     const address = this.targetForWrapper.contentSrcToContentAddress(this.src);
@@ -62,7 +64,7 @@ export class StaticHTMLFrameInstance {
       response = await fetch(address);
     } catch (err) {
       console.error("Failed to fetch static MML page", err);
-      this.loadingProgressManager.setInitialLoad(err);
+      this.loadingProgressManagerForFrameContent.setInitialLoad(err);
       return;
     }
     const text = await response.text();
@@ -72,11 +74,11 @@ export class StaticHTMLFrameInstance {
     );
     DOMSanitizer.sanitise(remoteDocumentAsHTMLNode.body);
     this.remoteDocumentWrapper.remoteDocument.append(remoteDocumentAsHTMLNode.body);
-    this.loadingProgressManager.setInitialLoad(true);
+    this.loadingProgressManagerForFrameContent.setInitialLoad(true);
   }
 
   public dispose() {
     this.targetForWrapper.removeChild(this.remoteDocumentWrapper.remoteDocument);
-    this.loadingProgressManager.removeLoadingDocument(this);
+    this.parentLoadingProgressManager?.removeLoadingDocument(this);
   }
 }
diff --git a/packages/mml-web/src/utils/frame/WebSocketFrameInstance.ts b/packages/mml-web/src/utils/frame/WebSocketFrameInstance.ts
@@ -16,13 +16,15 @@ export class WebSocketFrameInstance {
   private targetForWrapper: MElement;
   private scene: IMMLScene;
   private statusElement: THREE.Mesh | null = null;
-  private loadingProgressManager: LoadingProgressManager;
+  private loadingProgressManagerForFrameContent: LoadingProgressManager;
+  private parentLoadingProgressManager?: LoadingProgressManager | null;
 
   constructor(targetElement: MElement, src: string, scene: IMMLScene) {
     this.targetForWrapper = targetElement;
     this.src = src;
     this.scene = scene;
     this.container = new THREE.Group();
+    this.parentLoadingProgressManager = scene.getLoadingProgressManager?.();
 
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const windowTarget = targetElement.ownerDocument.defaultView!;
@@ -35,19 +37,19 @@ export class WebSocketFrameInstance {
       overriddenHandler(element, event);
     };
 
-    this.loadingProgressManager = new LoadingProgressManager();
-    this.loadingProgressManager.addProgressCallback(() => {
-      scene.getLoadingProgressManager?.()?.updateDocumentProgress(this);
+    this.loadingProgressManagerForFrameContent = new LoadingProgressManager();
+    this.loadingProgressManagerForFrameContent.addProgressCallback(() => {
+      this.parentLoadingProgressManager?.updateDocumentProgress(this);
     });
 
     scene
       .getLoadingProgressManager?.()
-      ?.addLoadingDocument(this, this.src, this.loadingProgressManager);
+      ?.addLoadingDocument(this, this.src, this.loadingProgressManagerForFrameContent);
 
     const wrappedScene: IMMLScene = createWrappedScene(
       this.scene,
       this.container,
-      this.loadingProgressManager,
+      this.loadingProgressManagerForFrameContent,
     );
 
     const websocketAddress = this.srcToAddress(this.src);
@@ -80,9 +82,9 @@ export class WebSocketFrameInstance {
           mesh.position.set(0, height / 2, 0);
           this.statusElement = mesh;
           this.container.add(this.statusElement);
-          this.loadingProgressManager.setInitialLoad(new Error("Failed to connect"));
+          this.loadingProgressManagerForFrameContent.setInitialLoad(new Error("Failed to connect"));
         } else if (status === NetworkedDOMWebsocketStatus.Connected) {
-          this.loadingProgressManager.setInitialLoad(true);
+          this.loadingProgressManagerForFrameContent.setInitialLoad(true);
         }
       },
     );
@@ -120,6 +122,6 @@ export class WebSocketFrameInstance {
   dispose() {
     this.domWebsocket.stop();
     this.targetForWrapper.removeChild(this.remoteDocumentWrapper.remoteDocument);
-    this.scene.getLoadingProgressManager?.()?.removeLoadingDocument(this);
+    this.parentLoadingProgressManager?.removeLoadingDocument(this);
   }
 }