diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp b/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp index b0e3b4fb17b..4a2c9df723a 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.cpp @@ -37,6 +37,10 @@ bool InspectorFlags::getEnableCxxInspectorPackagerConnection() const { enableModernCDPRegistry_; } +void InspectorFlags::dangerouslyResetFlags() { + *this = InspectorFlags{}; +} + void InspectorFlags::assertFlagsMatchUpstream() const { if (inconsistentFlagsStateLogged_) { return; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h b/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h index 605fad31723..74ec341be42 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/InspectorFlags.h @@ -30,14 +30,20 @@ class InspectorFlags { */ bool getEnableCxxInspectorPackagerConnection() const; + /** + * Reset flags to their upstream values. The caller must ensure any resources + * that have read previous flag values have been cleaned up. + */ + void dangerouslyResetFlags(); + private: InspectorFlags(); InspectorFlags(const InspectorFlags&) = delete; - InspectorFlags& operator=(const InspectorFlags&) = delete; + InspectorFlags& operator=(const InspectorFlags&) = default; ~InspectorFlags() = default; - const bool enableModernCDPRegistry_; - const bool enableCxxInspectorPackagerConnection_; + bool enableModernCDPRegistry_; + bool enableCxxInspectorPackagerConnection_; mutable bool inconsistentFlagsStateLogged_; void assertFlagsMatchUpstream() const; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp index 9a1b07c207a..e305713edba 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp @@ -12,19 +12,53 @@ #include #include +#include +#include +#include #include namespace facebook::react::jsinspector_modern { using namespace ::testing; +class ReactInstanceIntegrationTestFeatureFlagsProvider + : public ReactNativeFeatureFlagsDefaults { + private: + FeatureFlags flags_; + + public: + explicit ReactInstanceIntegrationTestFeatureFlagsProvider( + FeatureFlags featureFlags) + : flags_(featureFlags) {} + + bool inspectorEnableModernCDPRegistry() override { + return flags_.enableModernCDPRegistry; + } + bool inspectorEnableCxxInspectorPackagerConnection() override { + return flags_.enableCxxInspectorPackagerConnection; + } +}; + ReactInstanceIntegrationTest::ReactInstanceIntegrationTest() : runtime(nullptr), instance(nullptr), messageQueueThread(std::make_shared()), - errorHandler(std::make_shared()) {} + errorHandler(std::make_shared()), + featureFlags(std::make_unique()) {} void ReactInstanceIntegrationTest::SetUp() { + ReactNativeFeatureFlags::override( + std::make_unique( + *featureFlags)); + // Reset InspectorFlags to overridden upstream values before creating objects + // that may access them. + InspectorFlags::getInstance().dangerouslyResetFlags(); + + // Double check that a flag matches our overrides. + ASSERT_EQ( + InspectorFlags::getInstance().getEnableModernCDPRegistry(), + featureFlags->enableModernCDPRegistry); + auto mockRegistry = std::make_unique(); auto timerManager = std::make_shared(std::move(mockRegistry)); @@ -47,11 +81,24 @@ void ReactInstanceIntegrationTest::SetUp() { "ErrorUtils", jsi::Object::createFromHostObject(*jsiRuntime, errorHandler)); + std::shared_ptr hostTargetIfModernCDP = nullptr; + + if (featureFlags->enableModernCDPRegistry) { + VoidExecutor inspectorExecutor = [this](auto callback) { + immediateExecutor_.add(callback); + }; + MockHostTargetDelegate hostTargetDelegate; + hostTargetIfModernCDP = + HostTarget::create(hostTargetDelegate, inspectorExecutor); + } + instance = std::make_unique( std::move(runtime_), messageQueueThread, timerManager, - std::move(jsErrorHandlingFunc)); + std::move(jsErrorHandlingFunc), + hostTargetIfModernCDP == nullptr ? nullptr : hostTargetIfModernCDP.get()); + timerManager->setRuntimeExecutor(instance->getBufferedRuntimeExecutor()); // JS Environment: @@ -59,18 +106,53 @@ void ReactInstanceIntegrationTest::SetUp() { // Inspector: auto& inspector = getInspectorInstance(); - auto pages = inspector.getPages(); - // We should now have at least a single page once the above runtime has been - // initialized. - assert(pages.size() > 0); - size_t pageId = pages.back().id; + if (hostTargetIfModernCDP != nullptr) { + // Under modern CDP, the React host is responsible for adding itself as + // the root target on startup. + pageId_ = inspector.addPage( + "mock-title", + "mock-vm", + [hostTargetIfModernCDP](std::unique_ptr remote) + -> std::unique_ptr { + auto localConnection = hostTargetIfModernCDP->connect( + std::move(remote), + { + .integrationName = "ReactInstanceIntegrationTest", + }); + return localConnection; + }, + // TODO: Allow customisation of InspectorTargetCapabilities + {}); + } else { + // Under legacy CDP, Hermes' DecoratedRuntime adds its page automatically + // within ConnectionDemux.enableDebugging. + auto pages = inspector.getPages(); + ASSERT_GT(pages.size(), 0); + pageId_ = pages.back().id; + } - clientToVM_ = inspector.connect(pageId, mockRemoteConnections_.make_unique()); + clientToVM_ = + inspector.connect(pageId_.value(), mockRemoteConnections_.make_unique()); + + ASSERT_NE(clientToVM_, nullptr); } void ReactInstanceIntegrationTest::TearDown() { clientToVM_->disconnect(); + // Destroy the local connection. + clientToVM_.reset(); + + if (pageId_.has_value() && featureFlags->enableModernCDPRegistry) { + // Under modern CDP, clean up the page we added in SetUp and destroy + // resources owned by HostTarget. + getInspectorInstance().removePage(pageId_.value()); + } + pageId_.reset(); + + // Expect the remote connection to have been destroyed. + EXPECT_EQ(mockRemoteConnections_[0], nullptr); + ReactNativeFeatureFlags::dangerouslyReset(); } void ReactInstanceIntegrationTest::initializeRuntime(std::string_view script) { @@ -84,8 +166,6 @@ void ReactInstanceIntegrationTest::initializeRuntime(std::string_view script) { std::string init(script); // JS calls no longer buffered after calling loadScript instance->loadScript(std::make_unique(init), ""); - - messageQueueThread->flush(); } void ReactInstanceIntegrationTest::send( @@ -134,18 +214,28 @@ TEST_F(ReactInstanceIntegrationTest, RuntimeEvalTest) { EXPECT_EQ(val.asNumber(), 3); } -TEST_F(ReactInstanceIntegrationTest, ConsoleLogTest) { - InSequence s; - - EXPECT_CALL(getRemoteConnection(), onMessage(_)) - .Times(2) - .RetiresOnSaturation(); - +TEST_P(ReactInstanceIntegrationTestWithFlags, ConsoleLog) { EXPECT_CALL( getRemoteConnection(), - onMessage(JsonParsed(AllOf( - AtJsonPtr("/params/args/0/value", Eq("Hello, World!")), - AtJsonPtr("/method", Eq("Runtime.consoleAPICalled")))))); + onMessage(JsonParsed( + AtJsonPtr("/method", Eq("Runtime.executionContextCreated"))))); + + EXPECT_CALL( + getRemoteConnection(), onMessage(JsonParsed(AtJsonPtr("/id", Eq(1))))); + + InSequence s; + + // Hermes console.* interception is currently explicitly disabled under the + // modern registry, and the runtime does not yet fire these events. When the + // implementation is more complete we should be able to remove this + // condition. + if (!featureFlags->enableModernCDPRegistry) { + EXPECT_CALL( + getRemoteConnection(), + onMessage(JsonParsed(AllOf( + AtJsonPtr("/params/args/0/value", Eq("Hello, World!")), + AtJsonPtr("/method", Eq("Runtime.consoleAPICalled")))))); + } EXPECT_CALL(getRemoteConnection(), onDisconnect()); @@ -153,4 +243,18 @@ TEST_F(ReactInstanceIntegrationTest, ConsoleLogTest) { run("console.log('Hello, World!');"); } +INSTANTIATE_TEST_SUITE_P( + ReactInstanceVaryingInspectorFlags, + ReactInstanceIntegrationTestWithFlags, + ::testing::Values( + FeatureFlags{ + .enableCxxInspectorPackagerConnection = true, + .enableModernCDPRegistry = true}, + FeatureFlags{ + .enableCxxInspectorPackagerConnection = false, + .enableModernCDPRegistry = false}, + FeatureFlags{ + .enableCxxInspectorPackagerConnection = true, + .enableModernCDPRegistry = false})); + } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.h index d21144bdbcb..b95500f609c 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include #include @@ -18,7 +19,14 @@ namespace facebook::react::jsinspector_modern { -class ReactInstanceIntegrationTest : public ::testing::Test { +using namespace ::testing; + +struct FeatureFlags { + const bool enableCxxInspectorPackagerConnection = true; + const bool enableModernCDPRegistry = true; +}; + +class ReactInstanceIntegrationTest : public Test { protected: ReactInstanceIntegrationTest(); void SetUp() override; @@ -36,9 +44,13 @@ class ReactInstanceIntegrationTest : public ::testing::Test { std::unique_ptr instance; std::shared_ptr messageQueueThread; std::shared_ptr errorHandler; + std::unique_ptr featureFlags; MockRemoteConnection& getRemoteConnection() { - return *mockRemoteConnections_[0]; + EXPECT_EQ(mockRemoteConnections_.objectsVended(), 1); + auto rawPtr = mockRemoteConnections_[0]; + ASSERT(rawPtr != nullptr); + return *rawPtr; } private: @@ -46,8 +58,18 @@ class ReactInstanceIntegrationTest : public ::testing::Test { size_t id_ = 1; bool verbose_ = false; + std::optional pageId_; UniquePtrFactory mockRemoteConnections_; std::unique_ptr clientToVM_; + folly::QueuedImmediateExecutor immediateExecutor_; }; +class ReactInstanceIntegrationTestWithFlags + : public ReactInstanceIntegrationTest, + public ::testing::WithParamInterface { + void SetUp() override { + featureFlags = std::make_unique(GetParam()); + ReactInstanceIntegrationTest::SetUp(); + } +}; } // namespace facebook::react::jsinspector_modern