Remove getMarkerSchemaName special cases - look up marker schemas from data.type and nothing else

docs-developer/CHANGELOG-formats.md

@@ -6,6 +6,12 @@ Note that this is not an exhaustive list. Processed profile format upgraders can
 
 ## Processed profile format
 
+### Version 52
+
+No format changes, but a front-end behavior change: The schema for a marker is now looked up purely based on its `data.type`. In the past there were some special cases when `data` was `null`, or when `data.type` was `tracing` or `Text`. These special cases have been removed. The new behavior is simpler and more predictable, and was probably what you expected anyway.
+
+This change came with a new version because we needed to upgrade old profiles from Firefox which were relying on the more complex behavior.
+
 ### Version 51
 
 Two new marker schema field format types have been added: `flow-id` and `terminating-flow-id`, with string index values (like `unique-string`).

src/actions/receive-profile.js

@@ -38,7 +38,6 @@ import {
   getRelevantPagesForActiveTab,
   getSymbolServerUrl,
   getActiveTabID,
-  getMarkerSchemaByName,
 } from 'firefox-profiler/selectors';
 import {
   getSelectedTab,
@@ -321,11 +320,7 @@ export function finalizeFullProfileView(
       tabFilter,
       tabToThreadIndexesMap
     );
-    const localTracksByPid = computeLocalTracksByPid(
-      profile,
-      globalTracks,
-      getMarkerSchemaByName(getState())
-    );
+    const localTracksByPid = computeLocalTracksByPid(profile, globalTracks);
 
     const legacyThreadOrder = getLegacyThreadOrder(getState());
     const globalTrackOrder = initializeGlobalTrackOrder(
@@ -1835,11 +1830,7 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction<void> {
       tabID,
       tabToThreadIndexesMap
     );
-    const localTracksByPid = computeLocalTracksByPid(
-      profile,
-      globalTracks,
-      getMarkerSchemaByName(getState())
-    );
+    const localTracksByPid = computeLocalTracksByPid(profile, globalTracks);
 
     const legacyThreadOrder = getLegacyThreadOrder(getState());
     const globalTrackOrder = initializeGlobalTrackOrder(

src/app-logic/constants.js

@@ -14,7 +14,7 @@ export const GECKO_PROFILE_VERSION = 31;
 // 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 = 51;
+export const PROCESSED_PROFILE_VERSION = 52;
 
 // The following are the margin sizes for the left and right of the timeline. Independent
 // components need to share these values.

src/components/tooltip/Marker.js

@@ -239,11 +239,7 @@ class MarkerTooltipContents extends React.PureComponent<Props> {
 
     if (data) {
       // Add the details for the markers based on their Marker schema.
-      const schema = getSchemaFromMarker(
-        markerSchemaByName,
-        marker.name,
-        marker.data
-      );
+      const schema = getSchemaFromMarker(markerSchemaByName, marker.data);
       if (schema) {
         for (const schemaData of schema.data) {
           // Check for a schema that is looking up and formatting a value from

src/profile-logic/gecko-profile-versioning.js

@@ -1262,27 +1262,27 @@ const _upgraders = {
           // eventType is in the payload as well.
         ],
       },
+
+      // The following three schemas should have just been a single schema named
+      // "tracing". They are kept here for historical accuracy.
+      // There is a processed profile format upgrader (version 52) which adds the
+      // "tracing" schema for profiles which don't have it.
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Paint',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Navigation',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Layout',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
+
       {
         name: 'IPC',
         tooltipLabel: 'IPC — {marker.data.niceDirection}',
@@ -1298,6 +1298,14 @@ const _upgraders = {
         ],
       },
       {
+        // An unused schema for RefreshDriverTick markers.
+        // This schema is not consistent with what post-schema Firefox would
+        // output. Firefox (as of Jan 2026) is still using Text markers and does
+        // not have a RefreshDriverTick schema. Furthermore, upgraded profiles
+        // which get this schema do not have any { type: 'RefreshDriverTick' }
+        // markers - in the past they picked up this schema due to a compat hack,
+        // but this hack is now removed. So this schema is unused. It is kept
+        // here for historical accuracy.
         name: 'RefreshDriverTick',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'name', label: 'Tick Reasons', format: 'string' }],

src/profile-logic/marker-data.js

@@ -14,7 +14,6 @@ import {
   INTERVAL_END,
 } from 'firefox-profiler/app-logic/constants';
 import {
-  getMarkerSchemaName,
   getSchemaFromMarker,
   markerPayloadMatchesSearch,
 } from './marker-schema';
@@ -226,11 +225,7 @@ function positiveFilterMarker(
     }
 
     // Now check the schema for the marker payload for searchable
-    const markerSchema = getSchemaFromMarker(
-      markerSchemaByName,
-      marker.name,
-      marker.data
-    );
+    const markerSchema = getSchemaFromMarker(markerSchemaByName, marker.data);
     if (
       markerSchema &&
       markerPayloadMatchesSearch(markerSchema, marker, stringTable, test)
@@ -292,11 +287,7 @@ function negativeFilterMarker(
     }
 
     // Now check the schema for the marker payload for searchable
-    const markerSchema = getSchemaFromMarker(
-      markerSchemaByName,
-      marker.name,
-      marker.data
-    );
+    const markerSchema = getSchemaFromMarker(markerSchemaByName, marker.data);
 
     if (
       markerSchema &&
@@ -1287,14 +1278,12 @@ export function getAllowMarkersWithNoSchema(
     const { data } = marker;
 
     if (!data) {
-      // Keep the marker if there is payload.
+      // Keep the marker if there is no payload.
       return true;
     }
 
     if (!markerSchemaByName[data.type]) {
-      // Keep the marker if there is no schema. Note that this function doesn't use
-      // the getMarkerSchemaName function, as that function attempts to find a
-      // marker schema name in a very permissive manner. In the marker chart
+      // Keep the marker if there is no schema. In the marker chart
       // and marker table, most likely we want to show everything.
       return true;
     }
@@ -1528,9 +1517,8 @@ export function filterMarkerByDisplayLocation(
       return additionalResult;
     }
 
-    return markerTypes.has(
-      getMarkerSchemaName(markerSchemaByName, marker.name, marker.data)
-    );
+    const schemaName = marker.data ? (marker.data.type ?? null) : null;
+    return schemaName !== null && markerTypes.has(schemaName);
   });
 }
 

src/profile-logic/marker-schema.js

@@ -82,56 +82,16 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [
   },
 ];
 
-/**
- * For the most part, schema is matched up by the Payload's "type" field,
- * but for practical purposes, there are a few other options, see the
- * implementation of this function for details.
- */
-export function getMarkerSchemaName(
-  markerSchemaByName: MarkerSchemaByName,
-  markerName: string,
-  markerData: MarkerPayload | null
-): string {
-  if (!markerData) {
-    // Fall back to using the name if no payload exists.
-    return markerName;
-  }
-
-  const { type } = markerData;
-  if (type === 'tracing' && markerData.category) {
-    // TODO - Tracing markers have a duplicate "category" field.
-    // See issue #2749
-
-    // Does a marker schema for the "category" exist?
-    return markerSchemaByName[markerData.category] === undefined
-      ? // If not, default back to tracing
-        'tracing'
-      : // If so, use the category as the schema name.
-        markerData.category;
-  }
-  if (type === 'Text') {
-    // Text markers are a cheap and easy way to create markers with
-    // a category. Check for schema if it exists, if not, fallback to
-    // a Text type marker.
-    return markerSchemaByName[markerName] === undefined ? 'Text' : markerName;
-  }
-  return type;
-}
-
 /**
  * This function takes the intended marker schema for a marker field, and applies
  * the appropriate formatting function.
  */
 export function getSchemaFromMarker(
   markerSchemaByName: MarkerSchemaByName,
-  markerName: string,
   markerData: MarkerPayload | null
 ): MarkerSchema | null {
-  return (
-    markerSchemaByName[
-      getMarkerSchemaName(markerSchemaByName, markerName, markerData)
-    ] || null
-  );
+  const schemaName = markerData ? markerData.type : null;
+  return schemaName ? (markerSchemaByName[schemaName] ?? null) : null;
 }
 
 /**
@@ -385,12 +345,8 @@ export function getLabelGetter(
     // No label exists, it will have to be generated for the first time.
     if (label === undefined) {
       const marker = getMarker(markerIndex);
-      const schemaName = getMarkerSchemaName(
-        markerSchemaByName,
-        marker.name,
-        marker.data
-      );
-      const applyLabel = labelFns.get(schemaName);
+      const schemaName = marker.data ? marker.data.type : null;
+      const applyLabel = schemaName ? labelFns.get(schemaName) : null;
 
       label = applyLabel
         ? // A label function is available, apply it.

src/profile-logic/processed-profile-versioning.js

@@ -1651,27 +1651,26 @@
           // eventType is in the payload as well.
         ],
       },
+
+      // The following three schemas should have just been a single schema named
+      // "tracing". They are kept here for historical accuracy.
+      // The upgrader for version 52 adds the missing "tracing" schema.
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Paint',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Navigation',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
       {
-        // TODO - Note that this marker is a "tracing" marker currently.
-        // See issue #2749
         name: 'Layout',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'category', label: 'Type', format: 'string' }],
       },
+
       {
         name: 'IPC',
         tooltipLabel: 'IPC — {marker.data.niceDirection}',
@@ -1687,6 +1686,14 @@
         ],
       },
       {
+        // An unused schema for RefreshDriverTick markers.
+        // This schema is not consistent with what post-schema Firefox would
+        // output. Firefox (as of Jan 2025) is still using Text markers and does
+        // not have a RefreshDriverTick schema. Furthermore, upgraded profiles
+        // which get this schema do not have any { type: 'RefreshDriverTick' }
+        // markers - in the past they picked up this schema due to a compat hack,
+        // but this hack is now removed. So this schema is unused. It is kept
+        // here for historical accuracy.
         name: 'RefreshDriverTick',
         display: ['marker-chart', 'marker-table', 'timeline-overview'],
         data: [{ key: 'name', label: 'Tick Reasons', format: 'string' }],
@@ -2284,6 +2291,77 @@
     // marker data with the new field types data, and no modification is needed in the
     // frontend to display older formats.
   },
+  [52]: (profile) => {
+    // This version simplifies how markers are mapped to their schema.
+    // The schema is now purely determined by data.type. The marker's name is ignored.
+    // If a marker has a null data, then it has no schema.
+    //
+    // In earlier versions, there were special cases for mapping markers with type
+    // "tracing" and "Text", and for markers with a null data property. These
+    // special cases have been removed.
+    //
+    // The upgrader for version 52 makes it so that existing profiles appear the
+    // same with the simplified logic. Specifically:
+    //  - Some old profiles have markers with data.type === 'tracing' but no schema
+    //    with the name 'tracing'. To ensure that the tracing markers from these
+    //    profile still show up in the 'timeline-overview' area, this upgrader adds
+    //    a schema to such profiles.
+    //  - Some old profiles have CC markers which only showed up in the memory track
+    //    because of special treatment of 'tracing' markers - the markers would have
+    //    data.type === 'tracing' and data.category === 'CC', and there would be a
+    //    'CC' schema with 'timeline-memory'. This upgrader moves these tracing CC
+    //    markers to a new 'tracingCCFrom52Upgrader' schema.
+    //
+    // Profiles from modern versions of Firefox already include a 'tracing' schema.
+    // And they don't use tracing markers for CC markers.
+
+    const schemaNames = new Set(profile.meta.markerSchema.map((s) => s.name));
+    const kTracingCCSchemaName = 'tracingCCFrom52Upgrader';
+    const shouldMigrateTracingCCMarkers =
+      schemaNames.has('CC') && !schemaNames.has(kTracingCCSchemaName);
+    let hasTracingMarkers = false;
+    let hasMigratedTracingCCMarkers = false;
+    for (const thread of profile.threads) {
+      const { markers } = thread;
+      for (let i = 0; i < markers.length; i++) {
+        const data = markers.data[i];
+        if (data && data.type === 'tracing' && data.category) {
+          hasTracingMarkers = true;
+          if (shouldMigrateTracingCCMarkers && data.category === 'CC') {
+            data.type = kTracingCCSchemaName;
+            hasMigratedTracingCCMarkers = true;
+            if (data.interval) {
+              // Also delete the interval property. This is present on old
+              // profiles where marker phase information was represented in the
+              // payload, i.e. you'd have interval: "start" / "end" on the
+              // data object.
+              // Our kTracingCCSchemaName schema does not list the interval field,
+              // so we shouldn't have this field on the payload either.
+              delete data.interval;
+            }
+          }
+        }
+      }
+    }
+    if (hasTracingMarkers && !schemaNames.has('tracing')) {
+      // Make sure that tracing markers still show up in the timeline-overview area.
+      profile.meta.markerSchema.push({
+        name: 'tracing',
+        display: ['marker-chart', 'marker-table', 'timeline-overview'],
+        data: [{ key: 'category', label: 'Type', format: 'string' }],
+      });
+    }
+
+    if (hasMigratedTracingCCMarkers) {
+      // Add the kTracingCCSchemaName schema for migrated tracing CC markers, to
+      // make sure that these markers still show up in the timeline-memory area.
+      profile.meta.markerSchema.push({
+        name: kTracingCCSchemaName,
+        display: ['marker-chart', 'marker-table', 'timeline-memory'],
+        data: [{ key: 'category', label: 'Type', format: 'string' }],
+      });
+    }
+  },
   // If you add a new upgrader here, please document the change in
   // `docs-developer/CHANGELOG-formats.md`.
 };

src/profile-logic/sanitize.js

@@ -297,11 +297,8 @@ function sanitizeThreadPII(
 
       if (currentMarker && PIIToBeRemoved.shouldRemoveUrls) {
         // Use the schema to find some properties that need to be sanitized.
-        const markerNameIndex = markerTable.name[i];
-        const markerName = thread.stringTable.getString(markerNameIndex);
         const markerSchema = getSchemaFromMarker(
           markerSchemaByName,
-          markerName,
           currentMarker
         );
         if (markerSchema) {