Skip to content

Commit f7d9bba

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Fix trace event search issues
1. The search feature was doing nothing if the query was <3 characters. This produced confusing results when deleting your query, since the last result highlights still lingered. 2. Ending the search sometimes threw an error from accessing a trace event indexed by -1. This resulted in search highlights remaining until the next time the panel was redrawn. TypeScript didn't catch this, because it was a normal array access. Fixed this by making the type nullable, instead of using -1 to denote "no selection". Bug: 373914908 Change-Id: Ib292054a47b99c39dc36fdc459d8b44adb7ba044 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5939712 Auto-Submit: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Paul Irish <[email protected]>
1 parent 3fb5584 commit f7d9bba

File tree

3 files changed

+13
-11
lines changed

3 files changed

+13
-11
lines changed

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,9 +1391,9 @@ export class TimelineFlameChartView extends UI.Widget.VBox implements PerfUI.Fla
13911391
delete this.searchResults;
13921392
delete this.selectedSearchResult;
13931393
delete this.searchRegex;
1394-
this.mainFlameChart.showPopoverForSearchResult(-1);
1394+
this.mainFlameChart.showPopoverForSearchResult(null);
13951395
this.mainFlameChart.removeSearchResultHighlights();
1396-
this.networkFlameChart.showPopoverForSearchResult(-1);
1396+
this.networkFlameChart.showPopoverForSearchResult(null);
13971397
this.networkFlameChart.removeSearchResultHighlights();
13981398
}
13991399

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
535535

536536
this.searchableViewInternal = new UI.SearchableView.SearchableView(this.flameChart, null);
537537
this.searchableViewInternal.setMinimumSize(0, 100);
538+
this.searchableViewInternal.setMinimalSearchQuerySize(0);
538539
this.searchableViewInternal.element.classList.add('searchable-view');
539540
this.searchableViewInternal.show(this.timelinePane.element);
540541
this.flameChart.show(this.searchableViewInternal.element);

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ interface GroupHiddenState {
184184

185185
interface PopoverState {
186186
// Index of the last entry the popover was shown over.
187-
entryIndex: number;
187+
entryIndex: number|null;
188188
// Index of the last group the popover was shown over.
189189
groupIndex: number;
190190
hiddenEntriesPopover: boolean;
@@ -313,7 +313,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
313313

314314
#font: string;
315315
#groupTreeRoot?: GroupTreeNode|null;
316-
#searchResultEntryIndex: number;
316+
#searchResultEntryIndex: number|null = null;
317317
#searchResultHighlightElements: HTMLElement[] = [];
318318
#inTrackConfigEditMode: boolean = false;
319319
#linkSelectionAnnotationIsInProgress: boolean = false;
@@ -400,7 +400,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
400400
this.highlightedMarkerIndex = -1;
401401
this.highlightedEntryIndex = -1;
402402
this.selectedEntryIndex = -1;
403-
this.#searchResultEntryIndex = -1;
403+
this.#searchResultEntryIndex = null;
404404
this.rawTimelineDataLength = 0;
405405
this.markerPositions = new Map();
406406
this.customDrawnPositions = new Map();
@@ -515,7 +515,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
515515
}
516516

517517
hideHighlight(): void {
518-
if (this.#searchResultEntryIndex === -1) {
518+
if (this.#searchResultEntryIndex === null) {
519519
this.popoverElement.removeChildren();
520520
this.lastPopoverState = {
521521
entryIndex: -1,
@@ -660,7 +660,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
660660
* 3. Inside a track -> update the highlight of hovered event
661661
*/
662662
private onMouseMove(mouseEvent: MouseEvent): void {
663-
this.#searchResultEntryIndex = -1;
663+
this.#searchResultEntryIndex = null;
664664
this.lastMouseOffsetX = mouseEvent.offsetX;
665665
this.lastMouseOffsetY = mouseEvent.offsetY;
666666
if (!this.enabled()) {
@@ -781,14 +781,15 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
781781
this.hideHighlight();
782782
}
783783

784-
showPopoverForSearchResult(selectedSearchResult: number): void {
784+
showPopoverForSearchResult(selectedSearchResult: number|null): void {
785785
this.#searchResultEntryIndex = selectedSearchResult;
786786
this.#updatePopoverForEntry(selectedSearchResult);
787787
}
788788

789-
#updatePopoverForEntry(entryIndex: number): void {
789+
#updatePopoverForEntry(entryIndex: number|null): void {
790790
// Just update position if cursor is hovering the same entry.
791-
const isMouseOverRevealChildrenArrow = this.isMouseOverRevealChildrenArrow(this.lastMouseOffsetX, entryIndex);
791+
const isMouseOverRevealChildrenArrow =
792+
entryIndex !== null && this.isMouseOverRevealChildrenArrow(this.lastMouseOffsetX, entryIndex);
792793
if (entryIndex === this.lastPopoverState.entryIndex &&
793794
isMouseOverRevealChildrenArrow === this.lastPopoverState.hiddenEntriesPopover) {
794795
return this.updatePopoverOffset();
@@ -803,7 +804,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
803804
const entryInfo = (isMouseOverRevealChildrenArrow && group) ?
804805
this.dataProvider.prepareHighlightedHiddenEntriesArrowInfo &&
805806
this.dataProvider.prepareHighlightedHiddenEntriesArrowInfo(entryIndex) :
806-
this.dataProvider.prepareHighlightedEntryInfo(entryIndex);
807+
entryIndex !== null && this.dataProvider.prepareHighlightedEntryInfo(entryIndex);
807808
if (entryInfo) {
808809
this.popoverElement.appendChild(entryInfo);
809810
this.updatePopoverOffset();

0 commit comments

Comments
 (0)