Skip to content

Commit 746bbd8

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[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 <[email protected]> Commit-Queue: Ergün Erdoğmuş <[email protected]>
1 parent f43c9c7 commit 746bbd8

File tree

6 files changed

+362
-34
lines changed

6 files changed

+362
-34
lines changed

front_end/core/sdk/AnimationModel.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,4 +254,28 @@ describeWithMockConnection('AnimationModel', () => {
254254
assert.strictEqual(animationImpl.delayOrStartTime(), 0); // in pixels
255255
});
256256
});
257+
258+
describe('ScreenshotCapture', () => {
259+
let mockAnimationModel: SDK.AnimationModel.AnimationModel;
260+
let mockScreenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel;
261+
let startScreencastStub:
262+
sinon.SinonStub<Parameters<typeof SDK.ScreenCaptureModel.ScreenCaptureModel.prototype.startScreencast>>;
263+
264+
beforeEach(() => {
265+
startScreencastStub = sinon.stub();
266+
mockAnimationModel = sinon.createStubInstance(SDK.AnimationModel.AnimationModel);
267+
mockScreenCaptureModel = sinon.createStubInstance(SDK.ScreenCaptureModel.ScreenCaptureModel, {
268+
startScreencast: startScreencastStub,
269+
});
270+
});
271+
272+
it('should call `screenCaptureModel.startScreencast` on `captureScreenshots` call', async () => {
273+
const screenshotCapture = new SDK.AnimationModel.ScreenshotCapture(mockAnimationModel, mockScreenCaptureModel);
274+
275+
await screenshotCapture.captureScreenshots(100, []);
276+
await screenshotCapture.captureScreenshots(100, []);
277+
278+
sinon.assert.calledOnce(startScreencastStub);
279+
});
280+
});
257281
});

front_end/core/sdk/AnimationModel.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,8 @@ export class AnimationModel extends SDKModel<EventTypes> {
415415
if (!matchedGroup) {
416416
this.animationGroups.set(incomingGroup.id(), incomingGroup);
417417
if (this.#screenshotCapture) {
418-
this.#screenshotCapture.captureScreenshots(incomingGroup.finiteDuration(), incomingGroup.screenshotsInternal);
418+
void this.#screenshotCapture.captureScreenshots(
419+
incomingGroup.finiteDuration(), incomingGroup.screenshotsInternal);
419420
}
420421
this.dispatchEventToListeners(Events.AnimationGroupStarted, incomingGroup);
421422
} else {
@@ -1063,17 +1064,20 @@ export class ScreenshotCapture {
10631064
#requests: Request[];
10641065
readonly #screenCaptureModel: ScreenCaptureModel;
10651066
readonly #animationModel: AnimationModel;
1067+
// This prevents multiple synchronous calls to captureScreenshots to result in one startScreencast call in model.
1068+
#isCapturing: boolean = false;
1069+
// Holds the id for capturing & cancelling the screencast operation.
1070+
#screencastOperationId: number|undefined;
10661071
#stopTimer?: number;
10671072
#endTime?: number;
1068-
#capturing?: boolean;
10691073
constructor(animationModel: AnimationModel, screenCaptureModel: ScreenCaptureModel) {
10701074
this.#requests = [];
10711075
this.#screenCaptureModel = screenCaptureModel;
10721076
this.#animationModel = animationModel;
10731077
this.#animationModel.addEventListener(Events.ModelReset, this.stopScreencast, this);
10741078
}
10751079

1076-
captureScreenshots(duration: number, screenshots: string[]): void {
1080+
async captureScreenshots(duration: number, screenshots: string[]): Promise<void> {
10771081
const screencastDuration = Math.min(duration / this.#animationModel.playbackRate, 3000);
10781082
const endTime = screencastDuration + window.performance.now();
10791083
this.#requests.push({endTime, screenshots});
@@ -1084,11 +1088,12 @@ export class ScreenshotCapture {
10841088
this.#endTime = endTime;
10851089
}
10861090

1087-
if (this.#capturing) {
1091+
if (this.#isCapturing) {
10881092
return;
10891093
}
1090-
this.#capturing = true;
1091-
this.#screenCaptureModel.startScreencast(
1094+
1095+
this.#isCapturing = true;
1096+
this.#screencastOperationId = await this.#screenCaptureModel.startScreencast(
10921097
Protocol.Page.StartScreencastRequestFormat.Jpeg, 80, undefined, 300, 2, this.screencastFrame.bind(this),
10931098
_visible => {});
10941099
}
@@ -1098,7 +1103,7 @@ export class ScreenshotCapture {
10981103
return request.endTime >= now;
10991104
}
11001105

1101-
if (!this.#capturing) {
1106+
if (!this.#isCapturing) {
11021107
return;
11031108
}
11041109

@@ -1110,15 +1115,16 @@ export class ScreenshotCapture {
11101115
}
11111116

11121117
private stopScreencast(): void {
1113-
if (!this.#capturing) {
1118+
if (!this.#screencastOperationId) {
11141119
return;
11151120
}
11161121

1122+
this.#screenCaptureModel.stopScreencast(this.#screencastOperationId);
11171123
this.#stopTimer = undefined;
11181124
this.#endTime = undefined;
11191125
this.#requests = [];
1120-
this.#capturing = false;
1121-
this.#screenCaptureModel.stopScreencast();
1126+
this.#isCapturing = false;
1127+
this.#screencastOperationId = undefined;
11221128
}
11231129
}
11241130

front_end/core/sdk/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ ts_library("unittests") {
166166
"RemoteObject.test.ts",
167167
"ResourceTreeModel.test.ts",
168168
"RuntimeModel.test.ts",
169+
"ScreenCaptureModel.test.ts",
169170
"Script.test.ts",
170171
"ServerSentEventsProtocol.test.ts",
171172
"ServerTiming.test.ts",
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
// Copyright 2020 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import * as Protocol from '../../generated/protocol.js';
6+
import {createTarget} from '../../testing/EnvironmentHelpers.js';
7+
import {
8+
clearAllMockConnectionResponseHandlers,
9+
clearMockConnectionResponseHandler,
10+
describeWithMockConnection,
11+
dispatchEvent,
12+
setMockConnectionResponseHandler,
13+
} from '../../testing/MockConnection.js';
14+
15+
import * as SDK from './sdk.js';
16+
17+
const noop = () => {};
18+
19+
const QUALITY = 80;
20+
const MAX_WIDTH = 10;
21+
const MAX_HEIGHT = 20;
22+
const EVERY_NTH_FRAME = 2;
23+
24+
async function expectStartScreencastCalled<T>(action: () => Promise<T>| T): Promise<{
25+
cdpRequest: Protocol.Page.StartScreencastRequest,
26+
actionResult: T,
27+
}> {
28+
clearMockConnectionResponseHandler('Page.startScreencast');
29+
30+
const startScreencastCalledPromise = Promise.withResolvers<Protocol.Page.StartScreencastRequest>();
31+
setMockConnectionResponseHandler('Page.startScreencast', (request: Protocol.Page.StartScreencastRequest) => {
32+
startScreencastCalledPromise.resolve(request);
33+
return {};
34+
});
35+
36+
const response = await action();
37+
38+
return {
39+
cdpRequest: await startScreencastCalledPromise.promise,
40+
actionResult: response,
41+
};
42+
}
43+
44+
async function expectStopScreencastCaled<T>(action: () => Promise<T>| T): Promise<{
45+
actionResult: T,
46+
}> {
47+
clearMockConnectionResponseHandler('Page.stopScreencast');
48+
49+
const stopScreencastCalledPromise = Promise.withResolvers<void>();
50+
setMockConnectionResponseHandler('Page.stopScreencast', () => {
51+
stopScreencastCalledPromise.resolve();
52+
return {};
53+
});
54+
55+
const response = await action();
56+
57+
return {actionResult: response};
58+
}
59+
60+
async function startMockScreencast(screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel, {
61+
format = Protocol.Page.StartScreencastRequestFormat.Jpeg,
62+
quality = QUALITY,
63+
maxWidth = MAX_WIDTH,
64+
maxHeight = MAX_HEIGHT,
65+
everyNthFrame = EVERY_NTH_FRAME,
66+
onFrame = noop,
67+
onVisibilityChanged = noop,
68+
}: {
69+
format?: Protocol.Page.StartScreencastRequestFormat,
70+
quality?: number,
71+
maxWidth?: number,
72+
maxHeight?: number,
73+
everyNthFrame?: number,
74+
onFrame?: () => void,
75+
onVisibilityChanged?: () => void,
76+
} = {}) {
77+
const {
78+
cdpRequest,
79+
actionResult: id,
80+
} = await expectStartScreencastCalled(() => {
81+
return screenCaptureModel.startScreencast(
82+
format, quality, maxWidth, maxHeight, everyNthFrame, onFrame, onVisibilityChanged);
83+
});
84+
85+
return {
86+
id,
87+
cdpRequest,
88+
};
89+
}
90+
91+
async function stopMockScreencast(screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel, {id}: {
92+
id: number,
93+
}) {
94+
await expectStopScreencastCaled(() => {
95+
screenCaptureModel.stopScreencast(id);
96+
});
97+
}
98+
99+
describeWithMockConnection('ScreenCaptureModel', () => {
100+
let target: SDK.Target.Target;
101+
let screenCaptureModel: SDK.ScreenCaptureModel.ScreenCaptureModel;
102+
beforeEach(() => {
103+
target = createTarget();
104+
const model = target.model(SDK.ScreenCaptureModel.ScreenCaptureModel);
105+
assert.exists(model);
106+
screenCaptureModel = model;
107+
});
108+
109+
afterEach(() => {
110+
clearAllMockConnectionResponseHandlers();
111+
});
112+
113+
describe('Screencasting', () => {
114+
describe('only one screencast operation', () => {
115+
it('startScreencast should start screen casting', async () => {
116+
const {cdpRequest} = await startMockScreencast(screenCaptureModel, {
117+
format: Protocol.Page.StartScreencastRequestFormat.Jpeg,
118+
quality: 1,
119+
maxWidth: 2,
120+
maxHeight: 3,
121+
everyNthFrame: 4,
122+
});
123+
124+
assert.deepEqual(cdpRequest, {
125+
format: Protocol.Page.StartScreencastRequestFormat.Jpeg,
126+
quality: 1,
127+
maxWidth: 2,
128+
maxHeight: 3,
129+
everyNthFrame: 4
130+
});
131+
});
132+
133+
it('stopScreencast should stop screen casting', async () => {
134+
const {id} = await startMockScreencast(screenCaptureModel);
135+
136+
await stopMockScreencast(screenCaptureModel, {id});
137+
});
138+
139+
it('stopScreencast throws an error for trying to stop screencast when there are no screencast operations in progress',
140+
async () => {
141+
try {
142+
await stopMockScreencast(screenCaptureModel, {id: 42});
143+
assert.fail('Expected `stopScreencast` to throw');
144+
} catch (err) {
145+
assert.strictEqual(err.message, 'There is no screencast operation to stop.');
146+
}
147+
});
148+
149+
it('stopScreencast throws an error for trying to stop a different screencast than what is being in progress right now',
150+
async () => {
151+
await startMockScreencast(screenCaptureModel);
152+
try {
153+
await stopMockScreencast(screenCaptureModel, {id: 42});
154+
assert.fail('Expected `stopScreencast` to throw');
155+
} catch (err) {
156+
assert.strictEqual(
157+
err.message, 'Trying to stop a screencast operation that is not being served right now.');
158+
}
159+
});
160+
});
161+
162+
describe('multiple screencast operations', () => {
163+
beforeEach(() => {
164+
setMockConnectionResponseHandler('Page.stopScreencast', () => ({}));
165+
});
166+
167+
it('second call to startScreencast stops the ongoing screencasting', async () => {
168+
await startMockScreencast(screenCaptureModel);
169+
170+
// Stop screencast is called for the initial call before starting a new screencast.
171+
await expectStopScreencastCaled(async () => {
172+
await startMockScreencast(screenCaptureModel);
173+
});
174+
});
175+
176+
it('only the last operation receives the callbacks', async () => {
177+
const initialFrameCallback = sinon.stub();
178+
const initialVisibilityChangeCallback = sinon.stub();
179+
const lastFrameCallback = sinon.stub();
180+
const lastVisibilityChangeCallback = sinon.stub();
181+
182+
await startMockScreencast(
183+
screenCaptureModel, {onFrame: initialFrameCallback, onVisibilityChanged: initialVisibilityChangeCallback});
184+
await startMockScreencast(
185+
screenCaptureModel, {onFrame: lastFrameCallback, onVisibilityChanged: lastVisibilityChangeCallback});
186+
dispatchEvent(target, 'Page.screencastFrame', {});
187+
dispatchEvent(target, 'Page.screencastVisibilityChanged', {});
188+
189+
sinon.assert.notCalled(initialFrameCallback);
190+
sinon.assert.notCalled(initialVisibilityChangeCallback);
191+
sinon.assert.calledOnce(lastFrameCallback);
192+
sinon.assert.calledOnce(lastVisibilityChangeCallback);
193+
});
194+
195+
it('after the last operation is stopped, the previous one continues to receive callbacks', async () => {
196+
const initialFrameCallback = sinon.stub();
197+
const initialVisibilityChangeCallback = sinon.stub();
198+
const lastFrameCallback = sinon.stub();
199+
const lastVisibilityChangeCallback = sinon.stub();
200+
201+
await startMockScreencast(
202+
screenCaptureModel, {onFrame: initialFrameCallback, onVisibilityChanged: initialVisibilityChangeCallback});
203+
const {id} = await startMockScreencast(
204+
screenCaptureModel, {onFrame: lastFrameCallback, onVisibilityChanged: lastVisibilityChangeCallback});
205+
await stopMockScreencast(screenCaptureModel, {id});
206+
dispatchEvent(target, 'Page.screencastFrame', {});
207+
dispatchEvent(target, 'Page.screencastVisibilityChanged', {});
208+
209+
sinon.assert.calledOnce(initialFrameCallback);
210+
sinon.assert.calledOnce(initialVisibilityChangeCallback);
211+
sinon.assert.notCalled(lastFrameCallback);
212+
sinon.assert.notCalled(lastVisibilityChangeCallback);
213+
});
214+
});
215+
});
216+
});

0 commit comments

Comments
 (0)