Consolidate two UserTimingsHandler sorting functions into one.

BUG: 428939519
Change-Id: Ie72e2697b0aa30fffebe8011e2cf472c63396bcf
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6895484
Reviewed-by: Andres Olivares <andoli@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>

Author: finnurbreki

front_end/models/trace/handlers/UserTimingsHandler.test.ts

@@ -339,7 +339,7 @@ describeWithEnvironment('UserTimingsHandler', function() {
     });
   });
 
-  describe('sorting functions', () => {
+  describe('sorting function userTimingComparator', () => {
     function makeFakeSyntheticEvent(
         ts: number, dur: number, name: string, track: string): Trace.Types.Events.SyntheticEventPair {
       const beginEvent = makeCompleteEvent(name, ts, dur, 'blink.user_timing', 0, 0) as unknown as
@@ -364,11 +364,31 @@ describeWithEnvironment('UserTimingsHandler', function() {
       return syntheticEvent;
     }
 
-    function sortAll(events: Trace.Types.Events.SyntheticEventPair[]) {
+    function makeFakeConsoleTimestampEvent(
+        start: number, end: number, name: string, track: string): Trace.Types.Events.ConsoleTimeStamp {
+      return {
+        cat: 'devtools.timeline',
+        pid: Trace.Types.Events.ProcessID(2017),
+        tid: Trace.Types.Events.ThreadID(259),
+        name: Trace.Types.Events.Name.TIME_STAMP,
+        args: {
+          data: {
+            message: name,
+            start,
+            end,
+            track,
+          },
+        },
+        ts: Trace.Types.Timing.Micro(start),
+        ph: Trace.Types.Events.Phase.INSTANT,
+      };
+    }
+
+    function sortAll(events: Array<Trace.Types.Events.SyntheticEventPair|Trace.Types.Events.ConsoleTimeStamp>) {
       events.sort((a, b) => Trace.Handlers.ModelHandlers.UserTimings.userTimingComparator(a, b, [...events]));
     }
 
-    it('userTimingComparator sorts by start time in ASC order', () => {
+    it('sorts synthetic events by start time in ASC order', () => {
       const event1 = makeFakeSyntheticEvent(1, 1, 'E1', 'Track A');
       const event2 = makeFakeSyntheticEvent(2, 1, 'E2', 'Track A');
       const event3 = makeFakeSyntheticEvent(3, 1, 'E3', 'Track A');
@@ -378,7 +398,7 @@ describeWithEnvironment('UserTimingsHandler', function() {
       assert.deepEqual(events, [event1, event2, event3]);
     });
 
-    it('userTimingComparator sorts by longest duration if start is the same for all', () => {
+    it('sorts synthetic events by longest duration if start is the same for all', () => {
       const event1 = makeFakeSyntheticEvent(1, 1, 'E1', 'Track A');
       const event2 = makeFakeSyntheticEvent(1, 2, 'E2', 'Track A');
       const event3 = makeFakeSyntheticEvent(1, 3, 'E3', 'Track A');
@@ -388,7 +408,7 @@ describeWithEnvironment('UserTimingsHandler', function() {
       assert.deepEqual(events, [event3, event2, event1]);
     });
 
-    it('userTimingComparator flips if the timestamps are the same', () => {
+    it('flips synthetic events if the timestamps are the same', () => {
       const event1 = makeFakeSyntheticEvent(1, 2, 'E1', 'Track A');
       const event2 = makeFakeSyntheticEvent(1, 2, 'E2', 'Track A');
       const event3 = makeFakeSyntheticEvent(2, 4, 'E3', 'Track B');
@@ -399,7 +419,7 @@ describeWithEnvironment('UserTimingsHandler', function() {
       assert.deepEqual(events, [event2, event1, event4, event3]);
     });
 
-    it('userTimingComparator wont flip across tracks', () => {
+    it('wont flip synthetic events across tracks', () => {
       const event1 = makeFakeSyntheticEvent(1, 2, 'E1', 'Track A');
       const event2 = makeFakeSyntheticEvent(1, 2, 'E2', 'Track B');
       const event3 = makeFakeSyntheticEvent(1, 2, 'E3', 'Track C');
@@ -410,7 +430,7 @@ describeWithEnvironment('UserTimingsHandler', function() {
       assert.deepEqual(events, [event1, event2, event3, event4]);
     });
 
-    it('userTimingComparator three identical measure events', () => {
+    it('sorts three identical synthetic events correctly', () => {
       const event1 = makeFakeSyntheticEvent(1, 2, 'E1', 'Track A');
       const event2 = makeFakeSyntheticEvent(1, 2, 'E2', 'Track A');
       const event3 = makeFakeSyntheticEvent(1, 2, 'E3', 'Track A');
@@ -420,77 +440,86 @@ describeWithEnvironment('UserTimingsHandler', function() {
       assert.deepEqual(events, [event3, event2, event1]);
     });
 
-    function makeFakeConsoleTimestampEvent(
-        start: number, end: number, name: string, track: string): Trace.Types.Events.ConsoleTimeStamp {
-      return {
-        cat: 'devtools.timeline',
-        pid: Trace.Types.Events.ProcessID(2017),
-        tid: Trace.Types.Events.ThreadID(259),
-        name: Trace.Types.Events.Name.TIME_STAMP,
-        args: {
-          data: {
-            message: name,
-            start,
-            end,
-            track,
-          },
-        },
-        ts: Trace.Types.Timing.Micro(start),
-        ph: Trace.Types.Events.Phase.INSTANT,
-      };
-    }
-
-    function sortAllConsoleTimestamps(events: Trace.Types.Events.ConsoleTimeStamp[]) {
-      events.sort((a, b) => Trace.Handlers.ModelHandlers.UserTimings.consoleTimestampComparator(a, b, [...events]));
-    }
-
-    it('consoleTimestampComparator sorts by start time in ASC order', () => {
+    it('sorts console timestamps by start time in ASC order', () => {
       const event1 = makeFakeConsoleTimestampEvent(1, 1, 'E1', 'Track A');
       const event2 = makeFakeConsoleTimestampEvent(2, 1, 'E2', 'Track A');
       const event3 = makeFakeConsoleTimestampEvent(3, 1, 'E3', 'Track A');
       const events = [event3, event1, event2];
-      sortAllConsoleTimestamps(events);
+
+      sortAll(events);
       assert.deepEqual(events, [event1, event2, event3]);
     });
 
-    it('consoleTimestampComparator sorts by longest duration if start is the same for all', () => {
+    it('sorts console timestamps by longest duration if start is the same for all', () => {
       const event1 = makeFakeConsoleTimestampEvent(1, 1, 'E1', 'Track A');
       const event2 = makeFakeConsoleTimestampEvent(1, 2, 'E2', 'Track A');
       const event3 = makeFakeConsoleTimestampEvent(1, 3, 'E3', 'Track A');
       const events = [event1, event2, event3];
-      sortAllConsoleTimestamps(events);
+
+      sortAll(events);
       assert.deepEqual(events, [event3, event2, event1]);
     });
 
-    it('consoleTimestampComparator flips if the timestamps are the same', () => {
+    it('flips console timestamps if the timestamps are the same', () => {
       const event1 = makeFakeConsoleTimestampEvent(1, 3, 'E1', 'Track A');
       const event2 = makeFakeConsoleTimestampEvent(1, 3, 'E2', 'Track A');
       const event3 = makeFakeConsoleTimestampEvent(2, 6, 'E3', 'Track B');
       const event4 = makeFakeConsoleTimestampEvent(2, 6, 'E4', 'Track B');
       const events = [event1, event2, event3, event4];
-      sortAllConsoleTimestamps(events);
+
+      sortAll(events);
       assert.deepEqual(events, [event2, event1, event4, event3]);
     });
 
-    it('consoleTimestampComparator wont flip across tracks', () => {
+    it('wont flip console timestamps across tracks', () => {
       const event1 = makeFakeConsoleTimestampEvent(1, 2, 'E1', 'Track A');
       const event2 = makeFakeConsoleTimestampEvent(1, 2, 'E2', 'Track B');
       const event3 = makeFakeConsoleTimestampEvent(1, 2, 'E3', 'Track C');
       const event4 = makeFakeConsoleTimestampEvent(1, 2, 'E4', 'Track D');
       const events = [event1, event2, event3, event4];
 
-      sortAllConsoleTimestamps(events);
+      sortAll(events);
       assert.deepEqual(events, [event1, event2, event3, event4]);
     });
 
-    it('consoleTimestampComparator three identical measure events', () => {
+    it('sorts three identical console timestamp events', () => {
       const event1 = makeFakeConsoleTimestampEvent(1, 2, 'E1', 'Track A');
       const event2 = makeFakeConsoleTimestampEvent(1, 2, 'E2', 'Track A');
       const event3 = makeFakeConsoleTimestampEvent(1, 2, 'E3', 'Track A');
       const events = [event1, event2, event3];
 
-      sortAllConsoleTimestamps(events);
+      sortAll(events);
       assert.deepEqual(events, [event3, event2, event1]);
     });
+
+    // Mixed synthetic events and console timestamps.
+
+    it('sorts mixed events by start time in ASC order', () => {
+      const event1a = makeFakeSyntheticEvent(1, 1, 'E1a', 'Track A');
+      const event1b = makeFakeConsoleTimestampEvent(1, 1, 'E1b', 'Track A');
+      const event2a = makeFakeSyntheticEvent(2, 1, 'E2a', 'Track A');
+      const event2b = makeFakeConsoleTimestampEvent(2, 1, 'E2b', 'Track A');
+      const event3a = makeFakeSyntheticEvent(3, 1, 'E3a', 'Track A');
+      const event3b = makeFakeConsoleTimestampEvent(3, 1, 'E3b', 'Track A');
+      const events = [event3b, event3a, event1b, event1a, event2b, event2a];
+
+      sortAll(events);
+      assert.deepEqual(events, [event1a, event1b, event2a, event2b, event3a, event3b]);
+    });
+
+    it('wont flip mixed events across tracks', () => {
+      const event1a = makeFakeSyntheticEvent(1, 2, 'E1a', 'Track A');
+      const event2a = makeFakeSyntheticEvent(1, 2, 'E2a', 'Track B');
+      const event3a = makeFakeSyntheticEvent(1, 2, 'E3a', 'Track C');
+      const event4a = makeFakeSyntheticEvent(1, 2, 'E4a', 'Track D');
+      const event1b = makeFakeConsoleTimestampEvent(1, 2, 'E1b', 'Track E');
+      const event2b = makeFakeConsoleTimestampEvent(1, 2, 'E2b', 'Track F');
+      const event3b = makeFakeConsoleTimestampEvent(1, 2, 'E3b', 'Track G');
+      const event4b = makeFakeConsoleTimestampEvent(1, 2, 'E4b', 'Track H');
+      const events = [event1a, event2a, event3a, event4a, event1b, event2b, event3b, event4b];
+
+      sortAll(events);
+      assert.deepEqual(events, [event1a, event2a, event3a, event4a, event1b, event2b, event3b, event4b]);
+    });
   });
 });

front_end/models/trace/handlers/UserTimingsHandler.ts

@@ -112,6 +112,44 @@ const navTimingNames = [
 // flame chart).
 const ignoredNames = [...resourceTimingNames, ...navTimingNames];
 
+function getEventTimings(event: Types.Events.SyntheticEventPair|Types.Events.ConsoleTimeStamp):
+    {start: Types.Timing.Micro, end: Types.Timing.Micro} {
+  if ('dur' in event) {
+    // It's a SyntheticEventPair.
+    return {start: event.ts, end: Types.Timing.Micro(event.ts + (event.dur ?? 0))};
+  }
+
+  if (Types.Events.isConsoleTimeStamp(event)) {
+    const {start, end} = event.args.data || {};
+    if (typeof start === 'number' && typeof end === 'number') {
+      return {start: Types.Timing.Micro(start), end: Types.Timing.Micro(end)};
+    }
+  }
+
+  // A ConsoleTimeStamp without start/end is just a point in time, so dur is 0.
+  return {start: event.ts, end: event.ts};
+}
+
+function getEventTrack(event: Types.Events.SyntheticEventPair|Types.Events.ConsoleTimeStamp): string|undefined {
+  if (event.cat === 'blink.user_timing') {
+    // This is a SyntheticUserTimingPair
+    const detailString =
+        ((event as Types.Events.SyntheticUserTimingPair).args.data.beginEvent.args as {detail?: string})?.detail;
+    if (detailString) {
+      const details = Helpers.Trace.parseDevtoolsDetails(detailString, 'devtools');
+      if (details && 'track' in details) {
+        return details.track;
+      }
+    }
+  } else if (Types.Events.isConsoleTimeStamp(event)) {
+    const track = event.args.data?.track;
+    return typeof track === 'string' ? track : undefined;
+  }
+
+  // SyntheticConsoleTimingPair does not have track info.
+  return undefined;
+}
+
 /**
  * Similar to the default {@see Helpers.Trace.eventTimeComparator}
  * but with a twist:
@@ -132,55 +170,18 @@ const ignoredNames = [...resourceTimingNames, ...navTimingNames];
  * the second event is the parent of the first. Hence the switch.
  *
  */
-export function userTimingComparator(
-    a: Types.Events.SyntheticEventPair, b: Types.Events.SyntheticEventPair,
-    originalArray: readonly Types.Events.SyntheticEventPair[]): number {
-  const aStart = a.ts;
-  const bStart = b.ts;
-  const aEnd = aStart + (a.dur ?? 0);
-  const bEnd = bStart + (b.dur ?? 0);
-  const timeDifference = Helpers.Trace.compareBeginAndEnd(aStart, bStart, aEnd, bEnd);
-  if (timeDifference) {
-    return timeDifference;
-  }
-
-  // Never re-order entries across different tracks.
-  const aDetailString = (a.args.data.beginEvent.args as {detail?: string})?.detail;
-  const bDetailString = (b.args.data.beginEvent.args as {detail?: string})?.detail;
-  const aDetails = aDetailString ? Helpers.Trace.parseDevtoolsDetails(aDetailString, 'devtools') : null;
-  const bDetails = bDetailString ? Helpers.Trace.parseDevtoolsDetails(bDetailString, 'devtools') : null;
-  const aTrack = aDetails && 'track' in aDetails ? aDetails.track : undefined;
-  const bTrack = bDetails && 'track' in bDetails ? bDetails.track : undefined;
-  if (aTrack !== bTrack) {
-    return 0;  // Preserve current positions.
-  }
-
-  // Prefer the event located in a further position in the original array.
-  const aIndex = originalArray.indexOf(a);
-  const bIndex = originalArray.indexOf(b);
-  return bIndex - aIndex;
-}
-
-export function consoleTimestampComparator(
-    a: Types.Events.ConsoleTimeStamp, b: Types.Events.ConsoleTimeStamp,
-    originalArray: readonly Types.Events.ConsoleTimeStamp[]): number {
-  const aStart = a.args.data?.start;
-  const aEnd = a.args.data?.end;
-  const bStart = b.args.data?.start;
-  const bEnd = b.args.data?.end;
-  if (typeof aStart !== 'number' || typeof aEnd !== 'number' || typeof bStart !== 'number' ||
-      typeof bEnd !== 'number') {
-    return 0;
-  }
-
+export function userTimingComparator<T extends Types.Events.SyntheticEventPair|Types.Events.ConsoleTimeStamp>(
+    a: T, b: T, originalArray: readonly T[]): number {
+  const {start: aStart, end: aEnd} = getEventTimings(a);
+  const {start: bStart, end: bEnd} = getEventTimings(b);
   const timeDifference = Helpers.Trace.compareBeginAndEnd(aStart, bStart, aEnd, bEnd);
   if (timeDifference) {
     return timeDifference;
   }
 
   // Never re-order entries across different tracks.
-  const aTrack = a.args.data?.track;
-  const bTrack = b.args.data?.track;
+  const aTrack = getEventTrack(a);
+  const bTrack = getEventTrack(b);
   if (aTrack !== bTrack) {
     return 0;  // Preserve current positions.
   }
@@ -217,7 +218,7 @@ export async function finalize(): Promise<void> {
   const asyncEvents = [...performanceMeasureEvents, ...consoleTimings];
   syntheticEvents = Helpers.Trace.createMatchedSortedSyntheticEvents(asyncEvents);
   syntheticEvents = syntheticEvents.sort((a, b) => userTimingComparator(a, b, [...syntheticEvents]));
-  timestampEvents = timestampEvents.sort((a, b) => consoleTimestampComparator(a, b, [...timestampEvents]));
+  timestampEvents = timestampEvents.sort((a, b) => userTimingComparator(a, b, [...timestampEvents]));
 }
 
 export function data(): UserTimingsData {