From cb0c4a7be2bd6e3e4d0ec267498c6b782fbc4abc Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 24 Aug 2026 08:17:27 -0700 Subject: [PATCH 1/3] fix(chat): stop observing layout while pet is disabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aacd276e-cf84-48bd-a2ab-6f6a4d4c3431 --- .../chat/browser/widget/chatPetWidget.ts | 13 +++- .../test/browser/widget/chatPetWidget.test.ts | 49 ++++++++++++++ .../chat/chatWidget.fixture.ts | 66 +++++++++++++++++++ .../tests/chatPetResizeObserver.spec.ts | 26 ++++++++ 4 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 test/componentFixtures/playwright/tests/chatPetResizeObserver.spec.ts diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts index db852c03195efb..e7ad6e222701f4 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts @@ -1089,6 +1089,7 @@ export class ChatPetWidget extends Disposable { private _respawnPosition: readonly [number, number] | undefined; private _platformTopProvider: (() => number | undefined) | undefined; private readonly _resizeObserver: dom.DisposableResizeObserver; + private readonly _resizeObservations = this._register(new MutableDisposable()); private _variant: ChatPetVariant; private _selectedAccessory: ChatPetAccessoryId | undefined; private _scale = 1; @@ -1212,9 +1213,6 @@ export class ChatPetWidget extends Disposable { } } }, dom.getWindow(this._button.element))); - this._register(this._resizeObserver.observe(this.dragBounds)); - this._register(this._resizeObserver.observe(this.movementBounds)); - this._register(this._resizeObserver.observe(this.parent)); if (this._getHorizontalBounds() !== undefined) { this._updateVerticalPosition(); this._restoreHorizontalPosition(); @@ -1393,6 +1391,15 @@ export class ChatPetWidget extends Disposable { const wasInitialized = this._enablementInitialized; this._enablementInitialized = true; this._enabled = enabled; + if (enabled) { + const observations = new DisposableStore(); + observations.add(this._resizeObserver.observe(this.dragBounds)); + observations.add(this._resizeObserver.observe(this.movementBounds)); + observations.add(this._resizeObserver.observe(this.parent)); + this._resizeObservations.value = observations; + } else { + this._resizeObservations.clear(); + } if (enabled) { if (isDead) { this._showRespawnSequence(); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts index 49182a0d66580c..34c5e9e9c1955c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts @@ -123,6 +123,55 @@ suite('ChatPetWidget', () => { }); }); + test('observes layout bounds only while visible and enabled', () => { + const observedTargets = new Set(); + class TestResizeObserver implements ResizeObserver { + observe(target: Element): void { observedTargets.add(target); } + unobserve(target: Element): void { observedTargets.delete(target); } + disconnect(): void { observedTargets.clear(); } + takeRecords(): ResizeObserverEntry[] { return []; } + } + const originalResizeObserver = mainWindow.ResizeObserver; + Object.defineProperty(mainWindow, 'ResizeObserver', { configurable: true, value: TestResizeObserver }); + disposables.add(toDisposable(() => Object.defineProperty(mainWindow, 'ResizeObserver', { configurable: true, value: originalResizeObserver }))); + + const parent = mainWindow.document.createElement('div'); + const dragBounds = mainWindow.document.createElement('div'); + const movementBounds = mainWindow.document.createElement('div'); + mainWindow.document.body.append(parent, dragBounds, movementBounds); + disposables.add(toDisposable(() => { + parent.remove(); + dragBounds.remove(); + movementBounds.remove(); + })); + const service = disposables.add(new ChatPetService(disposables.add(new TestStorageService()), new TestTelemetryService(), new NullLogService())); + disposables.add(new ChatPetWidget( + parent, + dragBounds, + movementBounds, + constObservable(undefined), + constObservable(false), + constObservable(true), + Event.None, + service, + new TestAccessibilityService(), + new class extends mock() { }(), + new class extends mock() { }(), + new NullLogService(), + new class extends mock() { + override readonly hasFocus = true; + override readonly onDidChangeFocus = Event.None; + override readonly onDidChangeActiveWindow = Event.None; + }(), + )); + + assert.strictEqual(observedTargets.size, 0); + service.toggle(); + assert.deepStrictEqual(observedTargets, new Set([dragBounds, movementBounds, parent])); + service.toggle(); + assert.strictEqual(observedTargets.size, 0); + }); + test('repeats hops while key requests remain within the hold grace period', () => { const clock = sinon.useFakeTimers(); const { controller, events } = createHopHarness(); diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts index f2c153abc4ac70..49a1f4ec728ff1 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts @@ -37,6 +37,7 @@ import { MockChatService } from '../../../../contrib/chat/test/common/chatServic import { ComponentFixtureContext, createEditorServices, defineComponentFixture, defineThemedFixtureGroup } from '../fixtureUtils.js'; import { FixtureMenuService, registerChatFixtureServices } from './chatFixtureUtils.js'; import { ChatTurnStatusPillsSetting, isChatTurnStatusPillsEnabled } from '../../../../contrib/chat/browser/widget/chatTurnPills.js'; +import { ChatPetWidget } from '../../../../contrib/chat/browser/widget/chatPetWidget.js'; import '../../../../contrib/chat/browser/widget/media/chat.css'; @@ -820,6 +821,66 @@ async function renderResizeObserverLoopHarness(context: ComponentFixtureContext, })); } +async function renderDisabledPetResizeObserverProbe(context: ComponentFixtureContext): Promise { + const targetWindow = dom.getWindow(context.container); + const instantiationService = createEditorServices(context.disposableStore, { + colorTheme: context.theme, + additionalServices: registerChatFixtureServices, + }); + context.container.style.width = '720px'; + context.container.style.height = '600px'; + const movementBounds = dom.append(context.container, dom.$('.disabled-pet-movement-bounds')); + const petHost = dom.append(movementBounds, dom.$('.disabled-pet-host')); + const dragBounds = dom.append(petHost, dom.$('.disabled-pet-drag-bounds')); + const trigger = dom.append(dragBounds, dom.$('.disabled-pet-resize-observer-trigger')); + movementBounds.style.width = '100%'; + movementBounds.style.height = '200px'; + petHost.style.width = '100%'; + petHost.style.height = '100px'; + dragBounds.style.width = '100%'; + dragBounds.style.height = '100%'; + trigger.style.width = '10px'; + trigger.style.height = '10px'; + context.disposableStore.add(instantiationService.createInstance( + ChatPetWidget, + petHost, + dragBounds, + movementBounds, + constObservable(undefined), + constObservable(false), + constObservable(true), + Event.None, + )); + + const status = dom.append(context.container, dom.$('.disabled-pet-resize-observer-status')); + status.role = 'status'; + status.textContent = 'Running disabled pet observer probe'; + status.dataset['warningCount'] = '0'; + context.disposableStore.add(dom.addDisposableListener(targetWindow, dom.EventType.ERROR, event => { + if (event instanceof ErrorEvent && event.message.includes('ResizeObserver loop')) { + status.dataset['warningCount'] = String(Number(status.dataset['warningCount']) + 1); + status.dataset['observerContext'] = dom.getRecentDisposableResizeObserverContextForLoopError(event.message, targetWindow) ?? event.message; + } + })); + + let triggerCallbacks = 0; + const triggerObserver = context.disposableStore.add(new dom.DisposableResizeObserver('DisabledPetFixture.deepTrigger', () => { + triggerCallbacks++; + if (triggerCallbacks === 2) { + dragBounds.style.height = `${dragBounds.getBoundingClientRect().height + 1}px`; + } + }, targetWindow)); + context.disposableStore.add(triggerObserver.observe(trigger)); + + const nextFrame = () => new Promise(resolve => targetWindow.requestAnimationFrame(() => resolve())); + await nextFrame(); + await nextFrame(); + trigger.style.width = '11px'; + await nextFrame(); + await nextFrame(); + status.textContent = 'Completed disabled pet observer probe'; +} + export default defineThemedFixtureGroup({ path: 'chat/widget/' }, { SimpleQA: defineComponentFixture({ render: ctx => renderChatWidget(ctx, { messages: SIMPLE_QA }) }), ScrollToBottomAction: defineComponentFixture({ render: renderScrollToBottomAction }), @@ -845,6 +906,11 @@ export default defineThemedFixtureGroup({ path: 'chat/widget/' }, { virtualTime: { enabled: false }, render: context => renderResizeObserverLoopHarness(context, 'none'), }), + DisabledPetResizeObserverProbe: defineComponentFixture({ + labels: { kind: 'animated' }, + virtualTime: { enabled: false }, + render: renderDisabledPetResizeObserverProbe, + }), CodeBlockInList: defineComponentFixture({ render: ctx => renderChatWidget(ctx, { messages: CODE_BLOCK_IN_LIST }) }), bugs: defineThemedFixtureGroup({ 'issue-309796-missing-backslash': defineComponentFixture({ render: ctx => renderChatWidget(ctx, { messages: ISSUE_309796_MISSING_BACKSLASH }) }), diff --git a/test/componentFixtures/playwright/tests/chatPetResizeObserver.spec.ts b/test/componentFixtures/playwright/tests/chatPetResizeObserver.spec.ts new file mode 100644 index 00000000000000..4756e88f2911a8 --- /dev/null +++ b/test/componentFixtures/playwright/tests/chatPetResizeObserver.spec.ts @@ -0,0 +1,26 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { expect, test } from '@playwright/test'; +import { openFixture } from './utils.js'; + +test('does not observe chat layout while the pet is disabled', async ({ page }) => { + const resizeObserverErrors: string[] = []; + page.on('pageerror', error => { + if (error.message.includes('ResizeObserver loop')) { + resizeObserverErrors.push(error.message); + } + }); + + await openFixture(page, 'chat/widget/chatWidget/DisabledPetResizeObserverProbe/Dark', '.disabled-pet-resize-observer-status'); + await expect(page.getByRole('status')).toContainText('Completed'); + const status = page.locator('.disabled-pet-resize-observer-status'); + const warningCount = Number(await status.getAttribute('data-warning-count')); + const observerContext = await status.getAttribute('data-observer-context'); + console.log(`[disabled-pet-resize-observer] warnings: ${warningCount}; page errors: ${resizeObserverErrors.length}; observer context: ${observerContext}`); + + expect(warningCount).toBe(0); + expect(resizeObserverErrors).toEqual([]); +}); From 680fa78f8db5505febb3e7b933ae065f688545b0 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 24 Aug 2026 08:35:36 -0700 Subject: [PATCH 2/3] test(chat): update disabled pet fixture host Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aacd276e-cf84-48bd-a2ab-6f6a4d4c3431 --- .../chat/chatWidget.fixture.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts index 19beb51af15fb2..4f48ecd1c4ed6a 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts @@ -852,13 +852,16 @@ async function renderDisabledPetResizeObserverProbe(context: ComponentFixtureCon trigger.style.height = '10px'; context.disposableStore.add(instantiationService.createInstance( ChatPetWidget, - petHost, - dragBounds, - movementBounds, - constObservable(undefined), - constObservable(false), - constObservable(true), - Event.None, + { + parent: petHost, + dragBounds, + movementBounds, + model: constObservable(undefined), + hasInput: constObservable(false), + inputChanged: Event.None, + getPlatformTop: () => undefined, + onDidChangePlatform: Event.None, + }, )); const status = dom.append(context.container, dom.$('.disabled-pet-resize-observer-status')); From 18ce28e8b96e38c4c58782d0655c2d93e68928c5 Mon Sep 17 00:00:00 2001 From: Bryan Chen Date: Mon, 24 Aug 2026 09:38:41 -0700 Subject: [PATCH 3/3] test(chat): isolate pet resize observer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aacd276e-cf84-48bd-a2ab-6f6a4d4c3431 --- .../contrib/chat/browser/widget/chatPetWidget.ts | 3 ++- .../contrib/chat/browser/widget/chatPetWidgetService.ts | 2 +- .../chat/test/browser/widget/chatPetWidget.test.ts | 9 +++++---- .../browser/componentFixtures/chat/chatWidget.fixture.ts | 1 + 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts index d70dc47b17c47d..c0af66a8d2bf4e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts @@ -1159,6 +1159,7 @@ export class ChatPetWidget extends Disposable { constructor( host: IChatPetWidgetHost, + resizeObserverCtor: typeof ResizeObserver | undefined, @IChatPetService private readonly chatPetService: IChatPetService, @IAccessibilityService private readonly accessibilityService: IAccessibilityService, @IContextMenuService private readonly contextMenuService: IContextMenuService, @@ -1244,7 +1245,7 @@ export class ChatPetWidget extends Disposable { speechBubbleImage.alt = ''; speechBubbleImage.setAttribute('aria-hidden', 'true'); this._speechBubble = { container: speechBubbleContainer, image: speechBubbleImage, canvas: speechBubbleCanvas }; - this._resizeObserver = this._register(new dom.DisposableResizeObserver('ChatPetWidget.dragBounds', () => this._handleHostLayoutChange(), dom.getWindow(this._button.element))); + this._resizeObserver = this._register(new dom.DisposableResizeObserver('ChatPetWidget.dragBounds', () => this._handleHostLayoutChange(), dom.getWindow(this._button.element), { resizeObserverCtor })); this._observeHost(host); if (this._getHorizontalBounds() !== undefined) { this._restoreHorizontalPosition(); diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidgetService.ts b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidgetService.ts index b869eb05703ea6..a2acc4a57129e8 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidgetService.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidgetService.ts @@ -218,7 +218,7 @@ export class ChatPetWidgetService extends Disposable implements IChatPetWidgetSe ) { super(); this.coordinator = this._register(new ChatPetWidgetCoordinator( - host => instantiationService.createInstance(ChatPetWidget, host), + host => instantiationService.createInstance(ChatPetWidget, host, undefined), chatWidgetService, Event.map(dom.onWillUnregisterWindow, window => dom.getWindowId(window)), )); diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts index a5e281b5386957..7fb52692ccaa4f 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts @@ -105,6 +105,7 @@ suite('ChatPetWidget', () => { const service = disposables.add(new ChatPetService(disposables.add(new TestStorageService()), new TestTelemetryService(), new NullLogService())); disposables.add(new ChatPetWidget( createPetHost(parent, dragBounds, movementBounds), + undefined, service, new TestAccessibilityService(), new class extends mock() { }(), @@ -138,10 +139,6 @@ suite('ChatPetWidget', () => { disconnect(): void { observedTargets.clear(); } takeRecords(): ResizeObserverEntry[] { return []; } } - const originalResizeObserver = mainWindow.ResizeObserver; - Object.defineProperty(mainWindow, 'ResizeObserver', { configurable: true, value: TestResizeObserver }); - disposables.add(toDisposable(() => Object.defineProperty(mainWindow, 'ResizeObserver', { configurable: true, value: originalResizeObserver }))); - const parent = mainWindow.document.createElement('div'); const dragBounds = mainWindow.document.createElement('div'); const movementBounds = mainWindow.document.createElement('div'); @@ -154,6 +151,7 @@ suite('ChatPetWidget', () => { const service = disposables.add(new ChatPetService(disposables.add(new TestStorageService()), new TestTelemetryService(), new NullLogService())); disposables.add(new ChatPetWidget( createPetHost(parent, dragBounds, movementBounds), + TestResizeObserver as unknown as typeof ResizeObserver, service, new TestAccessibilityService(), new class extends mock() { }(), @@ -190,6 +188,7 @@ suite('ChatPetWidget', () => { const service = disposables.add(new ChatPetService(disposables.add(new TestStorageService()), new TestTelemetryService(), new NullLogService())); const widget = disposables.add(new ChatPetWidget( createPetHost(firstParent, firstBounds, movementBounds), + undefined, service, new TestAccessibilityService(), new class extends mock() { }(), @@ -416,6 +415,7 @@ suite('ChatPetWidget', () => { const service = disposables.add(new ChatPetService(disposables.add(new TestStorageService()), new TestTelemetryService(), new NullLogService())); disposables.add(new ChatPetWidget( createPetHost(parent, dragBounds, movementBounds), + undefined, service, new TestAccessibilityService(), new class extends mock() { }(), @@ -613,6 +613,7 @@ suite('ChatPetWidget', () => { const service = disposables.add(new ChatPetService(storageService, new TestTelemetryService(), new NullLogService())); const widget = disposables.add(new ChatPetWidget( createPetHost(parent, dragBounds, movementBounds), + undefined, service, new TestAccessibilityService(), new class extends mock() { }(), diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts index 4f48ecd1c4ed6a..e42e8bb512c27d 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts @@ -862,6 +862,7 @@ async function renderDisabledPetResizeObserverProbe(context: ComponentFixtureCon getPlatformTop: () => undefined, onDidChangePlatform: Event.None, }, + undefined, )); const status = dom.append(context.container, dom.$('.disabled-pet-resize-observer-status'));