Skip to content

Commit eb03ef2

Browse files
Adam RaineDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Handle empty overlays array from insight
If `overlays` was empty, `overlaysBounds` would have bounds of `Infinity`. This causes several error messages in the console when expanding certain insights. Bug: None Change-Id: Ia9c9bfff1760c95b7b120efdd5519ce74307528b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6082494 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Adam Raine <[email protected]>
1 parent e2022a7 commit eb03ef2

File tree

4 files changed

+34
-15
lines changed

4 files changed

+34
-15
lines changed

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -529,17 +529,19 @@ export class TimelineFlameChartView extends
529529

530530
if (options.updateTraceWindow) {
531531
const overlaysBounds = Overlays.Overlays.traceWindowContainingOverlays(this.#currentInsightOverlays);
532-
// Trace window covering all overlays expanded by 100% so that the overlays cover 50% of the visible window.
533-
const expandedBounds =
534-
Trace.Helpers.Timing.expandWindowByPercentOrToOneMillisecond(overlaysBounds, traceBounds, 100);
535-
536-
// Set the timeline visible window and ignore the minimap bounds. This
537-
// allows us to pick a visible window even if the overlays are outside of
538-
// the current breadcrumb. If this happens, the event listener for
539-
// BoundsManager changes in TimelineMiniMap will detect it and activate
540-
// the correct breadcrumb for us.
541-
TraceBounds.TraceBounds.BoundsManager.instance().setTimelineVisibleWindow(
542-
expandedBounds, {ignoreMiniMapBounds: true, shouldAnimate: true});
532+
if (overlaysBounds) {
533+
// Trace window covering all overlays expanded by 100% so that the overlays cover 50% of the visible window.
534+
const expandedBounds =
535+
Trace.Helpers.Timing.expandWindowByPercentOrToOneMillisecond(overlaysBounds, traceBounds, 100);
536+
537+
// Set the timeline visible window and ignore the minimap bounds. This
538+
// allows us to pick a visible window even if the overlays are outside of
539+
// the current breadcrumb. If this happens, the event listener for
540+
// BoundsManager changes in TimelineMiniMap will detect it and activate
541+
// the correct breadcrumb for us.
542+
TraceBounds.TraceBounds.BoundsManager.instance().setTimelineVisibleWindow(
543+
expandedBounds, {ignoreMiniMapBounds: true, shouldAnimate: true});
544+
}
543545
}
544546

545547
// Reveal entry if we have one.

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,10 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
580580

581581
this.flameChart.overlays().addEventListener(Overlays.Overlays.TimeRangeMouseOverEvent.eventName, event => {
582582
const {overlay} = event as Overlays.Overlays.TimeRangeMouseOverEvent;
583-
const overlayBounds = overlay && Overlays.Overlays.traceWindowContainingOverlays([overlay]);
583+
const overlayBounds = Overlays.Overlays.traceWindowContainingOverlays([overlay]);
584+
if (!overlayBounds) {
585+
return;
586+
}
584587
this.#minimapComponent.highlightBounds(overlayBounds, /* withBracket */ false);
585588
});
586589

@@ -602,7 +605,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
602605

603606
this.flameChart.setOverlays(overlays, options);
604607

605-
const overlaysBounds = overlays && Overlays.Overlays.traceWindowContainingOverlays(overlays);
608+
const overlaysBounds = Overlays.Overlays.traceWindowContainingOverlays(overlays);
606609
if (overlaysBounds) {
607610
this.#minimapComponent.highlightBounds(overlaysBounds, /* withBracket */ true);
608611
} else {

front_end/panels/timeline/overlays/OverlaysImpl.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,9 +911,18 @@ describeWithEnvironment('Overlays', () => {
911911
outlineReason: 'INFO',
912912
};
913913
const traceWindow = Overlays.Overlays.traceWindowContainingOverlays([overlay1, overlay2]);
914+
if (!traceWindow) {
915+
throw new Error('No trace window for overlays');
916+
}
917+
914918
assert.strictEqual(traceWindow.min, 0);
915919
assert.strictEqual(traceWindow.max, 105);
916920
});
921+
922+
it('returns null for no overlays', () => {
923+
const traceWindow = Overlays.Overlays.traceWindowContainingOverlays([]);
924+
assert.isNull(traceWindow);
925+
});
917926
});
918927

919928
describe('jslogcontext for overlays', () => {

front_end/panels/timeline/overlays/OverlaysImpl.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,17 @@ export interface TimeRangeLabel {
8181
/**
8282
* Given a list of overlays, this method will calculate the smallest possible
8383
* trace window that will contain all of the overlays.
84-
* `overlays` is expected to be non-empty.
84+
* `overlays` is expected to be non-empty, and this will return `null` if it is empty.
8585
*/
86-
export function traceWindowContainingOverlays(overlays: TimelineOverlay[]): Trace.Types.Timing.TraceWindowMicroSeconds {
86+
export function traceWindowContainingOverlays(overlays: TimelineOverlay[]): Trace.Types.Timing.TraceWindowMicroSeconds|
87+
null {
8788
let minTime = Trace.Types.Timing.MicroSeconds(Number.POSITIVE_INFINITY);
8889
let maxTime = Trace.Types.Timing.MicroSeconds(Number.NEGATIVE_INFINITY);
8990

91+
if (overlays.length === 0) {
92+
return null;
93+
}
94+
9095
for (const overlay of overlays) {
9196
const windowForOverlay = traceWindowForOverlay(overlay);
9297
if (windowForOverlay.min < minTime) {

0 commit comments

Comments
 (0)