[Animations] Fix animations freezing screencast view

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 <dsv@chromium.org>
Commit-Queue: Ergün Erdoğmuş <ergunsh@chromium.org>

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<Parameters<typeof SDK.ScreenCaptureModel.ScreenCaptureModel.prototype.startScreencast>>;
+
+    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);
+    });
+  });
 });

front_end/core/sdk/AnimationModel.ts

@@ -415,7 +415,8 @@ export class AnimationModel extends SDKModel<EventTypes> {
     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<void> {
     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;
   }
 }
 

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",

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<T>(action: () => Promise<T>| T): Promise<{
+  cdpRequest: Protocol.Page.StartScreencastRequest,
+  actionResult: T,
+}> {
+  clearMockConnectionResponseHandler('Page.startScreencast');
+
+  const startScreencastCalledPromise = Promise.withResolvers<Protocol.Page.StartScreencastRequest>();
+  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<T>(action: () => Promise<T>| T): Promise<{
+  actionResult: T,
+}> {
+  clearMockConnectionResponseHandler('Page.stopScreencast');
+
+  const stopScreencastCalledPromise = Promise.withResolvers<void>();
+  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);
+      });
+    });
+  });
+});

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<void> 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<number> {
+    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<void> 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);
     }
   }
 

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<void> {
     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();
   }