fix(webview): retain instance when destroy fails (#2836)

* fix(webview): retain instance when destroy fails

* test(webview): cover destroy failure lifecycle

* fix(webview): serialize concurrent destruction

* test(webview): cover concurrent destruction

Author: azamkassim

src/lib/webview.js

@@ -59,6 +59,7 @@ class WebView {
 		this._messageCallbacks = [];
 		this._eventCallbacks = [];
 		this._destroyed = false;
+		this._destroyPromise = null;
 
 		instances.set(id, this);
 	}
@@ -126,11 +127,21 @@ class WebView {
 
 	async destroy() {
 		this._checkDestroyed();
-		this._destroyed = true;
-		await nativeBridge.destroy(this.id);
-		instances.delete(this.id);
-		this._messageCallbacks = [];
-		this._eventCallbacks = [];
+		if (!this._destroyPromise) {
+			this._destroyPromise = (async () => {
+				await nativeBridge.destroy(this.id);
+				this._destroyed = true;
+				instances.delete(this.id);
+				this._messageCallbacks = [];
+				this._eventCallbacks = [];
+			})();
+		}
+
+		try {
+			await this._destroyPromise;
+		} finally {
+			this._destroyPromise = null;
+		}
 	}
 
 	_checkDestroyed() {

tests/unit/webview.test.js

@@ -0,0 +1,71 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const { nativeBridge } = vi.hoisted(() => ({
+	nativeBridge: {
+		setMessageCallback: vi.fn(),
+		create: vi.fn(),
+		evaluate: vi.fn(),
+		destroy: vi.fn(),
+	},
+}));
+
+vi.mock("../../src/plugins/webview/www/webview", () => ({
+	default: nativeBridge,
+}));
+
+import webviewAPI from "lib/webview";
+
+describe("WebView lifecycle", () => {
+	beforeEach(() => {
+		vi.clearAllMocks();
+		nativeBridge.create.mockResolvedValue("webview-1");
+		nativeBridge.evaluate.mockResolvedValue("result");
+		nativeBridge.destroy.mockResolvedValue(undefined);
+	});
+
+	it("keeps the instance usable when native destruction fails", async () => {
+		const webview = await webviewAPI.create();
+		nativeBridge.destroy.mockRejectedValueOnce(new Error("native failure"));
+
+		await expect(webview.destroy()).rejects.toThrow("native failure");
+		await expect(webview.evaluate("1 + 1")).resolves.toBe("result");
+		expect(nativeBridge.evaluate).toHaveBeenCalledWith("webview-1", "1 + 1");
+
+		await expect(webview.destroy()).resolves.toBeUndefined();
+		expect(nativeBridge.destroy).toHaveBeenCalledTimes(2);
+	});
+
+	it("marks the instance destroyed only after native destruction succeeds", async () => {
+		const webview = await webviewAPI.create();
+
+		await webview.destroy();
+
+		await expect(webview.evaluate("1 + 1")).rejects.toThrow(
+			"WebView has been destroyed",
+		);
+	});
+
+	it("shares native destruction between concurrent callers", async () => {
+		let resolveDestroy;
+		nativeBridge.destroy.mockImplementationOnce(
+			() =>
+				new Promise((resolve) => {
+					resolveDestroy = resolve;
+				}),
+		);
+		const webview = await webviewAPI.create();
+
+		const firstDestroy = webview.destroy();
+		const secondDestroy = webview.destroy();
+
+		expect(nativeBridge.destroy).toHaveBeenCalledTimes(1);
+		resolveDestroy();
+		await expect(Promise.all([firstDestroy, secondDestroy])).resolves.toEqual([
+			undefined,
+			undefined,
+		]);
+		await expect(webview.evaluate("1 + 1")).rejects.toThrow(
+			"WebView has been destroyed",
+		);
+	});
+});