Skip to content

Commit 15a0b96

Browse files
jackfranklinDevtools-frontend LUCI CQ
jackfranklin
authored and
Devtools-frontend LUCI CQ
committed
Remove ThemeSupport from EntryStyles
It makes it easier to use this module elsewhere. I also took the opportunity to tidy up; `childColor` was not used anywhere in the code base, and `color` was always a CSS Variable, so I updated the code & types to reflect. Bug: 441265851 Change-Id: Id749ed7c44c11cd77a6b5b574ac5de704abae936 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6943173 Reviewed-by: Alex Rudenko <alexrudenko@chromium.org> Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
1 parent aeab712 commit 15a0b96

File tree

7 files changed

+71
-92
lines changed

7 files changed

+71
-92
lines changed

‎front_end/panels/timeline/ThreadAppender.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as SDK from '../../core/sdk/sdk.js';
99
import * as Bindings from '../../models/bindings/bindings.js';
1010
import * as Trace from '../../models/trace/trace.js';
1111
import * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
12+
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
1213

1314
import {
1415
addDecorationToEvent,
@@ -545,22 +546,25 @@ export class ThreadAppender implements TrackAppender {
545546

546547
if (Trace.Types.Events.isProfileCall(event)) {
547548
if (event.callFrame.functionName === '(idle)') {
548-
return Utils.EntryStyles.getCategoryStyles().idle.getComputedColorValue();
549+
return categoryColorValue(Utils.EntryStyles.getCategoryStyles().idle);
549550
}
550551
if (event.callFrame.functionName === '(program)') {
551-
return Utils.EntryStyles.getCategoryStyles().other.getComputedColorValue();
552+
return categoryColorValue(Utils.EntryStyles.getCategoryStyles().other);
552553
}
553554
if (event.callFrame.scriptId === '0') {
554555
// If we can not match this frame to a script, return the
555556
// generic "scripting" color.
556-
return Utils.EntryStyles.getCategoryStyles().scripting.getComputedColorValue();
557+
return categoryColorValue(Utils.EntryStyles.getCategoryStyles().scripting);
557558
}
558559
// Otherwise, return a color created based on its URL.
559560
return this.#colorGenerator.colorForID(event.callFrame.url);
560561
}
561-
const defaultColor =
562-
Utils.EntryStyles.getEventStyle(event.name as Trace.Types.Events.Name)?.category.getComputedColorValue();
563-
return defaultColor || Utils.EntryStyles.getCategoryStyles().other.getComputedColorValue();
562+
const eventStyles = Utils.EntryStyles.getEventStyle(event.name as Trace.Types.Events.Name);
563+
if (eventStyles) {
564+
return categoryColorValue(eventStyles.category);
565+
}
566+
567+
return categoryColorValue(Utils.EntryStyles.getCategoryStyles().other);
564568
}
565569

566570
/**
@@ -587,3 +591,7 @@ export class ThreadAppender implements TrackAppender {
587591
info.formattedTime = getDurationString(event.dur, selfTime);
588592
}
589593
}
594+
595+
function categoryColorValue(category: Utils.EntryStyles.TimelineCategory): string {
596+
return ThemeSupport.ThemeSupport.instance().getComputedValue(category.cssVariable);
597+
}

‎front_end/panels/timeline/TimelineEventOverview.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as Trace from '../../models/trace/trace.js';
99
import * as TraceBounds from '../../services/trace_bounds/trace_bounds.js';
1010
import * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
1111
import * as UI from '../../ui/legacy/legacy.js';
12+
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
1213
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
1314

1415
import * as Utils from './utils/utils.js';
@@ -242,7 +243,8 @@ export class TimelineEventOverviewCPUActivity extends TimelineEventOverview {
242243
quantizer.appendInterval(this.#start + timeRange + quantTime, idleIndex); // Kick drawing the last bucket.
243244
for (let i = categoryOrder.length - 1; i > 0; --i) {
244245
paths[i].lineTo(width, height);
245-
const computedColorValue = categories[categoryOrder[i]].getComputedColorValue();
246+
const computedColorValue =
247+
ThemeSupport.ThemeSupport.instance().getComputedValue(categories[categoryOrder[i]].cssVariable);
246248
context.fillStyle = computedColorValue;
247249
context.fill(paths[i]);
248250
context.strokeStyle = 'white';

‎front_end/panels/timeline/TimelineTreeView.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import * as Buttons from '../../ui/components/buttons/buttons.js';
1313
import * as DataGrid from '../../ui/legacy/components/data_grid/data_grid.js';
1414
import * as Components from '../../ui/legacy/components/utils/utils.js';
1515
import * as UI from '../../ui/legacy/legacy.js';
16+
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
1617
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
1718

1819
import {ActiveFilters} from './ActiveFilters.js';
@@ -989,7 +990,11 @@ export class AggregatedTimelineTreeView extends TimelineTreeView {
989990
case AggregatedTimelineTreeView.GroupBy.Category: {
990991
const idIsValid = id && Utils.EntryStyles.stringIsEventCategory(id);
991992
const category = idIsValid ? categories[id] || categories['other'] : {title: unattributed, color: unattributed};
992-
return {name: category.title, color: category.color, icon: undefined};
993+
994+
const color = category instanceof Utils.EntryStyles.TimelineCategory ?
995+
ThemeSupport.ThemeSupport.instance().getComputedValue(category.cssVariable) :
996+
category.color;
997+
return {name: category.title, color, icon: undefined};
993998
}
994999

9951000
case AggregatedTimelineTreeView.GroupBy.Domain:

‎front_end/panels/timeline/TimelineUIUtils.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,9 +475,9 @@ describeWithMockConnection('TimelineUIUtils', function() {
475475
ThemeSupport.ThemeSupport.clearThemeCache();
476476
});
477477

478-
it('should return the correct rgb value for a corresponding CSS variable', function() {
479-
const parsedColor = Utils.EntryStyles.getCategoryStyles().scripting.getComputedColorValue();
480-
assert.strictEqual('rgb(2 2 2)', parsedColor);
478+
it('should return the right color', function() {
479+
const parsedColor = Utils.EntryStyles.getCategoryStyles().scripting.cssVariable;
480+
assert.strictEqual(parsedColor, '--app-color-scripting');
481481
});
482482

483483
it('should return the color as a CSS variable', function() {
@@ -1515,7 +1515,7 @@ describeWithMockConnection('TimelineUIUtils', function() {
15151515
const profileCalls = thread.entries.filter(entry => Trace.Types.Events.isProfileCall(entry));
15161516
const style = Timeline.TimelineUIUtils.TimelineUIUtils.eventStyle(profileCalls[0]);
15171517
assert.strictEqual(style.category.name, 'scripting');
1518-
assert.strictEqual(style.category.color, 'rgb(250 204 21 / 100%)');
1518+
assert.strictEqual(style.category.cssVariable, '--app-color-scripting');
15191519
});
15201520
});
15211521

‎front_end/panels/timeline/TimelineUIUtils.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
5151
import imagePreviewStyles from '../../ui/legacy/components/utils/imagePreview.css.js';
5252
import * as LegacyComponents from '../../ui/legacy/components/utils/utils.js';
5353
import * as UI from '../../ui/legacy/legacy.js';
54+
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
5455

5556
import {getDurationString} from './AppenderUtils.js';
5657
import * as TimelineComponents from './components/components.js';
@@ -604,11 +605,12 @@ export class TimelineUIUtils {
604605
if (Trace.Types.Extensions.isSyntheticExtensionEntry(event)) {
605606
return Extensions.ExtensionUI.extensionEntryColor(event);
606607
}
607-
let parsedColor = TimelineUIUtils.eventStyle(event).category.getComputedColorValue();
608+
const themeSupport = ThemeSupport.ThemeSupport.instance();
609+
let parsedColor = themeSupport.getComputedValue(TimelineUIUtils.eventStyle(event).category.cssVariable);
608610
// This event is considered idle time but still rendered as a scripting event here
609611
// to connect the StreamingCompileScriptParsing events it belongs to.
610612
if (event.name === Trace.Types.Events.Name.STREAMING_COMPILE_SCRIPT_WAITING) {
611-
parsedColor = Utils.EntryStyles.getCategoryStyles().scripting.getComputedColorValue();
613+
parsedColor = themeSupport.getComputedValue(Utils.EntryStyles.getCategoryStyles().scripting.cssVariable);
612614
if (!parsedColor) {
613615
throw new Error('Unable to parse color from getCategoryStyles().scripting.color');
614616
}

‎front_end/panels/timeline/UIDevtoolsUtils.ts‎

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -134,42 +134,26 @@ export class UIDevtoolsUtils {
134134
}
135135
const {TimelineCategory, EventCategory} = Utils.EntryStyles;
136136
categories = {
137-
layout: new TimelineCategory(
138-
EventCategory.LAYOUT, i18nString(UIStrings.layout), true, '--app-color-loading-children',
139-
'--app-color-loading'),
137+
layout: new TimelineCategory(EventCategory.LAYOUT, i18nString(UIStrings.layout), true, '--app-color-loading'),
140138
rasterizing: new TimelineCategory(
141-
EventCategory.RASTERIZING, i18nString(UIStrings.rasterizing), true, '--app-color-children',
142-
'--app-color-scripting'),
143-
drawing: new TimelineCategory(
144-
EventCategory.DRAWING, i18nString(UIStrings.drawing), true, '--app-color-rendering-children',
145-
'--app-color-rendering'),
146-
painting: new TimelineCategory(
147-
EventCategory.PAINTING, i18nString(UIStrings.painting), true, '--app-color-painting-children',
148-
'--app-color-painting'),
149-
other: new TimelineCategory(
150-
EventCategory.OTHER, i18nString(UIStrings.system), false, '--app-color-system-children',
151-
'--app-color-system'),
152-
idle: new TimelineCategory(
153-
EventCategory.IDLE, i18nString(UIStrings.idle), false, '--app-color-idle-children', '--app-color-idle'),
154-
loading: new TimelineCategory(
155-
EventCategory.LOADING, i18nString(UIStrings.loading), false, '--app-color-loading-children',
156-
'--app-color-loading'),
139+
EventCategory.RASTERIZING, i18nString(UIStrings.rasterizing), true, '--app-color-scripting'),
140+
drawing:
141+
new TimelineCategory(EventCategory.DRAWING, i18nString(UIStrings.drawing), true, '--app-color-rendering'),
142+
painting:
143+
new TimelineCategory(EventCategory.PAINTING, i18nString(UIStrings.painting), true, '--app-color-painting'),
144+
other: new TimelineCategory(EventCategory.OTHER, i18nString(UIStrings.system), false, '--app-color-system'),
145+
idle: new TimelineCategory(EventCategory.IDLE, i18nString(UIStrings.idle), false, '--app-color-idle'),
146+
loading: new TimelineCategory(EventCategory.LOADING, i18nString(UIStrings.loading), false, '--app-color-loading'),
157147
experience: new TimelineCategory(
158-
EventCategory.EXPERIENCE, i18nString(UIStrings.experience), false, '--app-color-rendering-children',
159-
'--pp-color-rendering'),
148+
EventCategory.EXPERIENCE, i18nString(UIStrings.experience), false, '--app-color-rendering'),
160149
messaging: new TimelineCategory(
161-
EventCategory.MESSAGING, i18nString(UIStrings.messaging), false, '--app-color-messaging-children',
162-
'--pp-color-messaging'),
150+
EventCategory.MESSAGING, i18nString(UIStrings.messaging), false, '--app-color-messaging'),
163151
scripting: new TimelineCategory(
164-
EventCategory.SCRIPTING, i18nString(UIStrings.scripting), false, '--app-color-scripting-children',
165-
'--pp-color-scripting'),
152+
EventCategory.SCRIPTING, i18nString(UIStrings.scripting), false, '--app-color-scripting'),
166153
rendering: new TimelineCategory(
167-
EventCategory.RENDERING, i18nString(UIStrings.rendering), false, '--app-color-rendering-children',
168-
'--pp-color-rendering'),
169-
gpu: new TimelineCategory(
170-
EventCategory.GPU, i18nString(UIStrings.gpu), false, '--app-color-painting-children', '--app-color-painting'),
171-
async: new TimelineCategory(
172-
EventCategory.ASYNC, i18nString(UIStrings.async), false, '--app-color-async-children', '--app-color-async'),
154+
EventCategory.RENDERING, i18nString(UIStrings.rendering), false, '--app-color-rendering'),
155+
gpu: new TimelineCategory(EventCategory.GPU, i18nString(UIStrings.gpu), false, '--app-color-painting'),
156+
async: new TimelineCategory(EventCategory.ASYNC, i18nString(UIStrings.async), false, '--app-color-async'),
173157
};
174158
return categories;
175159
}

‎front_end/panels/timeline/utils/EntryStyles.ts‎

Lines changed: 25 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import * as i18n from '../../../core/i18n/i18n.js';
66
import type * as Platform from '../../../core/platform/platform.js';
77
import * as Trace from '../../../models/trace/trace.js';
8-
import * as ThemeSupport from '../../../ui/legacy/theme_support/theme_support.js';
98

109
const UIStrings = {
1110
/**
@@ -558,34 +557,29 @@ export class TimelineCategory {
558557
name: EventCategory;
559558
title: Platform.UIString.LocalizedString;
560559
visible: boolean;
561-
childColor: string;
562-
#color: string;
563560
#hidden?: boolean;
561+
#cssVariable: `--app-color-${string}`;
564562

565563
constructor(
566-
name: EventCategory, title: Platform.UIString.LocalizedString, visible: boolean, childColor: string,
567-
color: string) {
564+
name: EventCategory, title: Platform.UIString.LocalizedString, visible: boolean,
565+
cssVariable: `--app-color-${string}`) {
568566
this.name = name;
569567
this.title = title;
570568
this.visible = visible;
571-
this.childColor = childColor;
572-
this.#color = color;
569+
this.#cssVariable = cssVariable;
573570
this.hidden = false;
574571
}
575572

576573
get hidden(): boolean {
577574
return Boolean(this.#hidden);
578575
}
579576

580-
get color(): string {
581-
return this.getComputedColorValue();
582-
}
583-
getCSSValue(): string {
584-
return `var(${this.#color})`;
577+
get cssVariable(): string {
578+
return this.#cssVariable;
585579
}
586580

587-
getComputedColorValue(): string {
588-
return ThemeSupport.ThemeSupport.instance().getComputedValue(this.#color);
581+
getCSSValue(): string {
582+
return `var(${this.#cssVariable})`;
589583
}
590584

591585
set hidden(hidden: boolean) {
@@ -630,41 +624,25 @@ export function getCategoryStyles(): CategoryPalette {
630624
return categoryStyles;
631625
}
632626
categoryStyles = {
633-
loading: new TimelineCategory(
634-
EventCategory.LOADING, i18nString(UIStrings.loading), true, '--app-color-loading-children',
635-
'--app-color-loading'),
627+
loading: new TimelineCategory(EventCategory.LOADING, i18nString(UIStrings.loading), true, '--app-color-loading'),
636628
experience: new TimelineCategory(
637-
EventCategory.EXPERIENCE, i18nString(UIStrings.experience), false, '--app-color-rendering-children',
638-
'--app-color-rendering'),
639-
messaging: new TimelineCategory(
640-
EventCategory.MESSAGING, i18nString(UIStrings.messaging), true, '--app-color-messaging-children',
641-
'--app-color-messaging'),
642-
scripting: new TimelineCategory(
643-
EventCategory.SCRIPTING, i18nString(UIStrings.scripting), true, '--app-color-scripting-children',
644-
'--app-color-scripting'),
645-
rendering: new TimelineCategory(
646-
EventCategory.RENDERING, i18nString(UIStrings.rendering), true, '--app-color-rendering-children',
647-
'--app-color-rendering'),
648-
painting: new TimelineCategory(
649-
EventCategory.PAINTING, i18nString(UIStrings.painting), true, '--app-color-painting-children',
650-
'--app-color-painting'),
651-
gpu: new TimelineCategory(
652-
EventCategory.GPU, i18nString(UIStrings.gpu), false, '--app-color-painting-children', '--app-color-painting'),
653-
async: new TimelineCategory(
654-
EventCategory.ASYNC, i18nString(UIStrings.async), false, '--app-color-async-children', '--app-color-async'),
655-
other: new TimelineCategory(
656-
EventCategory.OTHER, i18nString(UIStrings.system), false, '--app-color-system-children', '--app-color-system'),
657-
idle: new TimelineCategory(
658-
EventCategory.IDLE, i18nString(UIStrings.idle), false, '--app-color-idle-children', '--app-color-idle'),
659-
layout: new TimelineCategory(
660-
EventCategory.LAYOUT, i18nString(UIStrings.layout), false, '--app-color-loading-children',
661-
'--app-color-loading'),
629+
EventCategory.EXPERIENCE, i18nString(UIStrings.experience), false, '--app-color-rendering'),
630+
messaging:
631+
new TimelineCategory(EventCategory.MESSAGING, i18nString(UIStrings.messaging), true, '--app-color-messaging'),
632+
scripting:
633+
new TimelineCategory(EventCategory.SCRIPTING, i18nString(UIStrings.scripting), true, '--app-color-scripting'),
634+
rendering:
635+
new TimelineCategory(EventCategory.RENDERING, i18nString(UIStrings.rendering), true, '--app-color-rendering'),
636+
painting:
637+
new TimelineCategory(EventCategory.PAINTING, i18nString(UIStrings.painting), true, '--app-color-painting'),
638+
gpu: new TimelineCategory(EventCategory.GPU, i18nString(UIStrings.gpu), false, '--app-color-painting'),
639+
async: new TimelineCategory(EventCategory.ASYNC, i18nString(UIStrings.async), false, '--app-color-async'),
640+
other: new TimelineCategory(EventCategory.OTHER, i18nString(UIStrings.system), false, '--app-color-system'),
641+
idle: new TimelineCategory(EventCategory.IDLE, i18nString(UIStrings.idle), false, '--app-color-idle'),
642+
layout: new TimelineCategory(EventCategory.LAYOUT, i18nString(UIStrings.layout), false, '--app-color-loading'),
662643
rasterizing: new TimelineCategory(
663-
EventCategory.RASTERIZING, i18nString(UIStrings.rasterizing), false, '--app-color-children',
664-
'--app-color-scripting'),
665-
drawing: new TimelineCategory(
666-
EventCategory.DRAWING, i18nString(UIStrings.drawing), false, '--app-color-rendering-children',
667-
'--app-color-rendering'),
644+
EventCategory.RASTERIZING, i18nString(UIStrings.rasterizing), false, '--app-color-scripting'),
645+
drawing: new TimelineCategory(EventCategory.DRAWING, i18nString(UIStrings.drawing), false, '--app-color-rendering'),
668646
};
669647
return categoryStyles;
670648
}

0 commit comments

Comments
 (0)