From 6205fb5d8cc87cb9f560fd2a1980e72dc7c35665 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= <florian@queze.net>
Date: Thu, 14 Sep 2023 17:42:03 +0200
Subject: [PATCH] Show the hovered time in the ruler at the top of the
 timeline.

---
 src/components/timeline/Selection.css         |  10 ++
 src/components/timeline/Selection.js          |  16 ++-
 src/profile-logic/committed-ranges.js         |  10 +-
 .../ActiveTabTimeline.test.js.snap            |   6 +-
 .../__snapshots__/Timeline.test.js.snap       |   8 +-
 src/test/unit/marker-schema.test.js           |  14 ++-
 src/utils/format-numbers.js                   | 107 +++++++++++++-----
 7 files changed, 134 insertions(+), 37 deletions(-)

diff --git a/src/components/timeline/Selection.css b/src/components/timeline/Selection.css
index 8552eb9139..e3bfa7ceb4 100644
--- a/src/components/timeline/Selection.css
+++ b/src/components/timeline/Selection.css
@@ -29,6 +29,16 @@
   pointer-events: none;
 }
 
+.timelineSelectionOverlayTime {
+  position: absolute;
+  z-index: 1;
+  padding: 3.25px 8px;
+  border-radius: 0 4px 4px 0;
+  margin-left: 1px;
+  background-color: rgb(153 153 153);
+  color: #fff;
+}
+
 .timelineSelectionOverlay {
   position: absolute;
   z-index: 2;
diff --git a/src/components/timeline/Selection.js b/src/components/timeline/Selection.js
index 8229d5695a..c81ea1ba14 100644
--- a/src/components/timeline/Selection.js
+++ b/src/components/timeline/Selection.js
@@ -243,6 +243,10 @@ class TimelineRulerAndSelection extends React.PureComponent<Props> {
       return;
     }
     const { width, committedRange, changeMouseTimePosition } = this.props;
+    if (width === 0) {
+      // This can happen when hovering before the profile is fully loaded.
+      return;
+    }
 
     const rect = getContentRect(this._container);
     if (
@@ -399,6 +403,7 @@ class TimelineRulerAndSelection extends React.PureComponent<Props> {
       mouseTimePosition,
       width,
       committedRange,
+      zeroAt,
     } = this.props;
 
     let hoverLocation = null;
@@ -431,7 +436,16 @@ class TimelineRulerAndSelection extends React.PureComponent<Props> {
                 : undefined,
             left: hoverLocation === null ? '0' : `${hoverLocation}px`,
           }}
-        />
+        >
+          <span className="timelineSelectionOverlayTime">
+            {mouseTimePosition !== null
+              ? getFormattedTimeLength(
+                  mouseTimePosition - zeroAt,
+                  (committedRange.end - committedRange.start) / width
+                )
+              : null}
+          </span>
+        </div>
       </div>
     );
   }
diff --git a/src/profile-logic/committed-ranges.js b/src/profile-logic/committed-ranges.js
index 7995c8df13..64b8e50cc8 100644
--- a/src/profile-logic/committed-ranges.js
+++ b/src/profile-logic/committed-ranges.js
@@ -4,7 +4,7 @@
 // @flow
 
 import { formatTimestamp } from 'firefox-profiler/utils/format-numbers';
-import type { StartEndRange } from 'firefox-profiler/types';
+import type { Milliseconds, StartEndRange } from 'firefox-profiler/types';
 
 /**
  * Users can make preview range selections on the profile, and then can commit these
@@ -164,11 +164,15 @@ export function stringifyCommittedRanges(
   return arrayValue.map(stringifyStartEnd).join('~');
 }
 
-export function getFormattedTimeLength(length: number): string {
+export function getFormattedTimeLength(
+  length: Milliseconds,
+  precision: Milliseconds = Infinity
+): string {
   return formatTimestamp(
     length,
     /*significantdigits*/ 2,
-    /*maxFractionalDigits*/ 2
+    /*maxFractionalDigits*/ 2,
+    precision
   );
 }
 
diff --git a/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap b/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap
index b2ababdffe..097fc11f8c 100644
--- a/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap
+++ b/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap
@@ -502,6 +502,10 @@ exports[`ActiveTabTimeline should be rendered properly from the Timeline compone
   <div
     class="timelineSelectionHoverLine"
     style="visibility: hidden; left: 0px;"
-  />
+  >
+    <span
+      class="timelineSelectionOverlayTime"
+    />
+  </div>
 </div>
 `;
diff --git a/src/test/components/__snapshots__/Timeline.test.js.snap b/src/test/components/__snapshots__/Timeline.test.js.snap
index 77465d5312..ea86c5e0a9 100644
--- a/src/test/components/__snapshots__/Timeline.test.js.snap
+++ b/src/test/components/__snapshots__/Timeline.test.js.snap
@@ -253,7 +253,13 @@ Process: \\"default\\" (0)"
       <div
         class="timelineSelectionHoverLine"
         style="left: 120px;"
-      />
+      >
+        <span
+          class="timelineSelectionOverlayTime"
+        >
+          3ms
+        </span>
+      </div>
     </div>
     <nav
       class="react-contextmenu timelineTrackContextMenu"
diff --git a/src/test/unit/marker-schema.test.js b/src/test/unit/marker-schema.test.js
index 47aa263142..a3ddcb3b39 100644
--- a/src/test/unit/marker-schema.test.js
+++ b/src/test/unit/marker-schema.test.js
@@ -220,13 +220,16 @@ describe('marker schema formatting', function () {
       ['duration', 50000000],
       ['duration', 123456789],
       ['duration', 0.000123456],
-      ['duration', 1000 * 60 - 0.01], // slightly less than 1min, should not be shown as '60s'
+      ['duration', 1000 * 60 - 501], // slightly less than 1min, should not be shown as '60s'
+      ['duration', 1000 * 60 - 500], // slightly less than 1min, should not be shown as '60s'
       ['duration', 1000 * 60 + 1], // slightly more than 1min, should not be shown as '1min0s'
       ['duration', 779873.639], // '13min60s' should be rounded to '14min'
-      ['duration', 1000 * 3600 - 0.01], // slightly less than 1h, should not be shown as '0h60min'
+      ['duration', 1000 * 3600 - 501], // 1h - 501ms slightly less than 1h, should be shown as '59min59s'
+      ['duration', 1000 * 3600 - 500], // 1h - 500s slightly less than 1h, should be shown as '1h', not '0h60min'
       ['duration', 1000 * 3600 + 1], // slightly more than 1h, should not be shown as '1h0min'
-      ['duration', 1000 * 3600 * 24 - 0.01], // slightly less than 1 day, should not be shown as '0d24h'
-      ['duration', 1000 * 3600 * 24 + 1], // slightly more than 1 day, should not be shown as '1min0s'
+      ['duration', 1000 * 3600 * 24 - 31 * 1000], // 1d - 31s, should be shown as '23h59min'
+      ['duration', 1000 * 3600 * 24 - 30 * 1000], // 1d - 30s, should be shown as '1d', not '0d24h'
+      ['duration', 1000 * 3600 * 24 + 1], // slightly more than 1 day, should not be shown as '1d0h'
       ['time', 12.3456789],
       ['seconds', 0],
       ['seconds', 10],
@@ -303,11 +306,14 @@ describe('marker schema formatting', function () {
         "duration - 13h53min",
         "duration - 1d10h",
         "duration - 123.46ns",
+        "duration - 59.499s",
         "duration - 1min",
         "duration - 1min",
         "duration - 13min",
+        "duration - 59min59s",
         "duration - 1h",
         "duration - 1h",
+        "duration - 23h59min",
         "duration - 1d",
         "duration - 1d",
         "time - 12.346ms",
diff --git a/src/utils/format-numbers.js b/src/utils/format-numbers.js
index c3e69976cc..f284158581 100644
--- a/src/utils/format-numbers.js
+++ b/src/utils/format-numbers.js
@@ -237,27 +237,43 @@ export function formatMilliseconds(
 export function formatSeconds(
   time: Milliseconds,
   significantDigits: number = 5,
-  maxFractionalDigits: number = 3
+  maxFractionalDigits: number = 3,
+  precision: Milliseconds = Infinity
 ) {
-  return (
-    formatNumber(time / 1000, significantDigits, maxFractionalDigits) + 's'
-  );
+  const msPerSecond = 1000;
+  const timeInSeconds = time / msPerSecond;
+  let result = '';
+  if (precision !== 0 && precision < msPerSecond) {
+    const exponent = Math.floor(Math.log10(precision / msPerSecond));
+    const digits = Math.max(0, -exponent);
+    result = timeInSeconds.toFixed(digits);
+  } else {
+    result = formatNumber(
+      timeInSeconds,
+      significantDigits,
+      maxFractionalDigits
+    );
+  }
+  return result + 's';
 }
 
 export function formatMinutes(
   time: Milliseconds,
   significantDigits: number = 5,
-  maxFractionalDigits: number = 2
+  maxFractionalDigits: number = 2,
+  precision: Milliseconds = Infinity
 ) {
   const msPerSecond = 1000;
-  time = Math.round(time / msPerSecond) * msPerSecond;
   const msPerMinute = 60 * msPerSecond;
+  if (precision >= msPerSecond) {
+    time = Math.round(time / msPerSecond) * msPerSecond;
+  }
   const seconds = time % msPerMinute;
   return (
     formatNumber((time - seconds) / msPerMinute, significantDigits, 0) +
     'min' +
-    (maxFractionalDigits > 0 && seconds > 0
-      ? formatSeconds(seconds, significantDigits, 0)
+    (seconds > 0 && (maxFractionalDigits > 0 || precision < msPerMinute)
+      ? formatSeconds(seconds, significantDigits, 0, precision)
       : '')
   );
 }
@@ -265,17 +281,20 @@ export function formatMinutes(
 export function formatHours(
   time: Milliseconds,
   significantDigits: number = 5,
-  maxFractionalDigits: number = 1
+  maxFractionalDigits: number = 1,
+  precision: Milliseconds = Infinity
 ) {
   const msPerMinute = 60 * 1000;
-  time = Math.round(time / msPerMinute) * msPerMinute;
+  if (precision >= msPerMinute) {
+    time = Math.round(time / msPerMinute) * msPerMinute;
+  }
   const msPerHour = 60 * msPerMinute;
   const minutes = time % msPerHour;
   return (
     formatNumber((time - minutes) / msPerHour, significantDigits, 0) +
     'h' +
-    (maxFractionalDigits > 0 && minutes > 0
-      ? formatMinutes(minutes, significantDigits, 0)
+    (minutes > 0 && (maxFractionalDigits > 0 || precision < msPerHour)
+      ? formatMinutes(minutes, significantDigits, 0, precision)
       : '')
   );
 }
@@ -283,17 +302,20 @@ export function formatHours(
 export function formatDays(
   time: Milliseconds,
   significantDigits: number = 5,
-  maxFractionalDigits: number = 1
+  maxFractionalDigits: number = 1,
+  precision: Milliseconds = Infinity
 ) {
   const msPerHour = 60 * 60 * 1000;
-  time = Math.round(time / msPerHour) * msPerHour;
+  if (precision >= msPerHour) {
+    time = Math.round(time / msPerHour) * msPerHour;
+  }
   const msPerDay = 24 * msPerHour;
   const hours = time % msPerDay;
   return (
     formatNumber((time - hours) / msPerDay, significantDigits, 0) +
     'd' +
-    (maxFractionalDigits > 0 && hours > 0
-      ? formatHours(hours, significantDigits, 0)
+    (hours > 0 && (maxFractionalDigits > 0 || precision < msPerDay)
+      ? formatHours(hours, significantDigits, 0, precision)
       : '')
   );
 }
@@ -301,28 +323,59 @@ export function formatDays(
 export function formatTimestamp(
   time: Milliseconds,
   significantDigits: number = 5,
-  maxFractionalDigits: number = 3
+  maxFractionalDigits: number = 3,
+  // precision is the minimum required precision.
+  precision: Milliseconds = Infinity
 ) {
+  if (precision !== Infinity) {
+    // Round the values to display nicer numbers when the extra precision
+    // isn't useful. (eg. show 3h52min10s instead of 3h52min14s)
+    precision = 10 ** Math.floor(Math.log10(precision));
+    if (time > precision) {
+      time = Math.round(time / precision) * precision;
+    }
+  }
   // Format in the closest base (days, hours, minutes, seconds, milliseconds,
   // microseconds, or nanoseconds), to avoid cases where times are displayed
   // with too many leading zeroes to be useful.
-  // The Math.round call is needed to avoid showing values like '0min60s'.
-  if (Math.round(time / 1000) / 60 >= 1) {
-    // The if blocks are nested to avoid calling Math.round repeatedly for
-    // the most common case where the value will be less than 1 minute.
-    if (Math.round(time / (60 * 1000)) / 60 >= 1) {
-      if (Math.round(time / (60 * 60 * 1000)) / 24 >= 1) {
-        return formatDays(time, significantDigits, maxFractionalDigits);
+  // 59.5s is the smallest value rounded to 1min.
+  if (time >= 60 * 1000 - (precision === Infinity ? 500 : 0)) {
+    // The if blocks are nested to avoid repeated tests for the most
+    // common case where the value will be less than 1 minute.
+    // 59min59.5s is the smallest value rounded to 1h
+    if (time >= 60 * 60 * 1000 - (precision === Infinity ? 500 : 0)) {
+      // 23h59min30s is the smallest value rounded to 1d.
+      if (
+        time >=
+        24 * 60 * 60 * 1000 - (precision === Infinity ? 30 * 1000 : 0)
+      ) {
+        return formatDays(
+          time,
+          significantDigits,
+          maxFractionalDigits,
+          precision
+        );
       }
-      return formatHours(time, significantDigits, maxFractionalDigits);
+      return formatHours(
+        time,
+        significantDigits,
+        maxFractionalDigits,
+        precision
+      );
     }
-    return formatMinutes(time, significantDigits, maxFractionalDigits);
+    return formatMinutes(
+      time,
+      significantDigits,
+      maxFractionalDigits,
+      precision
+    );
   }
   if (time >= 1000) {
     return formatSeconds(
       time,
       significantDigits,
-      Number.isInteger(time / 1000) ? 0 : maxFractionalDigits
+      Number.isInteger(time / 1000) ? 0 : maxFractionalDigits,
+      precision
     );
   }
   if (time >= 1) {