Skip to content

Commit

Permalink
Remove inspectorEnableCxxInspectorPackagerConnection flag (facebook#4…
Browse files Browse the repository at this point in the history
…4662)

Summary:
Pull Request resolved: facebook#44662

- Remove `inspectorEnableCxxInspectorPackagerConnection` feature flag.
- Replace existing checks with `getEnableModernCDPRegistry()`.

Changelog: [Internal]

Reviewed By: hoxyq

Differential Revision: D57730921

fbshipit-source-id: fd13b84a826a4fa5973053a721d311c7d68dd2f5
  • Loading branch information
huntie authored and facebook-github-bot committed May 26, 2024
1 parent e363eeb commit a908387
Show file tree
Hide file tree
Showing 31 changed files with 34 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,10 @@ + (void)disableDebugger
NSString *key = [inspectorURL absoluteString];
id<RCTInspectorPackagerConnectionProtocol> connection = socketConnections[key];
if (!connection || !connection.isConnected) {
if (facebook::react::jsinspector_modern::InspectorFlags::getInstance().getEnableCxxInspectorPackagerConnection()) {
if (facebook::react::jsinspector_modern::InspectorFlags::getInstance().getEnableModernCDPRegistry()) {
connection = [[RCTCxxInspectorPackagerConnection alloc] initWithURL:inspectorURL];
} else {
// TODO(T190163403): Remove legacy RCTInspectorPackagerConnection
connection = [[RCTInspectorPackagerConnection alloc] initWithURL:inspectorURL];
}

Expand Down
1 change: 0 additions & 1 deletion packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,6 @@ public abstract interface class com/facebook/react/devsupport/HMRClient : com/fa

public final class com/facebook/react/devsupport/InspectorFlags {
public static final field INSTANCE Lcom/facebook/react/devsupport/InspectorFlags;
public static final fun getEnableCxxInspectorPackagerConnection ()Z
public static final fun getEnableModernCDPRegistry ()Z
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,11 @@ public void openInspectorConnection() {
new AsyncTask<Void, Void, Void>() {
@Override
protected Void doInBackground(Void... params) {
if (InspectorFlags.getEnableCxxInspectorPackagerConnection()) {
if (InspectorFlags.getEnableModernCDPRegistry()) {
mInspectorPackagerConnection =
new CxxInspectorPackagerConnection(getInspectorDeviceUrl(), mPackageName);
} else {
// TODO(T190163403): Remove legacy InspectorPackagerConnection
mInspectorPackagerConnection =
new InspectorPackagerConnection(getInspectorDeviceUrl(), mPackageName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ public object InspectorFlags {
}

@DoNotStrip @JvmStatic public external fun getEnableModernCDPRegistry(): Boolean

@DoNotStrip @JvmStatic public external fun getEnableCxxInspectorPackagerConnection(): Boolean
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<0ae7be647ca12c3efcc34f6098155550>>
* @generated SignedSource<<53200319e16c1f14028d9cb8a2c87078>>
*/

/**
Expand Down Expand Up @@ -106,12 +106,6 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun forceBatchingMountItemsOnAndroid(): Boolean = accessor.forceBatchingMountItemsOnAndroid()

/**
* Flag determining if the C++ implementation of InspectorPackagerConnection should be used instead of the per-platform one. This flag is global and should not be changed across React Host lifetimes.
*/
@JvmStatic
public fun inspectorEnableCxxInspectorPackagerConnection(): Boolean = accessor.inspectorEnableCxxInspectorPackagerConnection()

/**
* Flag determining if the modern CDP backend should be enabled. This flag is global and should not be changed across React Host lifetimes.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<5323fb8be9ec7ee6ac43d7f01bca020e>>
* @generated SignedSource<<54a79c6e6f4946c4c36692fbc19b927a>>
*/

/**
Expand Down Expand Up @@ -33,7 +33,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var enableUIConsistencyCache: Boolean? = null
private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null
private var forceBatchingMountItemsOnAndroidCache: Boolean? = null
private var inspectorEnableCxxInspectorPackagerConnectionCache: Boolean? = null
private var inspectorEnableModernCDPRegistryCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var preventDoubleTextMeasureCache: Boolean? = null
Expand Down Expand Up @@ -159,15 +158,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun inspectorEnableCxxInspectorPackagerConnection(): Boolean {
var cached = inspectorEnableCxxInspectorPackagerConnectionCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.inspectorEnableCxxInspectorPackagerConnection()
inspectorEnableCxxInspectorPackagerConnectionCache = cached
}
return cached
}

override fun inspectorEnableModernCDPRegistry(): Boolean {
var cached = inspectorEnableModernCDPRegistryCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<e68ae1e7b4f0d7714722fb7b442c66c9>>
* @generated SignedSource<<4eee31ea22fc4116e9070f714dc59daf>>
*/

/**
Expand Down Expand Up @@ -54,8 +54,6 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun forceBatchingMountItemsOnAndroid(): Boolean

@DoNotStrip @JvmStatic public external fun inspectorEnableCxxInspectorPackagerConnection(): Boolean

@DoNotStrip @JvmStatic public external fun inspectorEnableModernCDPRegistry(): Boolean

@DoNotStrip @JvmStatic public external fun lazyAnimationCallbacks(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<64ab913d2c940a7abfcd2aac32be1b72>>
* @generated SignedSource<<f29998f04860c4af0eb027b2e660c4b4>>
*/

/**
Expand Down Expand Up @@ -49,8 +49,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun forceBatchingMountItemsOnAndroid(): Boolean = false

override fun inspectorEnableCxxInspectorPackagerConnection(): Boolean = false

override fun inspectorEnableModernCDPRegistry(): Boolean = false

override fun lazyAnimationCallbacks(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<00a695402781a9f3c45ba0594a785b57>>
* @generated SignedSource<<a2706af43bfe0803eab9a70d24cd79c9>>
*/

/**
Expand Down Expand Up @@ -37,7 +37,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var enableUIConsistencyCache: Boolean? = null
private var fixStoppedSurfaceRemoveDeleteTreeUIFrameCallbackLeakCache: Boolean? = null
private var forceBatchingMountItemsOnAndroidCache: Boolean? = null
private var inspectorEnableCxxInspectorPackagerConnectionCache: Boolean? = null
private var inspectorEnableModernCDPRegistryCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var preventDoubleTextMeasureCache: Boolean? = null
Expand Down Expand Up @@ -176,16 +175,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun inspectorEnableCxxInspectorPackagerConnection(): Boolean {
var cached = inspectorEnableCxxInspectorPackagerConnectionCache
if (cached == null) {
cached = currentProvider.inspectorEnableCxxInspectorPackagerConnection()
accessedFeatureFlags.add("inspectorEnableCxxInspectorPackagerConnection")
inspectorEnableCxxInspectorPackagerConnectionCache = cached
}
return cached
}

override fun inspectorEnableModernCDPRegistry(): Boolean {
var cached = inspectorEnableModernCDPRegistryCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<e63ccfcb6e8d1db840cefb3d0a6df6cd>>
* @generated SignedSource<<fff46073d92efbec147cd2f1dad110cb>>
*/

/**
Expand Down Expand Up @@ -49,8 +49,6 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun forceBatchingMountItemsOnAndroid(): Boolean

@DoNotStrip public fun inspectorEnableCxxInspectorPackagerConnection(): Boolean

@DoNotStrip public fun inspectorEnableModernCDPRegistry(): Boolean

@DoNotStrip public fun lazyAnimationCallbacks(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,11 @@ bool JInspectorFlags::getEnableModernCDPRegistry(jni::alias_ref<jclass>) {
return inspectorFlags.getEnableModernCDPRegistry();
}

bool JInspectorFlags::getEnableCxxInspectorPackagerConnection(
jni::alias_ref<jclass>) {
auto& inspectorFlags = InspectorFlags::getInstance();
return inspectorFlags.getEnableCxxInspectorPackagerConnection();
}

void JInspectorFlags::registerNatives() {
javaClassLocal()->registerNatives({
makeNativeMethod(
"getEnableModernCDPRegistry",
JInspectorFlags::getEnableModernCDPRegistry),
makeNativeMethod(
"getEnableCxxInspectorPackagerConnection",
JInspectorFlags::getEnableCxxInspectorPackagerConnection),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ class JInspectorFlags : public jni::JavaClass<JInspectorFlags> {

static bool getEnableModernCDPRegistry(jni::alias_ref<jclass>);

static bool getEnableCxxInspectorPackagerConnection(jni::alias_ref<jclass>);

static void registerNatives();

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<5d0f0b1953af3f0dd2ee66fed2b1115a>>
* @generated SignedSource<<f2a88e318efcf92bf7cd2b07e3ed5faf>>
*/

/**
Expand Down Expand Up @@ -117,12 +117,6 @@ class ReactNativeFeatureFlagsProviderHolder
return method(javaProvider_);
}

bool inspectorEnableCxxInspectorPackagerConnection() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("inspectorEnableCxxInspectorPackagerConnection");
return method(javaProvider_);
}

bool inspectorEnableModernCDPRegistry() override {
static const auto method =
getReactNativeFeatureFlagsProviderJavaClass()->getMethod<jboolean()>("inspectorEnableModernCDPRegistry");
Expand Down Expand Up @@ -234,11 +228,6 @@ bool JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid(
return ReactNativeFeatureFlags::forceBatchingMountItemsOnAndroid();
}

bool JReactNativeFeatureFlagsCxxInterop::inspectorEnableCxxInspectorPackagerConnection(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::inspectorEnableCxxInspectorPackagerConnection();
}

bool JReactNativeFeatureFlagsCxxInterop::inspectorEnableModernCDPRegistry(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop> /*unused*/) {
return ReactNativeFeatureFlags::inspectorEnableModernCDPRegistry();
Expand Down Expand Up @@ -330,9 +319,6 @@ void JReactNativeFeatureFlagsCxxInterop::registerNatives() {
makeNativeMethod(
"forceBatchingMountItemsOnAndroid",
JReactNativeFeatureFlagsCxxInterop::forceBatchingMountItemsOnAndroid),
makeNativeMethod(
"inspectorEnableCxxInspectorPackagerConnection",
JReactNativeFeatureFlagsCxxInterop::inspectorEnableCxxInspectorPackagerConnection),
makeNativeMethod(
"inspectorEnableModernCDPRegistry",
JReactNativeFeatureFlagsCxxInterop::inspectorEnableModernCDPRegistry),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<d2f016e3a0648f6fd0b6c1f45b9a1f62>>
* @generated SignedSource<<48f683554e70183664d5a1bf6a7ab328>>
*/

/**
Expand Down Expand Up @@ -69,9 +69,6 @@ class JReactNativeFeatureFlagsCxxInterop
static bool forceBatchingMountItemsOnAndroid(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool inspectorEnableCxxInspectorPackagerConnection(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

static bool inspectorEnableModernCDPRegistry(
facebook::jni::alias_ref<JReactNativeFeatureFlagsCxxInterop>);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,36 +21,20 @@ bool InspectorFlags::getEnableModernCDPRegistry() const {
return loadFlagsAndAssertUnchanged().enableModernCDPRegistry;
}

bool InspectorFlags::getEnableCxxInspectorPackagerConnection() const {
auto& values = loadFlagsAndAssertUnchanged();

return values.enableCxxInspectorPackagerConnection ||
// If we are using the modern CDP registry, then we must also use the C++
// InspectorPackagerConnection implementation.
values.enableModernCDPRegistry;
}

void InspectorFlags::dangerouslyResetFlags() {
*this = InspectorFlags{};
}

const InspectorFlags::Values& InspectorFlags::loadFlagsAndAssertUnchanged()
const {
InspectorFlags::Values newValues = {
.enableCxxInspectorPackagerConnection =
#ifdef REACT_NATIVE_FORCE_ENABLE_FUSEBOX
true,
#else
ReactNativeFeatureFlags::
inspectorEnableCxxInspectorPackagerConnection(),
#endif
.enableModernCDPRegistry =
#ifdef REACT_NATIVE_FORCE_ENABLE_FUSEBOX
true,
#elif defined(HERMES_ENABLE_DEBUGGER)
ReactNativeFeatureFlags::inspectorEnableModernCDPRegistry(),
#else
false,
false,
#endif
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ class InspectorFlags {
*/
bool getEnableModernCDPRegistry() const;

/**
* Flag determining if the C++ implementation of InspectorPackagerConnection
* should be used instead of the per-platform one.
*/
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.
Expand All @@ -38,7 +32,6 @@ class InspectorFlags {

private:
struct Values {
bool enableCxxInspectorPackagerConnection;
bool enableModernCDPRegistry;
bool operator==(const Values&) const = default;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,8 @@ INSTANTIATE_TEST_SUITE_P(
ReactInstanceVaryingInspectorFlags,
ReactInstanceIntegrationTestWithFlags,
::testing::Values(
InspectorFlagOverrides{
.enableCxxInspectorPackagerConnection = false,
.enableModernCDPRegistry = false},
InspectorFlagOverrides{
.enableCxxInspectorPackagerConnection = true,
.enableModernCDPRegistry = false},
InspectorFlagOverrides{
.enableCxxInspectorPackagerConnection = true,
.enableModernCDPRegistry = true}));
InspectorFlagOverrides{.enableModernCDPRegistry = false},
InspectorFlagOverrides{.enableModernCDPRegistry = false},
InspectorFlagOverrides{.enableModernCDPRegistry = true}));

} // namespace facebook::react::jsinspector_modern
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ namespace facebook::react::jsinspector_modern {
using namespace ::testing;

struct FeatureFlags {
const bool enableCxxInspectorPackagerConnection = true;
const bool enableModernCDPRegistry = true;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ class ReactNativeFeatureFlagsOverrides
const InspectorFlagOverrides& overrides)
: overrides_(overrides) {}

bool inspectorEnableCxxInspectorPackagerConnection() override {
return overrides_.enableCxxInspectorPackagerConnection.value_or(
ReactNativeFeatureFlagsDefaults::
inspectorEnableCxxInspectorPackagerConnection());
}

bool inspectorEnableModernCDPRegistry() override {
return overrides_.enableModernCDPRegistry.value_or(
ReactNativeFeatureFlagsDefaults::inspectorEnableModernCDPRegistry());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ namespace facebook::react::jsinspector_modern {
struct InspectorFlagOverrides {
// NOTE: Keep these entries in sync with ReactNativeFeatureFlagsOverrides in
// the implementation file.
std::optional<bool> enableCxxInspectorPackagerConnection;
std::optional<bool> enableModernCDPRegistry;
};

Expand Down
Loading

0 comments on commit a908387

Please sign in to comment.