From 746bbd802986b72d93c4df8d74cd692033adf808 Mon Sep 17 00:00:00 2001 From: Ergun Erdogmus Date: Tue, 18 Feb 2025 12:56:25 +0100 Subject: [PATCH] [Animations] Fix animations freezing screencast view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an animation is captured, we start a screencast for taking its screenshots for preview and after finishing taking screenshots, we stop the screencast. However, if there already exists a screencast (i.e. if the user is inspecting a remote page using `chrome://inspect`), after the animations finished taking screenshots, they stop the screencast for `ScreencastView` too causing the view to freeze. This CL: * Adds ability to stack screencasts in screen capture model. When it receives a new `startScreencast`, it adds the existing screencast to a stack and serves to the newly added screencast. * Then, when the newly added screencast is stopped, it resumes the previous screencast from the stack. Fixed: 395838062 Change-Id: I7ee6df0cbb4e9f7927cb1cb250ceb69e78355a77 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6276143 Reviewed-by: Danil Somsikov Commit-Queue: Ergün Erdoğmuş --- front_end/core/sdk/AnimationModel.test.ts | 24 ++ front_end/core/sdk/AnimationModel.ts | 26 ++- front_end/core/sdk/BUILD.gn | 1 + front_end/core/sdk/ScreenCaptureModel.test.ts | 216 ++++++++++++++++++ front_end/core/sdk/ScreenCaptureModel.ts | 113 +++++++-- front_end/panels/screencast/ScreencastView.ts | 16 +- 6 files changed, 362 insertions(+), 34 deletions(-) create mode 100644 front_end/core/sdk/ScreenCaptureModel.test.ts diff --git a/front_end/core/sdk/AnimationModel.test.ts b/front_end/core/sdk/AnimationModel.test.ts index fb268fcee1..4e7d900e5d 100644 --- a/front_end/core/sdk/AnimationModel.test.ts +++ b/front_end/core/sdk/AnimationModel.test.ts @@ -254,4 +254,28 @@ describeWithMockConnection('AnimationModel', () => { assert.strictEqual(animationImpl.delayOrStartTime(), 0); // in pixels }); }); + + describe('ScreenshotCapture', () => { + let mockAnimationModel: SDK.AnimationModel.AnimationModel; + let mockScreenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel; + let startScreencastStub: + sinon.SinonStub>; + + beforeEach(() => { + startScreencastStub = sinon.stub(); + mockAnimationModel = sinon.createStubInstance(SDK.AnimationModel.AnimationModel); + mockScreenCaptureModel = sinon.createStubInstance(SDK.ScreenCaptureModel.ScreenCaptureModel, { + startScreencast: startScreencastStub, + }); + }); + + it('should call `screenCaptureModel.startScreencast` on `captureScreenshots` call', async () => { + const screenshotCapture = new SDK.AnimationModel.ScreenshotCapture(mockAnimationModel, mockScreenCaptureModel); + + await screenshotCapture.captureScreenshots(100, []); + await screenshotCapture.captureScreenshots(100, []); + + sinon.assert.calledOnce(startScreencastStub); + }); + }); }); diff --git a/front_end/core/sdk/AnimationModel.ts b/front_end/core/sdk/AnimationModel.ts index b013af36c8..71007195cb 100644 --- a/front_end/core/sdk/AnimationModel.ts +++ b/front_end/core/sdk/AnimationModel.ts @@ -415,7 +415,8 @@ export class AnimationModel extends SDKModel { if (!matchedGroup) { this.animationGroups.set(incomingGroup.id(), incomingGroup); if (this.#screenshotCapture) { - this.#screenshotCapture.captureScreenshots(incomingGroup.finiteDuration(), incomingGroup.screenshotsInternal); + void this.#screenshotCapture.captureScreenshots( + incomingGroup.finiteDuration(), incomingGroup.screenshotsInternal); } this.dispatchEventToListeners(Events.AnimationGroupStarted, incomingGroup); } else { @@ -1063,9 +1064,12 @@ export class ScreenshotCapture { #requests: Request[]; readonly #screenCaptureModel: ScreenCaptureModel; readonly #animationModel: AnimationModel; + // This prevents multiple synchronous calls to captureScreenshots to result in one startScreencast call in model. + #isCapturing: boolean = false; + // Holds the id for capturing & cancelling the screencast operation. + #screencastOperationId: number|undefined; #stopTimer?: number; #endTime?: number; - #capturing?: boolean; constructor(animationModel: AnimationModel, screenCaptureModel: ScreenCaptureModel) { this.#requests = []; this.#screenCaptureModel = screenCaptureModel; @@ -1073,7 +1077,7 @@ export class ScreenshotCapture { this.#animationModel.addEventListener(Events.ModelReset, this.stopScreencast, this); } - captureScreenshots(duration: number, screenshots: string[]): void { + async captureScreenshots(duration: number, screenshots: string[]): Promise { const screencastDuration = Math.min(duration / this.#animationModel.playbackRate, 3000); const endTime = screencastDuration + window.performance.now(); this.#requests.push({endTime, screenshots}); @@ -1084,11 +1088,12 @@ export class ScreenshotCapture { this.#endTime = endTime; } - if (this.#capturing) { + if (this.#isCapturing) { return; } - this.#capturing = true; - this.#screenCaptureModel.startScreencast( + + this.#isCapturing = true; + this.#screencastOperationId = await this.#screenCaptureModel.startScreencast( Protocol.Page.StartScreencastRequestFormat.Jpeg, 80, undefined, 300, 2, this.screencastFrame.bind(this), _visible => {}); } @@ -1098,7 +1103,7 @@ export class ScreenshotCapture { return request.endTime >= now; } - if (!this.#capturing) { + if (!this.#isCapturing) { return; } @@ -1110,15 +1115,16 @@ export class ScreenshotCapture { } private stopScreencast(): void { - if (!this.#capturing) { + if (!this.#screencastOperationId) { return; } + this.#screenCaptureModel.stopScreencast(this.#screencastOperationId); this.#stopTimer = undefined; this.#endTime = undefined; this.#requests = []; - this.#capturing = false; - this.#screenCaptureModel.stopScreencast(); + this.#isCapturing = false; + this.#screencastOperationId = undefined; } } diff --git a/front_end/core/sdk/BUILD.gn b/front_end/core/sdk/BUILD.gn index ebb8c8531f..167701c411 100644 --- a/front_end/core/sdk/BUILD.gn +++ b/front_end/core/sdk/BUILD.gn @@ -166,6 +166,7 @@ ts_library("unittests") { "RemoteObject.test.ts", "ResourceTreeModel.test.ts", "RuntimeModel.test.ts", + "ScreenCaptureModel.test.ts", "Script.test.ts", "ServerSentEventsProtocol.test.ts", "ServerTiming.test.ts", diff --git a/front_end/core/sdk/ScreenCaptureModel.test.ts b/front_end/core/sdk/ScreenCaptureModel.test.ts new file mode 100644 index 0000000000..2628a531f6 --- /dev/null +++ b/front_end/core/sdk/ScreenCaptureModel.test.ts @@ -0,0 +1,216 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import * as Protocol from '../../generated/protocol.js'; +import {createTarget} from '../../testing/EnvironmentHelpers.js'; +import { + clearAllMockConnectionResponseHandlers, + clearMockConnectionResponseHandler, + describeWithMockConnection, + dispatchEvent, + setMockConnectionResponseHandler, +} from '../../testing/MockConnection.js'; + +import * as SDK from './sdk.js'; + +const noop = () => {}; + +const QUALITY = 80; +const MAX_WIDTH = 10; +const MAX_HEIGHT = 20; +const EVERY_NTH_FRAME = 2; + +async function expectStartScreencastCalled(action: () => Promise| T): Promise<{ + cdpRequest: Protocol.Page.StartScreencastRequest, + actionResult: T, +}> { + clearMockConnectionResponseHandler('Page.startScreencast'); + + const startScreencastCalledPromise = Promise.withResolvers(); + setMockConnectionResponseHandler('Page.startScreencast', (request: Protocol.Page.StartScreencastRequest) => { + startScreencastCalledPromise.resolve(request); + return {}; + }); + + const response = await action(); + + return { + cdpRequest: await startScreencastCalledPromise.promise, + actionResult: response, + }; +} + +async function expectStopScreencastCaled(action: () => Promise| T): Promise<{ + actionResult: T, +}> { + clearMockConnectionResponseHandler('Page.stopScreencast'); + + const stopScreencastCalledPromise = Promise.withResolvers(); + setMockConnectionResponseHandler('Page.stopScreencast', () => { + stopScreencastCalledPromise.resolve(); + return {}; + }); + + const response = await action(); + + return {actionResult: response}; +} + +async function startMockScreencast(screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel, { + format = Protocol.Page.StartScreencastRequestFormat.Jpeg, + quality = QUALITY, + maxWidth = MAX_WIDTH, + maxHeight = MAX_HEIGHT, + everyNthFrame = EVERY_NTH_FRAME, + onFrame = noop, + onVisibilityChanged = noop, +}: { + format?: Protocol.Page.StartScreencastRequestFormat, + quality?: number, + maxWidth?: number, + maxHeight?: number, + everyNthFrame?: number, + onFrame?: () => void, + onVisibilityChanged?: () => void, +} = {}) { + const { + cdpRequest, + actionResult: id, + } = await expectStartScreencastCalled(() => { + return screenCaptureModel.startScreencast( + format, quality, maxWidth, maxHeight, everyNthFrame, onFrame, onVisibilityChanged); + }); + + return { + id, + cdpRequest, + }; +} + +async function stopMockScreencast(screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel, {id}: { + id: number, +}) { + await expectStopScreencastCaled(() => { + screenCaptureModel.stopScreencast(id); + }); +} + +describeWithMockConnection('ScreenCaptureModel', () => { + let target: SDK.Target.Target; + let screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel; + beforeEach(() => { + target = createTarget(); + const model = target.model(SDK.ScreenCaptureModel.ScreenCaptureModel); + assert.exists(model); + screenCaptureModel = model; + }); + + afterEach(() => { + clearAllMockConnectionResponseHandlers(); + }); + + describe('Screencasting', () => { + describe('only one screencast operation', () => { + it('startScreencast should start screen casting', async () => { + const {cdpRequest} = await startMockScreencast(screenCaptureModel, { + format: Protocol.Page.StartScreencastRequestFormat.Jpeg, + quality: 1, + maxWidth: 2, + maxHeight: 3, + everyNthFrame: 4, + }); + + assert.deepEqual(cdpRequest, { + format: Protocol.Page.StartScreencastRequestFormat.Jpeg, + quality: 1, + maxWidth: 2, + maxHeight: 3, + everyNthFrame: 4 + }); + }); + + it('stopScreencast should stop screen casting', async () => { + const {id} = await startMockScreencast(screenCaptureModel); + + await stopMockScreencast(screenCaptureModel, {id}); + }); + + it('stopScreencast throws an error for trying to stop screencast when there are no screencast operations in progress', + async () => { + try { + await stopMockScreencast(screenCaptureModel, {id: 42}); + assert.fail('Expected `stopScreencast` to throw'); + } catch (err) { + assert.strictEqual(err.message, 'There is no screencast operation to stop.'); + } + }); + + it('stopScreencast throws an error for trying to stop a different screencast than what is being in progress right now', + async () => { + await startMockScreencast(screenCaptureModel); + try { + await stopMockScreencast(screenCaptureModel, {id: 42}); + assert.fail('Expected `stopScreencast` to throw'); + } catch (err) { + assert.strictEqual( + err.message, 'Trying to stop a screencast operation that is not being served right now.'); + } + }); + }); + + describe('multiple screencast operations', () => { + beforeEach(() => { + setMockConnectionResponseHandler('Page.stopScreencast', () => ({})); + }); + + it('second call to startScreencast stops the ongoing screencasting', async () => { + await startMockScreencast(screenCaptureModel); + + // Stop screencast is called for the initial call before starting a new screencast. + await expectStopScreencastCaled(async () => { + await startMockScreencast(screenCaptureModel); + }); + }); + + it('only the last operation receives the callbacks', async () => { + const initialFrameCallback = sinon.stub(); + const initialVisibilityChangeCallback = sinon.stub(); + const lastFrameCallback = sinon.stub(); + const lastVisibilityChangeCallback = sinon.stub(); + + await startMockScreencast( + screenCaptureModel, {onFrame: initialFrameCallback, onVisibilityChanged: initialVisibilityChangeCallback}); + await startMockScreencast( + screenCaptureModel, {onFrame: lastFrameCallback, onVisibilityChanged: lastVisibilityChangeCallback}); + dispatchEvent(target, 'Page.screencastFrame', {}); + dispatchEvent(target, 'Page.screencastVisibilityChanged', {}); + + sinon.assert.notCalled(initialFrameCallback); + sinon.assert.notCalled(initialVisibilityChangeCallback); + sinon.assert.calledOnce(lastFrameCallback); + sinon.assert.calledOnce(lastVisibilityChangeCallback); + }); + + it('after the last operation is stopped, the previous one continues to receive callbacks', async () => { + const initialFrameCallback = sinon.stub(); + const initialVisibilityChangeCallback = sinon.stub(); + const lastFrameCallback = sinon.stub(); + const lastVisibilityChangeCallback = sinon.stub(); + + await startMockScreencast( + screenCaptureModel, {onFrame: initialFrameCallback, onVisibilityChanged: initialVisibilityChangeCallback}); + const {id} = await startMockScreencast( + screenCaptureModel, {onFrame: lastFrameCallback, onVisibilityChanged: lastVisibilityChangeCallback}); + await stopMockScreencast(screenCaptureModel, {id}); + dispatchEvent(target, 'Page.screencastFrame', {}); + dispatchEvent(target, 'Page.screencastVisibilityChanged', {}); + + sinon.assert.calledOnce(initialFrameCallback); + sinon.assert.calledOnce(initialVisibilityChangeCallback); + sinon.assert.notCalled(lastFrameCallback); + sinon.assert.notCalled(lastVisibilityChangeCallback); + }); + }); + }); +}); diff --git a/front_end/core/sdk/ScreenCaptureModel.ts b/front_end/core/sdk/ScreenCaptureModel.ts index 8bd385da9e..26321f316c 100644 --- a/front_end/core/sdk/ScreenCaptureModel.ts +++ b/front_end/core/sdk/ScreenCaptureModel.ts @@ -14,32 +14,108 @@ export const enum ScreenshotMode { FROM_CLIP = 'fromClip', FULLPAGE = 'fullpage', } + +// This structure holds a specific `startScreencast` request's parameters +// and its callbacks so that they can be re-started if needed. +interface ScreencastOperation { + id: number; + request: { + format: Protocol.Page.StartScreencastRequestFormat, + quality: number, + maxWidth: number|undefined, + maxHeight: number|undefined, + everyNthFrame: number|undefined, + }; + callbacks: { + onScreencastFrame: ScreencastFrameCallback, + onScreencastVisibilityChanged: ScreencastVisibilityChangedCallback, + }; +} + +type ScreencastFrameCallback = ((arg0: Protocol.binary, arg1: Protocol.Page.ScreencastFrameMetadata) => void); +type ScreencastVisibilityChangedCallback = ((arg0: boolean) => void); + +// Manages concurrent screencast requests by queuing and prioritizing. +// +// When startScreencast is invoked: +// - If a screencast is currently active, the existing screencast's parameters and callbacks are +// saved in the #screencastOperations array. +// - The active screencast is then stopped. +// - A new screencast is initiated using the parameters and callbacks from the current startScreencast call. +// +// When stopScreencast is invoked: +// - The currently active screencast is stopped. +// - The #screencastOperations is checked for interrupted screencast operations. +// - If any operations are found, the latest one is started +// using its saved parameters and callbacks. +// +// This ensures that: +// - Only one screencast is active at a time. +// - Interrupted screencasts are resumed after the current screencast is stopped. +// This ensures animation previews, which use screencasting, don't disrupt ongoing remote debugging sessions. Without this mechanism, stopping a preview screencast would terminate the debugging screencast, freezing the ScreencastView. export class ScreenCaptureModel extends SDKModel implements ProtocolProxyApi.PageDispatcher { readonly #agent: ProtocolProxyApi.PageApi; - #onScreencastFrame: ((arg0: Protocol.binary, arg1: Protocol.Page.ScreencastFrameMetadata) => void)|null; - #onScreencastVisibilityChanged: ((arg0: boolean) => void)|null; + #nextScreencastOperationId = 1; + #screencastOperations: ScreencastOperation[] = []; constructor(target: Target) { super(target); this.#agent = target.pageAgent(); - this.#onScreencastFrame = null; - this.#onScreencastVisibilityChanged = null; target.registerPageDispatcher(this); } - startScreencast( + async startScreencast( format: Protocol.Page.StartScreencastRequestFormat, quality: number, maxWidth: number|undefined, - maxHeight: number|undefined, everyNthFrame: number|undefined, - onFrame: (arg0: Protocol.binary, arg1: Protocol.Page.ScreencastFrameMetadata) => void, - onVisibilityChanged: (arg0: boolean) => void): void { - this.#onScreencastFrame = onFrame; - this.#onScreencastVisibilityChanged = onVisibilityChanged; + maxHeight: number|undefined, everyNthFrame: number|undefined, onFrame: ScreencastFrameCallback, + onVisibilityChanged: ScreencastVisibilityChangedCallback): Promise { + const currentRequest = this.#screencastOperations.at(-1); + if (currentRequest) { + // If there already is a screencast operation in progress, we need to stop it now and handle the + // incoming request. Once that request is stopped, we'll return back to handling the stopped operation. + await this.#agent.invoke_stopScreencast(); + } + + const operation = { + id: this.#nextScreencastOperationId++, + request: { + format, + quality, + maxWidth, + maxHeight, + everyNthFrame, + }, + callbacks: { + onScreencastFrame: onFrame, + onScreencastVisibilityChanged: onVisibilityChanged, + } + }; + this.#screencastOperations.push(operation); void this.#agent.invoke_startScreencast({format, quality, maxWidth, maxHeight, everyNthFrame}); + + return operation.id; } - stopScreencast(): void { - this.#onScreencastFrame = null; - this.#onScreencastVisibilityChanged = null; + stopScreencast(id: number): void { + const operationToStop = this.#screencastOperations.pop(); + if (!operationToStop) { + throw new Error('There is no screencast operation to stop.'); + } + + if (operationToStop.id !== id) { + throw new Error('Trying to stop a screencast operation that is not being served right now.'); + } void this.#agent.invoke_stopScreencast(); + + // The latest operation is concluded, let's return back to the previous request now, if it exists. + const nextOperation = this.#screencastOperations.at(-1); + if (nextOperation) { + void this.#agent.invoke_startScreencast({ + format: nextOperation.request.format, + quality: nextOperation.request.quality, + maxWidth: nextOperation.request.maxWidth, + maxHeight: nextOperation.request.maxHeight, + everyNthFrame: nextOperation.request.everyNthFrame, + }); + } } async captureScreenshot( @@ -73,14 +149,17 @@ export class ScreenCaptureModel extends SDKModel implements ProtocolProxyA screencastFrame({data, metadata, sessionId}: Protocol.Page.ScreencastFrameEvent): void { void this.#agent.invoke_screencastFrameAck({sessionId}); - if (this.#onScreencastFrame) { - this.#onScreencastFrame.call(null, data, metadata); + + const currentRequest = this.#screencastOperations.at(-1); + if (currentRequest) { + currentRequest.callbacks.onScreencastFrame.call(null, data, metadata); } } screencastVisibilityChanged({visible}: Protocol.Page.ScreencastVisibilityChangedEvent): void { - if (this.#onScreencastVisibilityChanged) { - this.#onScreencastVisibilityChanged.call(null, visible); + const currentRequest = this.#screencastOperations.at(-1); + if (currentRequest) { + currentRequest.callbacks.onScreencastVisibilityChanged.call(null, visible); } } diff --git a/front_end/panels/screencast/ScreencastView.ts b/front_end/panels/screencast/ScreencastView.ts index f7f23a4d67..09251447aa 100644 --- a/front_end/panels/screencast/ScreencastView.ts +++ b/front_end/panels/screencast/ScreencastView.ts @@ -116,7 +116,6 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H private navigationBack!: HTMLButtonElement; private navigationForward!: HTMLButtonElement; private canvasContainerElement?: HTMLElement; - private isCasting?: boolean; private checkerboardPattern?: CanvasPattern|null; private targetInactive?: boolean; private deferredCasting?: number; @@ -133,6 +132,8 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H private mouseInputToggleIcon?: IconButton.Icon.Icon; private historyIndex?: number; private historyEntries?: Protocol.Page.NavigationEntry[]; + private isCasting: boolean = false; + private screencastOperationId?: number; constructor(screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel) { super(); this.registerRequiredCSS(screencastViewStyles); @@ -188,7 +189,6 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H this.titleElement.style.left = '0'; this.imageElement = new Image(); - this.isCasting = false; this.context = this.canvasElement.getContext('2d') as CanvasRenderingContext2D; this.checkerboardPattern = this.createCheckerboardPattern(this.context); @@ -204,10 +204,11 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H this.stopCasting(); } - private startCasting(): void { + private async startCasting(): Promise { if (SDK.TargetManager.TargetManager.instance().allTargetsSuspended()) { return; } + if (this.isCasting) { return; } @@ -222,7 +223,7 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H dimensions.width *= window.devicePixelRatio; dimensions.height *= window.devicePixelRatio; // Note: startScreencast width and height are expected to be integers so must be floored. - this.screenCaptureModel.startScreencast( + this.screencastOperationId = await this.screenCaptureModel.startScreencast( Protocol.Page.StartScreencastRequestFormat.Jpeg, 80, Math.floor(Math.min(maxImageDimension, dimensions.width)), Math.floor(Math.min(maxImageDimension, dimensions.height)), undefined, this.screencastFrame.bind(this), this.screencastVisibilityChanged.bind(this)); @@ -232,11 +233,12 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H } private stopCasting(): void { - if (!this.isCasting) { + if (!this.screencastOperationId) { return; } + this.screenCaptureModel.stopScreencast(this.screencastOperationId); + this.screencastOperationId = undefined; this.isCasting = false; - this.screenCaptureModel.stopScreencast(); for (const emulationModel of SDK.TargetManager.TargetManager.instance().models(SDK.EmulationModel.EmulationModel)) { void emulationModel.overrideEmulateTouch(false); } @@ -286,7 +288,7 @@ export class ScreencastView extends UI.Widget.VBox implements SDK.OverlayModel.H if (SDK.TargetManager.TargetManager.instance().allTargetsSuspended()) { this.stopCasting(); } else { - this.startCasting(); + void this.startCasting(); } this.updateGlasspane(); }