Skip to content

Commit 61f924e

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: fix font family in screenshots
A fix we applied to the TimelineFlameChartView a long long time ago in the age of interaction tests to pick the right font was not correctly adapting to the Roboto font that gets set in test_setup.ts. Curiously this did not replicate on my Cloudtop, but did reliably on the bots, which caused these tests to be skipped. I think in this new world we can greatly simplify here and just always use the platform font, as we only run screenshots on Linux. If this works and these tests are stable now, I will follow up with a CL to remove `perf_ui/Font.ts` as we don't need that module now we just use the host font. Bug: 453626439, 453630637, 453629222 Change-Id: I6006cee53142e5bb8e0c9ae091490967c90dd360 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7079233 Reviewed-by: Peter Müller <[email protected]> Reviewed-by: Piotr Paulski <[email protected]> Commit-Queue: Jack Franklin <[email protected]> Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Philip Pfaffe <[email protected]>
1 parent fd282ef commit 61f924e

28 files changed

+30
-49
lines changed

front_end/panels/timeline/TimelineFlameChartView.test.ts

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -77,36 +77,33 @@ describeWithEnvironment('TimelineFlameChartView', function() {
7777
Common.Settings.Settings.instance().createSetting('timeline-flamechart-network-view-group-expansion', {}).set({});
7878
});
7979

80-
// Flaky
81-
it.skip(
82-
'[crbug.com/453626439] renders the network and other tracks in collapsed and expanded modes', async function() {
83-
const parsedTrace = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
84-
const mockViewDelegate = new MockViewDelegate();
85-
86-
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
87-
flameChartView.updateCountersGraphToggle(false); // don't care about the memory view in this test
88-
renderWidgetInVbox(flameChartView);
89-
// IMPORTANT: order is important; for the flame chart view to render properly
90-
// it must be in the DOM before we set the model, so it can calculate and
91-
// set heights.
92-
flameChartView.setModel(parsedTrace, new Map());
93-
94-
// Most of the network content is in the first ~700ms of this trace
95-
const {min} = parsedTrace.data.Meta.traceBounds;
96-
const interestingRange = Trace.Helpers.Timing.milliToMicro(Trace.Types.Timing.Milli(700));
97-
const max = Trace.Types.Timing.Micro(min + interestingRange);
98-
const newBounds = microsecondsTraceWindow(min, max);
99-
TraceBounds.TraceBounds.BoundsManager.instance().setTimelineVisibleWindow(newBounds);
100-
await raf();
101-
102-
await assertScreenshot('timeline/flamechart_view_network_collapsed.png');
103-
flameChartView.getNetworkFlameChart().toggleGroupExpand(0);
104-
await raf();
105-
await assertScreenshot('timeline/flamechart_view_network_expanded.png');
106-
});
80+
it('renders the network and other tracks in collapsed and expanded modes', async function() {
81+
const parsedTrace = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
82+
const mockViewDelegate = new MockViewDelegate();
83+
84+
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
85+
flameChartView.updateCountersGraphToggle(false); // don't care about the memory view in this test
86+
renderWidgetInVbox(flameChartView);
87+
// IMPORTANT: order is important; for the flame chart view to render properly
88+
// it must be in the DOM before we set the model, so it can calculate and
89+
// set heights.
90+
flameChartView.setModel(parsedTrace, new Map());
91+
92+
// Most of the network content is in the first ~700ms of this trace
93+
const {min} = parsedTrace.data.Meta.traceBounds;
94+
const interestingRange = Trace.Helpers.Timing.milliToMicro(Trace.Types.Timing.Milli(700));
95+
const max = Trace.Types.Timing.Micro(min + interestingRange);
96+
const newBounds = microsecondsTraceWindow(min, max);
97+
TraceBounds.TraceBounds.BoundsManager.instance().setTimelineVisibleWindow(newBounds);
98+
await raf();
99+
100+
await assertScreenshot('timeline/flamechart_view_network_collapsed.png');
101+
flameChartView.getNetworkFlameChart().toggleGroupExpand(0);
102+
await raf();
103+
await assertScreenshot('timeline/flamechart_view_network_expanded.png');
104+
});
107105

108-
// Flaky
109-
it.skip('[crbug.com/453630637] does not show the network track when there is no network request', async function() {
106+
it('does not show the network track when there is no network request', async function() {
110107
const parsedTrace = await TraceLoader.traceEngine(this, 'slow-interaction-keydown.json.gz');
111108
const mockViewDelegate = new MockViewDelegate();
112109
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
@@ -116,8 +113,7 @@ describeWithEnvironment('TimelineFlameChartView', function() {
116113
await assertScreenshot('timeline/flamechart_view_no_network_events.png');
117114
});
118115

119-
// Flaky
120-
it.skip('[crbug.com/453622839] shows the details for a selected network event', async function() {
116+
it('shows the details for a selected network event', async function() {
121117
const parsedTrace = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
122118
const mockViewDelegate = new MockViewDelegate();
123119

@@ -153,8 +149,7 @@ describeWithEnvironment('TimelineFlameChartView', function() {
153149
await assertScreenshot('timeline/timeline_with_network_selection.png');
154150
});
155151

156-
// Flaky
157-
it.skip('[crbug.com/453629222] shows the details for a selected main thread event', async function() {
152+
it('shows the details for a selected main thread event', async function() {
158153
const parsedTrace = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
159154
const mockViewDelegate = new MockViewDelegate();
160155

front_end/testing/TraceHelpers.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,14 +637,13 @@ export function renderWidgetInVbox(widget: UI.Widget.Widget, opts: {
637637
flexAuto?: boolean,
638638
} = {}): void {
639639
const target = document.createElement('div');
640-
target.innerHTML = `<style>${UI.inspectorCommonStyles}</style>`;
641640
target.classList.add('vbox');
642641
target.classList.toggle('flex-auto', Boolean(opts.flexAuto));
643642
target.style.width = (opts.width ?? 800) + 'px';
644643
target.style.height = (opts.height ?? 600) + 'px';
645644
widget.markAsRoot();
646645
widget.show(target);
647-
renderElementIntoDOM(target);
646+
renderElementIntoDOM(target, {includeCommonStyles: true});
648647
}
649648

650649
export function getMainThread(data: Trace.Handlers.ModelHandlers.Renderer.RendererHandlerData):

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
// found in the LICENSE file.
44
import * as Host from '../../../../core/host/host.js';
55

6-
let fontFamily: string|null = null;
7-
86
/**
97
* Because we run our UI in a couple of contexts (actual app & test
108
* environments) and on multiple platforms, the font is not consistent, so this
@@ -19,18 +17,7 @@ let fontFamily: string|null = null;
1917
* to ensure that the screenshot tests are consistent.
2018
**/
2119
export function getFontFamilyForCanvas(): string {
22-
if (fontFamily) {
23-
return fontFamily;
24-
}
25-
26-
const bodyStyles = getComputedStyle(document.body);
27-
if (bodyStyles.fontFamily) {
28-
fontFamily = bodyStyles.fontFamily;
29-
} else {
30-
fontFamily = Host.Platform.fontFamily();
31-
}
32-
33-
return fontFamily;
20+
return Host.Platform.fontFamily();
3421
}
3522

3623
export const DEFAULT_FONT_SIZE = '11px';
-153 Bytes
Loading
-456 Bytes
Loading
566 Bytes
Loading
-835 Bytes
Loading
-1.39 KB
Loading
-1.24 KB
Loading
1.45 KB
Loading

0 commit comments

Comments
 (0)