Skip to content

Commit f567b83

Browse files
danilsomsikovDevtools-frontend LUCI CQ
authored and
Devtools-frontend LUCI CQ
committed
Use scoped TargetManager API in media panel
Bug: 1417112 Change-Id: Iddb1e98627d784db1bc162f9a6090f0ca593e6d7 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4345505 Reviewed-by: Wolfgang Beyer <wolfi@chromium.org> Auto-Submit: Danil Somsikov <dsv@chromium.org> Commit-Queue: Danil Somsikov <dsv@chromium.org>
1 parent 6fdf052 commit f567b83

File tree

5 files changed

+88
-21
lines changed

5 files changed

+88
-21
lines changed

front_end/panels/media/MainView.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class PlayerDataCollection implements TriggerHandler {
6464
}
6565
}
6666

67-
class PlayerDataDownloadManager implements TriggerDispatcher {
67+
export class PlayerDataDownloadManager implements TriggerDispatcher {
6868
private readonly playerDataCollection: Map<string, PlayerDataCollection>;
6969
constructor() {
7070
this.playerDataCollection = new Map();
@@ -136,24 +136,23 @@ export class MainView extends UI.Panel.PanelWithSidebar implements SDK.TargetMan
136136
private readonly downloadStore: PlayerDataDownloadManager;
137137
private readonly sidebar: PlayerListView;
138138

139-
constructor() {
139+
constructor(downloadStore: PlayerDataDownloadManager) {
140140
super('Media');
141141
this.detailPanels = new Map();
142142

143143
this.deletedPlayers = new Set();
144144

145-
this.downloadStore = new PlayerDataDownloadManager();
145+
this.downloadStore = downloadStore;
146146

147147
this.sidebar = new PlayerListView(this);
148148
this.sidebar.show(this.panelSidebarElement());
149149

150-
SDK.TargetManager.TargetManager.instance().observeModels(MediaModel, this);
150+
SDK.TargetManager.TargetManager.instance().observeModels(MediaModel, this, {scoped: true});
151151
}
152152

153-
static instance(opts = {forceNew: null}): MainView {
154-
const {forceNew} = opts;
155-
if (!mainViewInstance || forceNew) {
156-
mainViewInstance = new MainView();
153+
static instance(opts?: {forceNew: boolean, downloadStore?: PlayerDataDownloadManager}): MainView {
154+
if (!mainViewInstance || opts?.forceNew) {
155+
mainViewInstance = new MainView(opts?.downloadStore || new PlayerDataDownloadManager());
157156
}
158157

159158
return mainViewInstance;
@@ -172,13 +171,13 @@ export class MainView extends UI.Panel.PanelWithSidebar implements SDK.TargetMan
172171

173172
wasShown(): void {
174173
super.wasShown();
175-
for (const model of SDK.TargetManager.TargetManager.instance().models(MediaModel)) {
174+
for (const model of SDK.TargetManager.TargetManager.instance().models(MediaModel, {scoped: true})) {
176175
this.addEventListeners(model);
177176
}
178177
}
179178

180179
willHide(): void {
181-
for (const model of SDK.TargetManager.TargetManager.instance().models(MediaModel)) {
180+
for (const model of SDK.TargetManager.TargetManager.instance().models(MediaModel, {scoped: true})) {
182181
this.removeEventListeners(model);
183182
}
184183
}

front_end/panels/media/TickingFlameChart.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ function getGroupDefaultTextColor(): string {
1616
return ThemeSupport.ThemeSupport.instance().getComputedValue('--color-text-primary');
1717
}
1818

19-
const DefaultStyle = {
19+
const DefaultStyle: () => PerfUI.FlameChart.GroupStyle = () => ({
2020
height: 20,
2121
padding: 2,
2222
collapsible: false,
@@ -28,7 +28,7 @@ const DefaultStyle = {
2828
shareHeaderLine: false,
2929
useFirstLineForOverview: false,
3030
useDecoratorsForOverview: false,
31-
};
31+
});
3232

3333
export const HotColorScheme = ['#ffba08', '#faa307', '#f48c06', '#e85d04', '#dc2f02', '#d00000', '#9d0208'];
3434
export const ColdColorScheme = ['#7400b8', '#6930c3', '#5e60ce', '#5390d9', '#4ea8de', '#48bfe3', '#56cfe1', '#64dfdf'];
@@ -397,7 +397,7 @@ class TickingFlameChartDataProvider implements PerfUI.FlameChart.FlameChartDataP
397397
startLevel: this.maxLevel,
398398
expanded: true,
399399
selectable: false,
400-
style: DefaultStyle,
400+
style: DefaultStyle(),
401401
track: null,
402402
};
403403
this.timelineDataInternal.groups.push(newGroup);

front_end/ui/legacy/components/perf_ui/ChartViewport.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55
import * as Common from '../../../../core/common/common.js';
66
import * as i18n from '../../../../core/i18n/i18n.js';
77
import * as Platform from '../../../../core/platform/platform.js';
8+
import * as Coordinator from '../../../components/render_coordinator/render_coordinator.js';
89
import * as UI from '../../legacy.js';
910

1011
import chartViewPortStyles from './chartViewport.css.legacy.js';
1112
import {MinimalTimeWindowMs} from './FlameChart.js';
1213

14+
const coordinator = Coordinator.RenderCoordinator.RenderCoordinator.instance();
15+
1316
export interface ChartViewportDelegate {
1417
windowChanged(startTime: number, endTime: number, animate: boolean): void;
1518
updateRangeSelection(startTime: number, endTime: number): void;
@@ -47,7 +50,6 @@ export class ChartViewport extends UI.Widget.VBox {
4750
private lastMouseOffsetX!: number;
4851
private minimumBoundary!: number;
4952
private totalTime!: number;
50-
private updateTimerId?: number;
5153
private cancelWindowTimesAnimation?: (() => void)|null;
5254

5355
constructor(delegate: ChartViewportDelegate) {
@@ -412,13 +414,10 @@ export class ChartViewport extends UI.Widget.VBox {
412414
}
413415

414416
scheduleUpdate(): void {
415-
if (this.updateTimerId || this.cancelWindowTimesAnimation) {
417+
if (this.cancelWindowTimesAnimation) {
416418
return;
417419
}
418-
this.updateTimerId = this.element.window().requestAnimationFrame(() => {
419-
this.updateTimerId = 0;
420-
this.update();
421-
});
420+
void coordinator.write(() => this.update());
422421
}
423422

424423
private update(): void {

test/unittests/front_end/panels/media/BUILD.gn

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,14 @@ import("../../../../../third_party/typescript/typescript.gni")
66

77
ts_library("media") {
88
testonly = true
9-
sources = [ "TickingFlameChartHelpers_test.ts" ]
9+
sources = [
10+
"MainView_test.ts",
11+
"TickingFlameChartHelpers_test.ts",
12+
]
1013

11-
deps = [ "../../../../../front_end/panels/media:bundle" ]
14+
deps = [
15+
"../../../../../front_end/core/sdk:bundle",
16+
"../../../../../front_end/panels/media:bundle",
17+
"../../helpers",
18+
]
1219
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2023 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 type * as Protocol from '../../../../../front_end/generated/protocol.js';
6+
import {assertNotNullOrUndefined} from '../../../../../front_end/core/platform/platform.js';
7+
import * as SDK from '../../../../../front_end/core/sdk/sdk.js';
8+
import * as Media from '../../../../../front_end/panels/media/media.js';
9+
import {createTarget} from '../../helpers/EnvironmentHelpers.js';
10+
import {describeWithMockConnection} from '../../helpers/MockConnection.js';
11+
import * as Coordinator from '../../../../../front_end/ui/components/render_coordinator/render_coordinator.js';
12+
13+
const {assert} = chai;
14+
15+
const PLAYER_ID = 'PLAYER_ID' as Protocol.Media.PlayerId;
16+
17+
describeWithMockConnection('MediaMainView', () => {
18+
let target: SDK.Target.Target;
19+
20+
beforeEach(() => {
21+
target = createTarget();
22+
// window.onerror = msg => console.error('onerror: ' + msg + (new Error()).stack);
23+
});
24+
25+
const testUiUpdate =
26+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
27+
(event: any, expectedMethod: keyof Media.MainView.PlayerDataDownloadManager, inScope: boolean) => async () => {
28+
if (inScope) {
29+
SDK.TargetManager.TargetManager.instance().setScopeTarget(target);
30+
}
31+
const downloadStore = new Media.MainView.PlayerDataDownloadManager();
32+
const expectedCall = sinon.stub(downloadStore, expectedMethod).returns();
33+
const mainView = Media.MainView.MainView.instance({forceNew: true, downloadStore});
34+
mainView.markAsRoot();
35+
mainView.show(document.body);
36+
const model = target.model(Media.MediaModel.MediaModel);
37+
assertNotNullOrUndefined(model);
38+
model.dispatchEventToListeners(Media.MediaModel.Events.PlayersCreated, [PLAYER_ID]);
39+
const field = [{name: 'kResolution', value: '{}', data: {}, stack: [], cause: []}];
40+
const data = {playerId: PLAYER_ID, properties: field, events: field, messages: field, errors: field};
41+
model.dispatchEventToListeners(event, data);
42+
await new Promise(resolve => setTimeout(resolve, 0));
43+
assert.strictEqual(expectedCall.called, inScope);
44+
await Coordinator.RenderCoordinator.RenderCoordinator.instance().done();
45+
mainView.detach();
46+
};
47+
48+
it('reacts to properties on in scope event',
49+
testUiUpdate(Media.MediaModel.Events.PlayerPropertiesChanged, 'onProperty', true));
50+
it('does not react to properties on out of scope event',
51+
testUiUpdate(Media.MediaModel.Events.PlayerPropertiesChanged, 'onProperty', false));
52+
it('reacts to event on in scope event', testUiUpdate(Media.MediaModel.Events.PlayerEventsAdded, 'onEvent', true));
53+
it('does not react to event on out of scope event',
54+
testUiUpdate(Media.MediaModel.Events.PlayerEventsAdded, 'onEvent', false));
55+
it('reacts to messages on in scope event',
56+
testUiUpdate(Media.MediaModel.Events.PlayerMessagesLogged, 'onMessage', true));
57+
it('does not react to messages on out of scope event',
58+
testUiUpdate(Media.MediaModel.Events.PlayerMessagesLogged, 'onMessage', false));
59+
it('reacts to error on in scope event', testUiUpdate(Media.MediaModel.Events.PlayerErrorsRaised, 'onError', true));
60+
it('does not react to error on out of scope event',
61+
testUiUpdate(Media.MediaModel.Events.PlayerErrorsRaised, 'onError', false));
62+
});

0 commit comments

Comments
 (0)