Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 82 additions & 32 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@
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,
Expand Down Expand Up @@ -837,6 +836,15 @@
}
}

/**
* 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 {
Expand Down Expand Up @@ -893,14 +901,6 @@
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()
Expand Down Expand Up @@ -1030,9 +1030,40 @@
}

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) {

Check warning on line 1035 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:1035: Survived ConditionalExpression mutant (replacement: true). See the job summary for the complete list and resolution guidance.
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")

Check warning on line 1053 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:1053: Survived StringLiteral mutant (replacement: ""). See the job summary for the complete list and resolution guidance.
await this.dispose()
} else {
this.log("Clearing webview resources for sidebar view")

Check warning on line 1056 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:1056: Survived StringLiteral mutant (replacement: ""). See the job summary for the complete list and resolution guidance.
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) {
Expand All @@ -1052,12 +1083,20 @@
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(
Expand Down Expand Up @@ -1088,9 +1127,15 @@
},
)

// 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.
Expand All @@ -1099,6 +1144,11 @@
// 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 {
Expand All @@ -1110,6 +1160,11 @@
} 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 {
Expand All @@ -1120,24 +1175,13 @@
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()) })
Expand Down Expand Up @@ -1704,13 +1748,19 @@
* 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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
9 changes: 3 additions & 6 deletions src/core/webview/__tests__/ClineProvider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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(() => {
Expand Down
Loading
Loading