Update InspectorFlags to lazily pull upstream values (#43390)

Summary:
Pull Request resolved: #43390

Refactor after D54639775. This avoids the unfortunate side effect where `InspectorFlags::dangerouslyResetFlags()` would immediately reread `ReactNativeFeatureFlags `. This call is now relocated in our test util.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D54684692

fbshipit-source-id: 962c7d78bbf71b1d81af412081d3ef5cfe443fa1

Author: huntie

packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp

@@ -17,55 +17,51 @@ InspectorFlags& InspectorFlags::getInstance() {
   return instance;
 }
 
-InspectorFlags::InspectorFlags()
-    : enableModernCDPRegistry_(
-          ReactNativeFeatureFlags::inspectorEnableModernCDPRegistry()),
-      enableCxxInspectorPackagerConnection_(
-          ReactNativeFeatureFlags::
-              inspectorEnableCxxInspectorPackagerConnection()),
-      enableHermesCDPAgent_(
-          ReactNativeFeatureFlags::inspectorEnableHermesCDPAgent()) {}
-
 bool InspectorFlags::getEnableModernCDPRegistry() const {
-  assertFlagsMatchUpstream();
-  return enableModernCDPRegistry_;
+  return loadFlagsAndAssertUnchanged().enableModernCDPRegistry;
 }
 
 bool InspectorFlags::getEnableCxxInspectorPackagerConnection() const {
-  assertFlagsMatchUpstream();
-  return enableCxxInspectorPackagerConnection_ ||
+  auto& values = loadFlagsAndAssertUnchanged();
+
+  return values.enableCxxInspectorPackagerConnection ||
       // If we are using the modern CDP registry, then we must also use the C++
       // InspectorPackagerConnection implementation.
-      enableModernCDPRegistry_;
+      values.enableModernCDPRegistry;
 }
 
 bool InspectorFlags::getEnableHermesCDPAgent() const {
-  assertFlagsMatchUpstream();
-  return enableHermesCDPAgent_;
+  return loadFlagsAndAssertUnchanged().enableHermesCDPAgent;
 }
 
 void InspectorFlags::dangerouslyResetFlags() {
   *this = InspectorFlags{};
 }
 
-void InspectorFlags::assertFlagsMatchUpstream() const {
-  if (inconsistentFlagsStateLogged_) {
-    return;
-  }
+const InspectorFlags::Values& InspectorFlags::loadFlagsAndAssertUnchanged()
+    const {
+  InspectorFlags::Values newValues = {
+      .enableCxxInspectorPackagerConnection = ReactNativeFeatureFlags::
+          inspectorEnableCxxInspectorPackagerConnection(),
+      .enableHermesCDPAgent =
+          ReactNativeFeatureFlags::inspectorEnableHermesCDPAgent(),
+      .enableModernCDPRegistry =
+          ReactNativeFeatureFlags::inspectorEnableModernCDPRegistry(),
+  };
 
-  if (enableModernCDPRegistry_ !=
-          ReactNativeFeatureFlags::inspectorEnableModernCDPRegistry() ||
-      enableCxxInspectorPackagerConnection_ !=
-          ReactNativeFeatureFlags::
-              inspectorEnableCxxInspectorPackagerConnection() ||
-      ReactNativeFeatureFlags::inspectorEnableHermesCDPAgent() !=
-          enableHermesCDPAgent_) {
-    LOG(ERROR)
-        << "[InspectorFlags] Error: One or more ReactNativeFeatureFlags values "
-        << "have changed during the global app lifetime. This may lead to "
-        << "inconsistent inspector behaviour. Please quit and restart the app.";
-    inconsistentFlagsStateLogged_ = true;
+  if (cachedValues_.has_value() && !inconsistentFlagsStateLogged_) {
+    if (cachedValues_ != newValues) {
+      LOG(ERROR)
+          << "[InspectorFlags] Error: One or more ReactNativeFeatureFlags values "
+          << "have changed during the global app lifetime. This may lead to "
+          << "inconsistent inspector behaviour. Please quit and restart the app.";
+      inconsistentFlagsStateLogged_ = true;
+    }
   }
+
+  cachedValues_ = newValues;
+
+  return cachedValues_.value();
 }
 
 } // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h

@@ -42,17 +42,22 @@ class InspectorFlags {
   void dangerouslyResetFlags();
 
  private:
-  InspectorFlags();
+  struct Values {
+    bool enableCxxInspectorPackagerConnection;
+    bool enableHermesCDPAgent;
+    bool enableModernCDPRegistry;
+    bool operator==(const Values&) const = default;
+  };
+
+  InspectorFlags() = default;
   InspectorFlags(const InspectorFlags&) = delete;
   InspectorFlags& operator=(const InspectorFlags&) = default;
   ~InspectorFlags() = default;
 
-  bool enableModernCDPRegistry_;
-  bool enableCxxInspectorPackagerConnection_;
-  bool enableHermesCDPAgent_;
-
+  mutable std::optional<Values> cachedValues_;
   mutable bool inconsistentFlagsStateLogged_{false};
-  void assertFlagsMatchUpstream() const;
+
+  const Values& loadFlagsAndAssertUnchanged() const;
 };
 
 } // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/tests/utils/InspectorFlagOverridesGuard.cpp

@@ -40,12 +40,12 @@ class ReactNativeFeatureFlagsOverrides
 
 InspectorFlagOverridesGuard::InspectorFlagOverridesGuard(
     const InspectorFlagOverrides& overrides) {
+  InspectorFlags::getInstance().dangerouslyResetFlags();
   ReactNativeFeatureFlags::override(
       std::make_unique<ReactNativeFeatureFlagsOverrides>(overrides));
 }
 
 InspectorFlagOverridesGuard::~InspectorFlagOverridesGuard() {
-  InspectorFlags::getInstance().dangerouslyResetFlags();
   ReactNativeFeatureFlags::dangerouslyReset();
 }