Use proper enums to communicate between JS and C++ in NativePerforman…

…ceObserver Summary: [Changelog][Internal] After D42884147 (facebook@ceb1d0d) (facebook#36030) had landed, we now can use enums as data types when bridging between JS and C++ TurboModules. This diff takes advantage of this and converts the API to use the proper enums instead of int types that it was forced to use earlier. NOTE: This also relies on additional change to codegen, D43292254, which is required in order for enums to work also as part of data structures. Differential Revision: D43292282 fbshipit-source-id: fd271704ab60aec2b3ae5b260e76665ad50e206e
rshest · Feb 14, 2023 · 2b95380 · 2b95380
1 parent 3524edd
commit 2b95380
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 60 deletions.
diff --git a/Libraries/WebPerformance/NativePerformanceObserver.cpp b/Libraries/WebPerformance/NativePerformanceObserver.cpp
@@ -22,18 +22,26 @@ NativePerformanceObserver::~NativePerformanceObserver() {
 
 void NativePerformanceObserver::startReporting(
     jsi::Runtime &rt,
-    int32_t entryType) {
+    PerformanceEntryType entryType) {
   PerformanceEntryReporter::getInstance().startReporting(
       static_cast<PerformanceEntryType>(entryType));
 }
 
 void NativePerformanceObserver::stopReporting(
     jsi::Runtime &rt,
-    int32_t entryType) {
+    PerformanceEntryType entryType) {
   PerformanceEntryReporter::getInstance().stopReporting(
       static_cast<PerformanceEntryType>(entryType));
 }
 
+void NativePerformanceObserver::setDurationThreshold(
+    jsi::Runtime &rt,
+    PerformanceEntryType entryType,
+    double durationThreshold) {
+  PerformanceEntryReporter::getInstance().setDurationThreshold(
+      static_cast<PerformanceEntryType>(entryType), durationThreshold);
+}
+
 GetPendingEntriesResult NativePerformanceObserver::popPendingEntries(
     jsi::Runtime &rt) {
   return PerformanceEntryReporter::getInstance().popPendingEntries();
@@ -51,14 +59,6 @@ void NativePerformanceObserver::logRawEntry(
   PerformanceEntryReporter::getInstance().logEntry(entry);
 }
 
-void NativePerformanceObserver::setDurationThreshold(
-    jsi::Runtime &rt,
-    int32_t entryType,
-    double durationThreshold) {
-  PerformanceEntryReporter::getInstance().setDurationThreshold(
-      static_cast<PerformanceEntryType>(entryType), durationThreshold);
-}
-
 std::vector<std::pair<std::string, uint32_t>>
 NativePerformanceObserver::getEventCounts(jsi::Runtime &rt) {
   const auto &eventCounts =

diff --git a/Libraries/WebPerformance/NativePerformanceObserver.h b/Libraries/WebPerformance/NativePerformanceObserver.h
@@ -18,9 +18,12 @@ class PerformanceEntryReporter;
 
 #pragma mark - Structs
 
+using PerformanceEntryType =
+    NativePerformanceObserverCxxRawPerformanceEntryType;
+
 using RawPerformanceEntry = NativePerformanceObserverCxxBaseRawPerformanceEntry<
     std::string,
-    int32_t,
+    PerformanceEntryType,
     double,
     double,
     // For "event" entries only:
@@ -32,7 +35,7 @@ template <>
 struct Bridging<RawPerformanceEntry>
     : NativePerformanceObserverCxxBaseRawPerformanceEntryBridging<
           std::string,
-          int32_t,
+          PerformanceEntryType,
           double,
           double,
           std::optional<double>,
@@ -59,9 +62,9 @@ class NativePerformanceObserver
   NativePerformanceObserver(std::shared_ptr<CallInvoker> jsInvoker);
   ~NativePerformanceObserver();
 
-  void startReporting(jsi::Runtime &rt, int32_t entryType);
+  void startReporting(jsi::Runtime &rt, PerformanceEntryType entryType);
 
-  void stopReporting(jsi::Runtime &rt, int32_t entryType);
+  void stopReporting(jsi::Runtime &rt, PerformanceEntryType entryType);
 
   GetPendingEntriesResult popPendingEntries(jsi::Runtime &rt);
 
@@ -73,7 +76,7 @@ class NativePerformanceObserver
 
   void setDurationThreshold(
       jsi::Runtime &rt,
-      int32_t entryType,
+      PerformanceEntryType entryType,
       double durationThreshold);
 
   std::vector<std::pair<std::string, uint32_t>> getEventCounts(

diff --git a/Libraries/WebPerformance/NativePerformanceObserver.js b/Libraries/WebPerformance/NativePerformanceObserver.js
@@ -12,7 +12,13 @@ import type {TurboModule} from '../TurboModule/RCTExport';
 
 import * as TurboModuleRegistry from '../TurboModule/TurboModuleRegistry';
 
-export type RawPerformanceEntryType = number;
+export enum RawPerformanceEntryType {
+  UNDEFINED = 0,
+  MARK = 1,
+  MEASURE = 2,
+  EVENT = 3,
+  _COUNT = 4,
+}
 
 export type RawPerformanceEntry = {|
   name: string,

diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.cpp b/Libraries/WebPerformance/PerformanceEntryReporter.cpp
@@ -31,19 +31,19 @@ void PerformanceEntryReporter::setReportingCallback(
 }
 
 void PerformanceEntryReporter::startReporting(PerformanceEntryType entryType) {
-  int entryTypeIdx = static_cast<int>(entryType);
+  size_t entryTypeIdx = static_cast<size_t>(entryType);
   reportingType_[entryTypeIdx] = true;
   durationThreshold_[entryTypeIdx] = DEFAULT_DURATION_THRESHOLD;
 }
 
 void PerformanceEntryReporter::setDurationThreshold(
     PerformanceEntryType entryType,
     double durationThreshold) {
-  durationThreshold_[static_cast<int>(entryType)] = durationThreshold;
+  durationThreshold_[static_cast<size_t>(entryType)] = durationThreshold;
 }
 
 void PerformanceEntryReporter::stopReporting(PerformanceEntryType entryType) {
-  reportingType_[static_cast<int>(entryType)] = false;
+  reportingType_[static_cast<size_t>(entryType)] = false;
 }
 
 GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() {
@@ -56,12 +56,11 @@ GetPendingEntriesResult PerformanceEntryReporter::popPendingEntries() {
 }
 
 void PerformanceEntryReporter::logEntry(const RawPerformanceEntry &entry) {
-  const auto entryType = static_cast<PerformanceEntryType>(entry.entryType);
-  if (entryType == PerformanceEntryType::EVENT) {
+  if (entry.entryType == PerformanceEntryType::EVENT) {
     eventCounts_[entry.name]++;
   }
 
-  if (!isReportingType(entryType)) {
+  if (!isReportingType(entry.entryType)) {
     return;
   }
 
@@ -113,7 +112,7 @@ void PerformanceEntryReporter::mark(
 
   logEntry(
       {name,
-       static_cast<int>(PerformanceEntryType::MARK),
+       PerformanceEntryType::MARK,
        startTime,
        duration,
        std::nullopt,
@@ -127,13 +126,13 @@ void PerformanceEntryReporter::clearMarks(
     PerformanceMark mark{{*markName, 0}};
     marksRegistry_.erase(&mark);
     clearEntries([&markName](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MARK) &&
+      return entry.entryType == PerformanceEntryType::MARK &&
           entry.name == markName;
     });
   } else {
     marksRegistry_.clear();
     clearEntries([](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MARK);
+      return entry.entryType == PerformanceEntryType::MARK;
     });
   }
 }
@@ -150,7 +149,7 @@ void PerformanceEntryReporter::measure(
   double durationVal = duration ? *duration : endTimeVal - startTimeVal;
   logEntry(
       {name,
-       static_cast<int>(PerformanceEntryType::MEASURE),
+       PerformanceEntryType::MEASURE,
        startTimeVal,
        durationVal,
        std::nullopt,
@@ -162,14 +161,13 @@ void PerformanceEntryReporter::clearMeasures(
     const std::optional<std::string> &measureName) {
   if (measureName) {
     clearEntries([&measureName](const RawPerformanceEntry &entry) {
-      return entry.entryType ==
-          static_cast<int>(PerformanceEntryType::MEASURE) &&
+      return entry.entryType == PerformanceEntryType::MEASURE &&
           entry.name == measureName;
     });
   } else {
     marksRegistry_.clear();
     clearEntries([](const RawPerformanceEntry &entry) {
-      return entry.entryType == static_cast<int>(PerformanceEntryType::MEASURE);
+      return entry.entryType == PerformanceEntryType::MEASURE;
     });
   }
 }
@@ -194,7 +192,7 @@ void PerformanceEntryReporter::event(
     uint32_t interactionId) {
   logEntry(
       {std::move(name),
-       static_cast<int>(PerformanceEntryType::EVENT),
+       PerformanceEntryType::EVENT,
        startTime,
        duration,
        processingStart,

diff --git a/Libraries/WebPerformance/PerformanceEntryReporter.h b/Libraries/WebPerformance/PerformanceEntryReporter.h
@@ -46,14 +46,6 @@ constexpr size_t MARKS_BUFFER_SIZE = 1024;
 
 constexpr double DEFAULT_DURATION_THRESHOLD = 0.0;
 
-enum class PerformanceEntryType {
-  UNDEFINED = 0,
-  MARK = 1,
-  MEASURE = 2,
-  EVENT = 3,
-  _COUNT = 4,
-};
-
 class PerformanceEntryReporter : public EventLogger {
  public:
   PerformanceEntryReporter(PerformanceEntryReporter const &) = delete;

diff --git a/Libraries/WebPerformance/RawPerformanceEntry.js b/Libraries/WebPerformance/RawPerformanceEntry.js
@@ -8,26 +8,17 @@
  * @flow strict
  */
 
-import type {
-  RawPerformanceEntry,
-  RawPerformanceEntryType,
-} from './NativePerformanceObserver';
+import type {RawPerformanceEntry} from './NativePerformanceObserver';
 import type {PerformanceEntryType} from './PerformanceEntry';
 
+import {RawPerformanceEntryType} from './NativePerformanceObserver';
 import {PerformanceEntry} from './PerformanceEntry';
 import {PerformanceEventTiming} from './PerformanceEventTiming';
 
-export const RawPerformanceEntryTypeValues = {
-  UNDEFINED: 0,
-  MARK: 1,
-  MEASURE: 2,
-  EVENT: 3,
-};
-
 export function rawToPerformanceEntry(
   entry: RawPerformanceEntry,
 ): PerformanceEntry {
-  if (entry.entryType === RawPerformanceEntryTypeValues.EVENT) {
+  if (entry.entryType === RawPerformanceEntryType.EVENT) {
     return new PerformanceEventTiming({
       name: entry.name,
       startTime: entry.startTime,
@@ -50,15 +41,15 @@ export function rawToPerformanceEntryType(
   type: RawPerformanceEntryType,
 ): PerformanceEntryType {
   switch (type) {
-    case RawPerformanceEntryTypeValues.MARK:
+    case RawPerformanceEntryType.MARK:
       return 'mark';
-    case RawPerformanceEntryTypeValues.MEASURE:
+    case RawPerformanceEntryType.MEASURE:
       return 'measure';
-    case RawPerformanceEntryTypeValues.EVENT:
+    case RawPerformanceEntryType.EVENT:
       return 'event';
     default:
       throw new TypeError(
-        `rawToPerformanceEntryType: unexpected performance entry type received: ${type}`,
+        'rawToPerformanceEntryType: unexpected RawPerformanceEntryType received',
       );
   }
 }
@@ -68,11 +59,11 @@ export function performanceEntryTypeToRaw(
 ): RawPerformanceEntryType {
   switch (type) {
     case 'mark':
-      return RawPerformanceEntryTypeValues.MARK;
+      return RawPerformanceEntryType.MARK;
     case 'measure':
-      return RawPerformanceEntryTypeValues.MEASURE;
+      return RawPerformanceEntryType.MEASURE;
     case 'event':
-      return RawPerformanceEntryTypeValues.EVENT;
+      return RawPerformanceEntryType.EVENT;
     default:
       // Verify exhaustive check with Flow
       (type: empty);

diff --git a/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js b/Libraries/WebPerformance/__mocks__/NativePerformanceObserver.js
@@ -11,11 +11,10 @@
 import type {
   GetPendingEntriesResult,
   RawPerformanceEntry,
-  RawPerformanceEntryType,
   Spec as NativePerformanceObserver,
 } from '../NativePerformanceObserver';
 
-import {RawPerformanceEntryTypeValues} from '../RawPerformanceEntry';
+import {RawPerformanceEntryType} from '../NativePerformanceObserver';
 
 const reportingType: Set<RawPerformanceEntryType> = new Set();
 const durationThresholds: Map<RawPerformanceEntryType, number> = new Map();
@@ -58,7 +57,7 @@ export const NativePerformanceObserverMock: NativePerformanceObserver = {
       entries.push(entry);
       onPerformanceEntryCallback?.();
     }
-    if (entry.entryType === RawPerformanceEntryTypeValues.EVENT) {
+    if (entry.entryType === RawPerformanceEntryType.EVENT) {
       eventCounts.set(entry.name, (eventCounts.get(entry.name) ?? 0) + 1);
     }
   },