Implement searchable property for marker schema

src/app-logic/constants.js

@@ -7,10 +7,10 @@
 import type { MarkerPhase } from 'firefox-profiler/types';
 
 // The current version of the Gecko profile format.
-export const GECKO_PROFILE_VERSION = 25;
+export const GECKO_PROFILE_VERSION = 26;
 
 // The current version of the "processed" profile format.
-export const PROCESSED_PROFILE_VERSION = 43;
+export const PROCESSED_PROFILE_VERSION = 44;
 
 // The following are the margin sizes for the left and right of the timeline. Independent
 // components need to share these values.

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

@@ -1365,5 +1365,69 @@ const _upgraders = {
     // capturing this data and no new mandatory values are present in this
     // version.
   },
+  [26]: (profile) => {
+    // `searchable` property in the marker schema wasn't implemented before and
+    // we had some manual checks for the marker fields below. With this version,
+    // we removed this manual check and started to use the `searchable` property
+    // of the marker schema.
+    function convertToVersion26Recursive(p) {
+      for (const schema of p.meta.markerSchema) {
+        let searchableFieldKeys;
+        switch (schema.name) {
+          case 'FileIO': {
+            // threadId wasn't in the schema before, so we need to add manually.
+            schema.data.push({
+              key: 'threadId',
+              label: 'Thread ID',
+              format: 'string',
+              searchable: true,
+            });
+            searchableFieldKeys = [];
+            break;
+          }
+          case 'Log': {
+            searchableFieldKeys = ['name', 'module'];
+            break;
+          }
+          case 'DOMEvent': {
+            // In the earlier versions of Firefox, DOMEvent doesn't include
+            // eventType in the backend.
+            schema.data.push({
+              key: 'eventType',
+              label: 'Event Type',
+              format: 'string',
+              searchable: true,
+            });
+            // 'target' wasn't included in our code before. But I thought this
+            // would be a useful addition.
+            searchableFieldKeys = ['target'];
+            break;
+          }
+          case 'TraceEvent': {
+            // These weren't included in our code before. But I thought this
+            // would be a useful addition.
+            searchableFieldKeys = ['name1', 'name2', 'val1', 'val2'];
+            break;
+          }
+          default: {
+            searchableFieldKeys = ['name', 'category'];
+            break;
+          }
+        }
+
+        for (const field of schema.data) {
+          if (searchableFieldKeys.includes(field.key)) {
+            field.searchable = true;
+          }
+        }
+      }
+
+      for (const subprocessProfile of p.processes) {
+        convertToVersion26Recursive(subprocessProfile);
+      }
+    }
+
+    convertToVersion26Recursive(profile);
+  },
 };
 /* eslint-enable no-useless-computed-key */

src/profile-logic/marker-data.js

@@ -13,7 +13,11 @@ import {
   INTERVAL_START,
   INTERVAL_END,
 } from 'firefox-profiler/app-logic/constants';
-import { getMarkerSchemaName } from './marker-schema';
+import {
+  getMarkerSchemaName,
+  getSchemaFromMarker,
+  markerPayloadMatchesSearch,
+} from './marker-schema';
 
 import type {
   Thread,
@@ -115,85 +119,52 @@ export function deriveJankMarkers(
 export function getSearchFilteredMarkerIndexes(
   getMarker: (MarkerIndex) => Marker,
   markerIndexes: MarkerIndex[],
+  markerSchemaByName: MarkerSchemaByName,
   searchRegExp: RegExp | null,
   categoryList: CategoryList
 ): MarkerIndex[] {
   if (!searchRegExp) {
     return markerIndexes;
   }
+  // Need to assign it to a constant variable so Flow doesn't complain about
+  // passing it inside a function below.
+  const regExp = searchRegExp;
   const newMarkers: MarkerIndex[] = [];
   for (const markerIndex of markerIndexes) {
-    const { data, name, category } = getMarker(markerIndex);
+    const marker = getMarker(markerIndex);
+    const { data, name, category } = marker;
 
     // Reset regexp for each iteration. Otherwise state from previous
     // iterations can cause matches to fail if the search is global or
     // sticky.
 
-    searchRegExp.lastIndex = 0;
+    regExp.lastIndex = 0;
 
     if (categoryList[category] !== undefined) {
       const markerCategory = categoryList[category].name;
-      if (searchRegExp.test(markerCategory)) {
+      if (regExp.test(markerCategory)) {
         newMarkers.push(markerIndex);
         continue;
       }
     }
 
-    if (searchRegExp.test(name)) {
+    if (regExp.test(name)) {
       newMarkers.push(markerIndex);
       continue;
     }
+
     if (data && typeof data === 'object') {
-      if (searchRegExp.test(data.type)) {
+      if (regExp.test(data.type)) {
+        // Check the type of the marker payload first.
         newMarkers.push(markerIndex);
         continue;
       }
 
-      if (data.type === 'FileIO') {
-        const { filename, operation, source, threadId } = data;
-        if (
-          searchRegExp.test(filename || '') ||
-          searchRegExp.test(operation) ||
-          searchRegExp.test(source) ||
-          (threadId !== undefined && searchRegExp.test(threadId.toString()))
-        ) {
-          newMarkers.push(markerIndex);
-          continue;
-        }
-      } else if (data.type === 'IPC') {
-        const { messageType, otherPid } = data;
-        if (
-          searchRegExp.test(messageType) ||
-          searchRegExp.test(otherPid.toString())
-        ) {
-          newMarkers.push(markerIndex);
-          continue;
-        }
-      } else if (data.type === 'Log') {
-        const { name, module } = data;
-
-        if (searchRegExp.test(name) || searchRegExp.test(module)) {
-          newMarkers.push(markerIndex);
-          continue;
-        }
-      }
-
-      if (
-        typeof data.eventType === 'string' &&
-        searchRegExp.test(data.eventType)
-      ) {
-        // Match DOMevents data.eventType
-        newMarkers.push(markerIndex);
-        continue;
-      }
-      if (typeof data.name === 'string' && searchRegExp.test(data.name)) {
-        // Match UserTiming's name.
-        newMarkers.push(markerIndex);
-        continue;
-      }
+      // Now check the schema for the marker payload for searchable
+      const markerSchema = getSchemaFromMarker(markerSchemaByName, marker);
       if (
-        typeof data.category === 'string' &&
-        searchRegExp.test(data.category)
+        markerSchema &&
+        markerPayloadMatchesSearch(markerSchema, marker, regExp)
       ) {
         newMarkers.push(markerIndex);
         continue;

src/profile-logic/marker-schema.js

@@ -56,10 +56,16 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [
     chartLabel: '{marker.data.messageType}',
     display: ['marker-chart', 'marker-table', 'timeline-ipc'],
     data: [
-      { key: 'messageType', label: 'Type', format: 'string' },
+      { key: 'messageType', label: 'Type', format: 'string', searchable: true },
       { key: 'sync', label: 'Sync', format: 'string' },
       { key: 'sendThreadName', label: 'From', format: 'string' },
       { key: 'recvThreadName', label: 'To', format: 'string' },
+      {
+        key: 'otherPid',
+        label: 'Other Pid',
+        format: 'string',
+        searchable: true,
+      },
     ],
   },
   {
@@ -564,3 +570,39 @@ export function formatMarkupFromMarkerSchema(
       throw new Error(`Unknown format type ${JSON.stringify((format: empty))}`);
   }
 }
+
+/**
+ * Takes a marker and a RegExp and checks if any of its `searchable` marker
+ * payload fields match the search regular expression.
+ */
+export function markerPayloadMatchesSearch(
+  markerSchema: MarkerSchema,
+  marker: Marker,
+  searchRegExp: RegExp
+): boolean {
+  const { data } = marker;
+  if (!data) {
+    return false;
+  }
+
+  // Reset regexp for each marker. Otherwise state from previous
+  // usages can cause matches to fail if the search is global or
+  // sticky.
+  searchRegExp.lastIndex = 0;
+
+  // Check if searchable fields match the search regular expression.
+  for (const payloadField of markerSchema.data) {
+    if (payloadField.searchable) {
+      const value = data[payloadField.key];
+      if (value === undefined || value === null || value === '') {
+        continue;
+      }
+
+      if (searchRegExp.test(value)) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}

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

@@ -2141,5 +2141,61 @@ const _upgraders = {
   [43]: (_) => {
     // The number property in counters is now optional.
   },
+  [44]: (profile) => {
+    // `searchable` property in the marker schema wasn't implemented before and
+    // we had some manual checks for the marker fields below. With this version,
+    // we removed this manual check and started to use the `searchable` property
+    // of the marker schema.
+    for (const schema of profile.meta.markerSchema) {
+      let searchableFieldKeys;
+      switch (schema.name) {
+        case 'FileIO': {
+          // threadId wasn't in the schema before, so we need to add manually.
+          schema.data.push({
+            key: 'threadId',
+            label: 'Thread ID',
+            format: 'string',
+            searchable: true,
+          });
+          searchableFieldKeys = [];
+          break;
+        }
+        case 'Log': {
+          searchableFieldKeys = ['name', 'module'];
+          break;
+        }
+        case 'DOMEvent': {
+          // In the earlier versions of Firefox, DOMEvent doesn't include
+          // eventType in the backend.
+          schema.data.push({
+            key: 'eventType',
+            label: 'Event Type',
+            format: 'string',
+            searchable: true,
+          });
+          // 'target' wasn't included in our code before. But I thought this
+          // would be a useful addition.
+          searchableFieldKeys = ['target'];
+          break;
+        }
+        case 'TraceEvent': {
+          // These weren't included in our code before. But I thought this
+          // would be a useful addition.
+          searchableFieldKeys = ['name1', 'name2', 'val1', 'val2'];
+          break;
+        }
+        default: {
+          searchableFieldKeys = ['name', 'category'];
+          break;
+        }
+      }
+
+      for (const field of schema.data) {
+        if (searchableFieldKeys.includes(field.key)) {
+          field.searchable = true;
+        }
+      }
+    }
+  },
 };
 /* eslint-enable no-useless-computed-key */

src/selectors/per-thread/markers.js

@@ -286,6 +286,7 @@ export function getMarkerSelectorsPerThread(
     createSelector(
       getMarkerGetter,
       getCommittedRangeAndTabFilteredMarkerIndexes,
+      ProfileSelectors.getMarkerSchemaByName,
       UrlState.getMarkersSearchStringsAsRegExp,
       ProfileSelectors.getCategories,
       MarkerData.getSearchFilteredMarkerIndexes
@@ -343,6 +344,7 @@ export function getMarkerSelectorsPerThread(
     createSelector(
       getMarkerGetter,
       getNetworkMarkerIndexes,
+      ProfileSelectors.getMarkerSchemaByName,
       UrlState.getNetworkSearchStringsAsRegExp,
       ProfileSelectors.getCategories,
       MarkerData.getSearchFilteredMarkerIndexes

src/test/components/__snapshots__/MarkerChart.test.js.snap

@@ -300,6 +300,13 @@ exports[`MarkerChart renders the hoveredItem markers properly 2`] = `
     <div
       class="tooltipDetails"
     >
+      <div
+        class="tooltipLabel"
+      >
+        Name
+        :
+      </div>
+      Marker B
       <div
         class="tooltipLabel"
       >
@@ -1238,6 +1245,13 @@ exports[`MarkerChart with active tab renders the hovered marker properly 2`] = `
         :
       </div>
       7ms
+      <div
+        class="tooltipLabel"
+      >
+        Event Type
+        :
+      </div>
+      click
       <div
         class="tooltipLabel"
       >

src/test/components/__snapshots__/MarkerSidebar.test.js.snap

@@ -52,6 +52,13 @@ exports[`MarkerSidebar matches the snapshots when displaying data about the curr
           :
         </div>
         Empty (Thread ID: 0)
+        <div
+          class="tooltipLabel"
+        >
+          Other Pid
+          :
+        </div>
+        2222
         <div
           class="tooltipLabel"
         >

src/test/components/__snapshots__/StackChart.test.js.snap

@@ -889,6 +889,13 @@ exports[`MarkerChart shows a tooltip when hovering 1`] = `
     <div
       class="tooltipDetails"
     >
+      <div
+        class="tooltipLabel"
+      >
+        Name
+        :
+      </div>
+      componentB
       <div
         class="tooltipLabel"
       >