[RPP] Remove all markers from minimap, but add nav start

These markers are really noisy, so we decided to remove them. But adding
NAV since it seems useful to see how the trace is broken up into
navigations at-a-glance.

The minimap component attempted to render the NAV markers, but since
they were not marked as "tall" it never rendered.

Also align the color used for the navigationStarted event type to be the
same as the marker color (black, not orange). Similar for the other
page load metrics.

Bug: 383368162
Change-Id: I75eed42d8ab33e27d8205dc560bf7d95371a652c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6084879
Commit-Queue: Connor Clark <[email protected]>
Reviewed-by: Adriana Ixba <[email protected]>
Reviewed-by: Paul Irish <[email protected]>

Author: Connor Clark

front_end/panels/timeline/TimelineMiniMap.ts

@@ -242,9 +242,9 @@ export class TimelineMiniMap extends
   #setMarkers(parsedTrace: Trace.Handlers.Types.ParsedTrace): void {
     const markers = new Map<number, HTMLDivElement>();
 
-    const {Meta, PageLoadMetrics} = parsedTrace;
+    const {Meta} = parsedTrace;
 
-    // Add markers for navigation start times.
+    // Only add markers for navigation start times.
     const navStartEvents = Meta.mainFrameNavigations;
     const minTimeInMilliseconds = Trace.Helpers.Timing.microSecondsToMilliseconds(Meta.traceBounds.min);
 
@@ -253,12 +253,6 @@ export class TimelineMiniMap extends
       markers.set(startTime, TimelineUIUtils.createEventDivider(event, minTimeInMilliseconds));
     }
 
-    // Now add markers for the page load events
-    for (const event of PageLoadMetrics.allMarkerEvents) {
-      const {startTime} = Trace.Helpers.Timing.eventTimingsMilliSeconds(event);
-      markers.set(startTime, TimelineUIUtils.createEventDivider(event, minTimeInMilliseconds));
-    }
-
     this.#overviewComponent.setMarkers(markers);
   }
 

front_end/panels/timeline/TimelineUIUtils.ts

@@ -2374,12 +2374,14 @@ export class TimelineUIUtils {
   }
 
   static markerStyleForEvent(event: Trace.Types.Events.Event): TimelineMarkerStyle {
+    // Note: keep the colors matching that of `markerDetailsForEvent`.
+
     const tallMarkerDashStyle = [6, 4];
     const title = TimelineUIUtils.eventTitle(event);
 
     if (event.name !== Trace.Types.Events.Name.NAVIGATION_START &&
-            Trace.Helpers.Trace.eventHasCategory(event, Trace.Types.Events.Categories.Console) ||
-        Trace.Helpers.Trace.eventHasCategory(event, Trace.Types.Events.Categories.UserTiming)) {
+        (Trace.Helpers.Trace.eventHasCategory(event, Trace.Types.Events.Categories.Console) ||
+         Trace.Helpers.Trace.eventHasCategory(event, Trace.Types.Events.Categories.UserTiming))) {
       return {
         title,
         dashStyle: tallMarkerDashStyle,
@@ -2393,7 +2395,7 @@ export class TimelineUIUtils {
     let color = 'grey';
     switch (event.name) {
       case Trace.Types.Events.Name.NAVIGATION_START:
-        color = '#FF9800';
+        color = 'var(--color-text-primary)';
         tall = true;
         break;
       case Trace.Types.Events.Name.FRAME_STARTED_LOADING:
@@ -2413,11 +2415,11 @@ export class TimelineUIUtils {
         tall = true;
         break;
       case Trace.Types.Events.Name.MARK_FCP:
-        color = '#1A6937';
+        color = 'var(--sys-color-green-bright)';
         tall = true;
         break;
       case Trace.Types.Events.Name.MARK_LCP_CANDIDATE:
-        color = '#1A3422';
+        color = 'var(--sys-color-green)';
         tall = true;
         break;
       case Trace.Types.Events.Name.TIME_STAMP:
@@ -2636,7 +2638,7 @@ export function timeStampForEventAdjustedForClosestNavigationIfPossible(
 export function isMarkerEvent(parsedTrace: Trace.Handlers.Types.ParsedTrace, event: Trace.Types.Events.Event): boolean {
   const {Name} = Trace.Types.Events;
 
-  if (event.name === Name.TIME_STAMP) {
+  if (event.name === Name.TIME_STAMP || event.name === Name.NAVIGATION_START) {
     return true;
   }
 

test/interactions/panels/performance/timeline/overview_test.ts

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 import {assert} from 'chai';
 
-import {waitFor, waitForMany} from '../../../../shared/helper.js';
+import {waitFor} from '../../../../shared/helper.js';
 import {assertElementScreenshotUnchanged} from '../../../../shared/screenshots.js';
 import {loadComponentDocExample} from '../../../helpers/shared.js';
 
@@ -39,18 +39,15 @@ describe('Performance panel overview/minimap', function() {
     await assertElementScreenshotUnchanged(pane, 'performance/timeline-overview-memory.png', 3);
   });
 
-  it('renders markers in the minimap correctly', async () => {
+  it('renders markers in the minimap correctly (just nav)', async () => {
     await loadComponentDocExample('performance_panel/basic.html?trace=web-dev');
-    const minimapMarkers = await waitForMany('.resources-event-divider', 4);
-    const promises = minimapMarkers.map(handle => {
-      return handle.evaluate(marker => {
-        const markerElement = marker as HTMLElement;
-        return markerElement.style.left;
-      });
-    });
-    const offsets = await Promise.all(promises);
-    offsets.forEach(offset => {
-      assert.isTrue(Boolean(offset));
+    const handle = await waitFor('.resources-event-divider');
+    const promise = handle.evaluate(marker => {
+      const markerElement = marker as HTMLElement;
+      return [markerElement.title, markerElement.style.left];
     });
+    const marker = await promise;
+    assert.strictEqual(marker[0], 'navigationStart at 12\xA0ms');
+    assert.isTrue(Boolean(marker[1]));
   });
 });