Make CompositorScreenshot markers schema-based

docs-developer/CHANGELOG-formats.md

@@ -6,6 +6,14 @@ Note that this is not an exhaustive list. Processed profile format upgraders can
 
 ## Processed profile format
 
+### Version 70
+
+The `CompositorScreenshot` marker payload's `windowWidth` and `windowHeight` fields were replaced with a single `windowSize` field of the form `{ width, height }`.
+
+These markers are now stored as pairs of start and end markers instead of instant markers. The window ID is part of the marker name (`CompositorScreenshot <windowID>`), so starts and ends are matched by name like any other marker pair. The `CompositorScreenshotWindowDestroyed` marker is gone: it is now the end marker of that window's last screenshot. The last screenshot of a window that is never destroyed has no end marker, and is extended to the end of the thread.
+
+Two marker schema field formats were added to describe these markers: `screenshot-size`, whose value is a `{ width, height }` object, and `screenshot-data-url`, an object format `{ type: "screenshot-data-url", sizeFieldForAspectRatio }` whose value is a string table index holding an image data URL. Profiles containing `CompositorScreenshot` markers don't necessarily carry a schema for them, so the front end supplies one.
+
 ### Version 69
 
 A new marker schema display location, `timeline-network`, was added. A marker schema can list `timeline-network` in its `display` array to have markers of that type surfaced in the Network track.

src/app-logic/constants.ts

@@ -12,7 +12,7 @@ export const GECKO_PROFILE_VERSION = 36;
 // The current version of the "processed" profile format.
 // Please don't forget to update the processed profile format changelog in
 // `docs-developer/CHANGELOG-formats.md`.
-export const PROCESSED_PROFILE_VERSION = 69;
+export const PROCESSED_PROFILE_VERSION = 70;
 
 // The following are the margin sizes for the left and right of the timeline. Independent
 // components need to share these values.

src/components/timeline/TrackScreenshots.tsx

@@ -244,19 +244,17 @@ class HoverPreview extends PureComponent<HoverPreviewProps> {
       return null;
     }
 
-    if (payload.url === undefined) {
+    const { url, windowSize } = payload;
+    if (url === undefined || windowSize === undefined) {
       return null;
     }
 
-    const { url } = payload;
-
     const maximumHoverSize = isMakingPreviewSelection
       ? MAXIMUM_HOVER_SIZE_WHEN_SELECTING_RANGE
       : MAXIMUM_HOVER_SIZE;
 
-    // Type guard: payload.url !== undefined means it has windowWidth and windowHeight
     const { width: hoverWidth, height: hoverHeight } = computeScreenshotSize(
-      payload as { windowWidth: number; windowHeight: number },
+      windowSize,
       maximumHoverSize
     );
 
@@ -354,18 +352,12 @@ class ScreenshotStrip extends PureComponent<ScreenshotStripProps> {
       // Coerce the payload into a screenshot one.
       const payload: ScreenshotPayload = screenshots[screenshotIndex]
         .data as any;
-      if (payload.url === undefined) {
+      const { url: urlStringIndex, windowSize } = payload;
+      if (urlStringIndex === undefined || windowSize === undefined) {
         continue;
       }
-      const {
-        url: urlStringIndex,
-        windowWidth,
-        windowHeight,
-      } = payload as ScreenshotPayload & {
-        windowWidth: number;
-        windowHeight: number;
-      };
-      const scaledImageWidth = (trackHeight * windowWidth) / windowHeight;
+      const scaledImageWidth =
+        (trackHeight * windowSize.width) / windowSize.height;
       images.push(
         <div
           className="timelineTrackScreenshotImgContainer"
@@ -378,7 +370,7 @@ class ScreenshotStrip extends PureComponent<ScreenshotStripProps> {
           {/* The following image is centered and cropped by the outer container. */}
           <img
             className="timelineTrackScreenshotImg"
-            src={thread.stringTable.getString(urlStringIndex as number)}
+            src={thread.stringTable.getString(urlStringIndex)}
             style={{
               width: scaledImageWidth,
               height: trackHeight,

src/components/tooltip/Marker.tsx

@@ -56,6 +56,7 @@ import type {
   MarkerSchemaByName,
   MarkerIndex,
   MarkerFormatType,
+  MarkerPayload,
   InnerWindowID,
   Page,
   Pid,
@@ -294,7 +295,8 @@ class MarkerTooltipContents extends React.PureComponent<Props> {
                 value,
                 thread.stringTable,
                 threadIdToNameMap,
-                processIdToNameMap
+                processIdToNameMap,
+                data
               )}
             </TooltipDetail>
           );
@@ -352,67 +354,6 @@ class MarkerTooltipContents extends React.PureComponent<Props> {
           );
           break;
         }
-        case 'CompositorScreenshot': {
-          if (
-            data.url !== undefined &&
-            'windowWidth' in data &&
-            'windowHeight' in data
-          ) {
-            const { width, height } = computeScreenshotSize(
-              data,
-              MAXIMUM_IMAGE_SIZE
-            );
-            details.push(
-              <TooltipDetail label="Image" key="CompositorScreenshot-image">
-                <img
-                  className="tooltipScreenshotImg"
-                  src={thread.stringTable.getString(data.url)}
-                  style={{
-                    width,
-                    height,
-                  }}
-                />
-              </TooltipDetail>,
-              <TooltipDetail
-                label="Window Size"
-                key="CompositorScreenshot-window size"
-              >
-                <>
-                  {data.windowWidth}px × {data.windowHeight}px
-                </>
-              </TooltipDetail>,
-              <TooltipDetail
-                label="Description"
-                key="CompositorScreenshot-description"
-              >
-                This marker spans the time between each composite of a window
-                and shows the window contents during that time.
-              </TooltipDetail>,
-              <TooltipDetail
-                label="Window ID"
-                key="CompositorScreenshot-window id"
-              >
-                {data.windowID}
-              </TooltipDetail>
-            );
-          } else if (marker.name === 'CompositorScreenshotWindowDestroyed') {
-            details.push(
-              <TooltipDetail
-                label="Description"
-                key="CompositorScreenshot-description"
-              >
-                This marker shows the moment a window has been destroyed.
-              </TooltipDetail>,
-              <TooltipDetail
-                label="Window ID"
-                key="CompositorScreenshot-window id"
-              >
-                {data.windowID}
-              </TooltipDetail>
-            );
-          }
-          break;
-        }
         default:
         // Do nothing
       }
@@ -587,7 +528,7 @@ const URL_REGEXP = /^(https?:\/\/)\S+$/;
 
 /**
  * This function may return structured markup for some types suchs as table,
- * list, or urls. For other types this falls back to formatFromMarkerSchema
+ * list, urls, or images. For other types this falls back to formatFromMarkerSchema
  * above.
  */
 export function renderMarkerFieldValue(
@@ -596,7 +537,9 @@ export function renderMarkerFieldValue(
   value: any,
   stringTable: StringTable,
   threadIdToNameMap?: Map<Tid, string>,
-  processIdToNameMap?: Map<Pid, string>
+  processIdToNameMap?: Map<Pid, string>,
+  // The payload the value comes from, for formats that refer to a sibling field.
+  payload?: MarkerPayload | null
 ): React.ReactElement | string {
   if (value === undefined || value === null) {
     console.warn(`Formatting ${value} for ${JSON.stringify(markerType)}`);
@@ -653,7 +596,8 @@ export function renderMarkerFieldValue(
                             cell,
                             stringTable,
                             threadIdToNameMap,
-                            processIdToNameMap
+                            processIdToNameMap,
+                            payload
                           )}
                         </td>
                       );
@@ -665,6 +609,23 @@ export function renderMarkerFieldValue(
           </table>
         );
       }
+      case 'screenshot-data-url': {
+        const size = (payload as any)?.[format.sizeFieldForAspectRatio];
+        return (
+          <img
+            className="tooltipScreenshotImg"
+            src={stringTable.getString(value)}
+            style={
+              size
+                ? computeScreenshotSize(size, MAXIMUM_IMAGE_SIZE)
+                : {
+                    maxWidth: MAXIMUM_IMAGE_SIZE,
+                    maxHeight: MAXIMUM_IMAGE_SIZE,
+                  }
+            }
+          />
+        );
+      }
       default:
         throw new Error(
           `Unknown format type ${JSON.stringify(format as never)}`

src/profile-logic/import/chrome.ts

@@ -6,7 +6,10 @@ import type {
   Profile,
   RawThread,
   IndexIntoStackTable,
+  MarkerPhase,
+  Milliseconds,
   MixedObject,
+  ScreenshotPayload,
 } from 'firefox-profiler/types';
 
 import {
@@ -29,6 +32,7 @@ import {
 } from 'firefox-profiler/app-logic/constants';
 
 import { getTimeRangeForThread } from '../profile-data';
+import { getScreenshotMarkerName } from '../marker-data';
 import { GlobalDataCollector } from '../global-data-collector';
 
 // Chrome Tracing Event Spec:
@@ -878,26 +882,60 @@ async function extractScreenshots(
     );
   }
 
+  // Chrome's Screenshot events don't say which window they belong to, so they
+  // all share one made-up window ID.
+  const windowID = 'id';
+  const nameIndex = stringTable.indexForString(
+    getScreenshotMarkerName(windowID)
+  );
+
+  function pushScreenshotMarker(
+    data: ScreenshotPayload,
+    startTime: Milliseconds | null,
+    endTime: Milliseconds | null,
+    phase: MarkerPhase
+  ) {
+    markers.data.push(data);
+    markers.name.push(nameIndex);
+    markers.startTime.push(startTime);
+    markers.endTime.push(endTime);
+    markers.phase.push(phase);
+    markers.category.push(graphicsIndex);
+    markers.length++;
+  }
+
+  let hasOpenScreenshot = false;
   for (const screenshot of screenshots) {
     const urlString = 'data:image/jpg;base64,' + screenshot.args.snapshot;
     const size = await getImageSize(urlString);
     if (size === null) {
       // The image could not be processed, do not add it.
       continue;
     }
-    markers.data.push({
-      type: 'CompositorScreenshot',
-      url: stringTable.indexForString(urlString),
-      windowID: 'id',
-      windowWidth: size.width,
-      windowHeight: size.height,
-    });
-    markers.name.push(stringTable.indexForString('CompositorScreenshot'));
-    markers.startTime.push(screenshot.ts / 1000);
-    markers.endTime.push(null);
-    markers.phase.push(INSTANT);
-    markers.category.push(graphicsIndex);
-    markers.length++;
+    const startTime = screenshot.ts / 1000;
+
+    // Each screenshot is valid until the next one.
+    if (hasOpenScreenshot) {
+      pushScreenshotMarker(
+        { type: 'CompositorScreenshot', windowID },
+        null,
+        startTime,
+        INTERVAL_END
+      );
+    }
+
+    pushScreenshotMarker(
+      {
+        type: 'CompositorScreenshot',
+        url: stringTable.indexForString(urlString),
+        windowID,
+        windowSize: { width: size.width, height: size.height },
+      },
+      startTime,
+      null,
+      INTERVAL_START
+    );
+    hasOpenScreenshot = true;
   }
 }