[Improve] Dispose view listeners and ignore stale views when the webview is replaced (#2213)

* fix(webview): dispose view listeners and ignore stale views when the webview is replaced

* fix(webview): handle disposal during resolve and test stale listener guards

* refactor(webview): remove unused provider disposables field and cleanup loop

---------

Co-authored-by: Roomote <roomote@roomote.dev>
Co-authored-by: Elliott de Launay <edelauna@gmail.com>

Author: 3 people

src/core/webview/ClineProvider.ts

@@ -219,7 +219,6 @@ export class ClineProvider
 	public static readonly sideBarId = `${Package.name}.SidebarProvider`
 	public static readonly tabPanelId = `${Package.name}.TabPanelProvider`
 	private static activeInstances: Set<ClineProvider> = new Set()
-	private disposables: vscode.Disposable[] = []
 	private webviewDisposables: vscode.Disposable[] = []
 	private pendingThemeFixtureProbes = new Map<
 		string,
@@ -837,6 +836,15 @@ export class ClineProvider
 		}
 	}
 
+	/**
+	 * True when an in-flight resolve no longer belongs to the provider's current
+	 * view because the view was disposed or replaced (or the provider itself was
+	 * disposed) while the resolve was awaiting HTML or state hydration.
+	 */
+	private isStaleViewResolve(webviewView: vscode.WebviewView | vscode.WebviewPanel): boolean {
+		return this._disposed || this.view !== webviewView
+	}
+
 	/** Drain one task's memoized cleanup without preventing the remaining provider shutdown work. */
 	private async drainTaskDisposal(task: Task): Promise<void> {
 		try {
@@ -893,14 +901,6 @@ export class ClineProvider
 			CloudService.instance.off("settings-updated", this.handleCloudSettingsUpdate)
 		}
 
-		while (this.disposables.length) {
-			const x = this.disposables.pop()
-
-			if (x) {
-				x.dispose()
-			}
-		}
-
 		this._workspaceTracker?.dispose()
 		this._workspaceTracker = undefined
 		await this.mcpHub?.unregisterClient()
@@ -1030,9 +1030,40 @@ export class ClineProvider
 	}
 
 	async resolveWebviewView(webviewView: vscode.WebviewView | vscode.WebviewPanel) {
+		// A different view replaces the current one: dispose its subscriptions so
+		// the replaced view can no longer dispatch messages or refresh provider state.
+		if (this.view !== webviewView) {
+			this.clearWebviewResources()
+		}
+
 		this.view = webviewView
 		const inTabMode = "onDidChangeViewState" in webviewView
 
+		// Register disposal before awaiting HTML or state so closing the view during
+		// either load invalidates this resolve and prevents later listener installation.
+		webviewView.onDidDispose(
+			async () => {
+				// A view that is no longer current must not clear the current
+				// view's resources or dispose the provider.
+				if (webviewView !== this.view) {
+					return
+				}
+
+				if (inTabMode) {
+					this.log("Disposing ClineProvider instance for tab view")
+					await this.dispose()
+				} else {
+					this.log("Clearing webview resources for sidebar view")
+					this.view = undefined
+					this.clearWebviewResources()
+				}
+			},
+			null,
+			// View-scoped, like the other view subscriptions: disposed both when
+			// the view is disposed and when a different view replaces it.
+			this.webviewDisposables,
+		)
+
 		if (inTabMode) {
 			setPanel(webviewView, "tab")
 		} else if ("onDidChangeVisibility" in webviewView) {
@@ -1052,12 +1083,20 @@ export class ClineProvider
 			localResourceRoots: resourceRoots,
 		}
 
-		webviewView.webview.html =
+		const html =
 			this.contextProxy.extensionMode === vscode.ExtensionMode.Development &&
 			process.env.ROO_CODE_THEME_FIXTURE_PROBE !== "1"
 				? await this.getHMRHtmlContent(webviewView.webview)
 				: await this.getHtmlContent(webviewView.webview)
 
+		// The HTML load above can finish after the view was disposed or replaced.
+		// A stale resolve must not assign HTML to a view the provider no longer owns.
+		if (this.isStaleViewResolve(webviewView)) {
+			return
+		}
+
+		webviewView.webview.html = html
+
 		// Initialize out-of-scope variables that need to receive persistent
 		// global state values.
 		await this.getState().then(
@@ -1088,9 +1127,15 @@ export class ClineProvider
 			},
 		)
 
+		// State hydration can also finish after the view was disposed or replaced.
+		// A stale resolve must not install listeners on a view the provider no longer owns.
+		if (this.isStaleViewResolve(webviewView)) {
+			return
+		}
+
 		// Sets up an event listener to listen for messages passed from the webview view context
 		// and executes code based on the message that is received.
-		this.setWebviewMessageListener(webviewView.webview)
+		this.setWebviewMessageListener(webviewView)
 		this.webviewDisposables.push(this.webviewFocusTracker.init(this, webviewView))
 
 		// Listen for when the panel becomes visible.
@@ -1099,6 +1144,11 @@ export class ClineProvider
 			// WebviewView and WebviewPanel have all the same properties except
 			// for this visibility listener panel.
 			const viewStateDisposable = webviewView.onDidChangeViewState(() => {
+				// Ignore events from a view that is no longer the current view.
+				if (webviewView !== this.view) {
+					return
+				}
+
 				if (this.view?.visible) {
 					void this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
 				} else {
@@ -1110,6 +1160,11 @@ export class ClineProvider
 		} else if ("onDidChangeVisibility" in webviewView) {
 			// sidebar
 			const visibilityDisposable = webviewView.onDidChangeVisibility(() => {
+				// Ignore events from a view that is no longer the current view.
+				if (webviewView !== this.view) {
+					return
+				}
+
 				if (this.view?.visible) {
 					void this.postMessageToWebview({ type: "action", action: "didBecomeVisible" })
 				} else {
@@ -1120,24 +1175,13 @@ export class ClineProvider
 			this.webviewDisposables.push(visibilityDisposable)
 		}
 
-		// Listen for when the view is disposed
-		// This happens when the user closes the view or when the view is closed programmatically
-		webviewView.onDidDispose(
-			async () => {
-				if (inTabMode) {
-					this.log("Disposing ClineProvider instance for tab view")
-					await this.dispose()
-				} else {
-					this.log("Clearing webview resources for sidebar view")
-					this.clearWebviewResources()
-				}
-			},
-			null,
-			this.disposables,
-		)
-
 		// Listen for when color changes
 		const configDisposable = vscode.workspace.onDidChangeConfiguration(async (e) => {
+			// Ignore configuration refreshes scoped to a view that is no longer current.
+			if (webviewView !== this.view) {
+				return
+			}
+
 			if (e && e.affectsConfiguration("workbench.colorTheme")) {
 				// Sends latest theme name to webview
 				await this.postMessageToWebview({ type: "theme", text: JSON.stringify(await getTheme()) })
@@ -1704,13 +1748,19 @@ export class ClineProvider
 	 * Sets up an event listener to listen for messages passed from the webview context and
 	 * executes code based on the message that is received.
 	 *
-	 * @param webview A reference to the extension webview
+	 * @param webviewView The view whose webview messages are listened to
 	 */
-	private setWebviewMessageListener(webview: vscode.Webview) {
-		const onReceiveMessage = async (message: WebviewMessage) =>
-			webviewMessageHandler(this, message, this.marketplaceManager)
+	private setWebviewMessageListener(webviewView: vscode.WebviewView | vscode.WebviewPanel) {
+		const onReceiveMessage = async (message: WebviewMessage) => {
+			// Ignore messages from a view that is no longer the current view.
+			if (webviewView !== this.view) {
+				return
+			}
+
+			await webviewMessageHandler(this, message, this.marketplaceManager)
+		}
 
-		const messageDisposable = webview.onDidReceiveMessage(onReceiveMessage)
+		const messageDisposable = webviewView.webview.onDidReceiveMessage(onReceiveMessage)
 		this.webviewDisposables.push(messageDisposable)
 	}
 

src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts

@@ -224,8 +224,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 				asWebviewUri: vi.fn(),
 			},
 			visible: true,
-			onDidDispose: vi.fn().mockImplementation((callback) => {
-				callback()
+			onDidDispose: vi.fn().mockImplementation(() => {
 				return { dispose: vi.fn() }
 			}),
 			onDidChangeVisibility: vi.fn().mockImplementation(() => {

src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts

@@ -316,8 +316,7 @@ describe("ClineProvider - Lock API Config Across Modes", () => {
 				cspSource: "vscode-webview://test-csp-source",
 			},
 			visible: true,
-			onDidDispose: vi.fn().mockImplementation((callback) => {
-				callback()
+			onDidDispose: vi.fn().mockImplementation(() => {
 				return { dispose: vi.fn() }
 			}),
 			onDidChangeVisibility: vi.fn().mockImplementation(() => {

src/core/webview/__tests__/ClineProvider.spec.ts

@@ -536,8 +536,7 @@ describe("ClineProvider", () => {
 				cspSource: "vscode-webview://test-csp-source",
 			},
 			visible: true,
-			onDidDispose: vi.fn().mockImplementation((callback) => {
-				callback()
+			onDidDispose: vi.fn().mockImplementation(() => {
 				return { dispose: vi.fn() }
 			}),
 			onDidChangeVisibility: vi.fn().mockImplementation(() => {
@@ -4512,8 +4511,7 @@ describe("ClineProvider - Router Models", () => {
 				asWebviewUri: vi.fn(),
 			},
 			visible: true,
-			onDidDispose: vi.fn().mockImplementation((callback) => {
-				callback()
+			onDidDispose: vi.fn().mockImplementation(() => {
 				return { dispose: vi.fn() }
 			}),
 			onDidChangeVisibility: vi.fn().mockImplementation(() => {
@@ -4911,8 +4909,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
 				asWebviewUri: vi.fn(),
 			},
 			visible: true,
-			onDidDispose: vi.fn().mockImplementation((callback) => {
-				callback()
+			onDidDispose: vi.fn().mockImplementation(() => {
 				return { dispose: vi.fn() }
 			}),
 			onDidChangeVisibility: vi.fn().mockImplementation(() => {