diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts index cd31d4d3a3c16a..c072ab7402a7c0 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(); @@ -1451,6 +1452,7 @@ export class ChatPetWidget extends Disposable { const wasInitialized = this._enablementInitialized; this._enablementInitialized = true; this._enabled = enabled; + this._observeHost(this._host.read(undefined)); if (enabled) { if (isDead) { this._showRespawnSequence(); @@ -1558,9 +1560,11 @@ export class ChatPetWidget extends Disposable { private _observeHost(host: IChatPetWidgetHost): void { const store = new DisposableStore(); - store.add(this._resizeObserver.observe(host.dragBounds)); - store.add(this._resizeObserver.observe(host.movementBounds)); - store.add(this._resizeObserver.observe(host.parent)); + if (this._enabled) { + store.add(this._resizeObserver.observe(host.dragBounds)); + store.add(this._resizeObserver.observe(host.movementBounds)); + store.add(this._resizeObserver.observe(host.parent)); + } store.add(host.onDidChangePlatform(() => this._updatePlatformPosition())); this._hostLayoutDisposables.value = store; } 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 da89a6db0b867a..079e0dd154570e 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() { }(), @@ -130,6 +131,46 @@ 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 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( + createPetHost(parent, dragBounds, movementBounds), + TestResizeObserver as unknown as typeof ResizeObserver, + 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('stacks the run cycle behind the input', () => { const parent = mainWindow.document.createElement('div'); const input = mainWindow.document.createElement('div'); @@ -144,6 +185,7 @@ suite('ChatPetWidget', () => { service.toggle(); disposables.add(new ChatPetWidget( createPetHost(parent, input, movementBounds), + undefined, service, new class extends TestAccessibilityService { override isMotionReduced(): boolean { return false; } @@ -223,6 +265,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() { }(), @@ -449,6 +492,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() { }(), @@ -646,6 +690,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 66b474587de1df..e42e8bb512c27d 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts @@ -38,6 +38,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'; @@ -829,6 +830,70 @@ 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, + { + parent: petHost, + dragBounds, + movementBounds, + model: constObservable(undefined), + hasInput: constObservable(false), + inputChanged: Event.None, + getPlatformTop: () => undefined, + onDidChangePlatform: Event.None, + }, + undefined, + )); + + 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 }), @@ -854,6 +919,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([]); +});