From 1977aefd41c2e4de45e3e95860a67b2a1d56fa01 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Mon, 24 Jun 2024 08:45:20 -0700 Subject: [PATCH 1/2] Keep HostTarget registered until ReactHostImpl/ReactInstanceManager is invalidated Differential Revision: D58284590 --- .../facebook/react/ReactInstanceManager.java | 95 +++++++++++-------- .../facebook/react/runtime/ReactHostImpl.java | 30 +++--- .../src/main/jni/react/jni/JWeakRefUtils.h | 23 +++++ .../ReactInstanceManagerInspectorTarget.cpp | 38 ++++---- .../jni/ReactInstanceManagerInspectorTarget.h | 11 +-- .../runtime/jni/JReactHostInspectorTarget.cpp | 48 ++++++---- .../runtime/jni/JReactHostInspectorTarget.h | 15 +-- 7 files changed, 151 insertions(+), 109 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/jni/JWeakRefUtils.h diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 9602bb752ac6..1f719d65b04d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -113,6 +113,7 @@ import com.facebook.soloader.SoLoader; import com.facebook.systrace.Systrace; import com.facebook.systrace.SystraceMessage; +import java.lang.ref.WeakReference; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; @@ -781,10 +782,13 @@ public void destroy() { } } - // If the host is being destroyed, now that the current context/instance + // If the host has been invalidated, now that the current context/instance // has been destroyed, we can safely destroy the host's inspector target. - if (mLifecycleState == LifecycleState.BEFORE_CREATE) { - destroyInspectorHostTarget(); + if (mInstanceManagerInvalidated) { + if (mInspectorTarget != null) { + mInspectorTarget.close(); + mInspectorTarget = null; + } } mHasStartedCreatingInitialContext = false; @@ -840,10 +844,6 @@ private synchronized void moveToBeforeCreateLifecycleState() { if (mLifecycleState == LifecycleState.BEFORE_RESUME) { currentContext.onHostDestroy(mKeepActivity); } - } else { - // There's no current context that requires the host inspector target to - // be kept alive, so we can destroy it immediately. - destroyInspectorHostTarget(); } mLifecycleState = LifecycleState.BEFORE_CREATE; } @@ -1539,46 +1539,59 @@ private void processPackage( SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush(); } - private @Nullable ReactInstanceManagerInspectorTarget getOrCreateInspectorTarget() { - if (mInspectorTarget == null && InspectorFlags.getFuseboxEnabled()) { + private static class InspectorTargetDelegateImpl + implements ReactInstanceManagerInspectorTarget.TargetDelegate { + // This weak reference breaks the cycle between the C++ HostTarget and the + // Java ReactInstanceManager, preventing memory leaks in apps that create + // multiple ReactInstanceManagers over time. + private WeakReference mReactInstanceManagerWeak; - mInspectorTarget = - new ReactInstanceManagerInspectorTarget( - new ReactInstanceManagerInspectorTarget.TargetDelegate() { - @Override - public void onReload() { - UiThreadUtil.runOnUiThread(() -> mDevSupportManager.handleReloadJS()); - } + public InspectorTargetDelegateImpl(ReactInstanceManager inspectorTarget) { + mReactInstanceManagerWeak = new WeakReference(inspectorTarget); + } - @Override - public void onSetPausedInDebuggerMessage(@Nullable String message) { - if (message == null) { - mDevSupportManager.hidePausedInDebuggerOverlay(); - } else { - mDevSupportManager.showPausedInDebuggerOverlay( - message, - new PausedInDebuggerOverlayCommandListener() { - @Override - public void onResume() { - UiThreadUtil.assertOnUiThread(); - if (mInspectorTarget != null) { - mInspectorTarget.sendDebuggerResumeCommand(); - } - } - }); - } - } - }); + @Override + public void onReload() { + UiThreadUtil.runOnUiThread( + () -> { + ReactInstanceManager reactInstanceManager = mReactInstanceManagerWeak.get(); + if (reactInstanceManager != null) { + reactInstanceManager.mDevSupportManager.handleReloadJS(); + } + }); } - return mInspectorTarget; + @Override + public void onSetPausedInDebuggerMessage(@Nullable String message) { + ReactInstanceManager reactInstanceManager = mReactInstanceManagerWeak.get(); + if (reactInstanceManager == null) { + return; + } + if (message == null) { + reactInstanceManager.mDevSupportManager.hidePausedInDebuggerOverlay(); + } else { + reactInstanceManager.mDevSupportManager.showPausedInDebuggerOverlay( + message, + new PausedInDebuggerOverlayCommandListener() { + @Override + public void onResume() { + UiThreadUtil.assertOnUiThread(); + if (reactInstanceManager.mInspectorTarget != null) { + reactInstanceManager.mInspectorTarget.sendDebuggerResumeCommand(); + } + } + }); + } + } } - @ThreadConfined(UI) - private void destroyInspectorHostTarget() { - if (mInspectorTarget != null) { - mInspectorTarget.close(); - mInspectorTarget = null; + private @Nullable ReactInstanceManagerInspectorTarget getOrCreateInspectorTarget() { + if (mInspectorTarget == null && InspectorFlags.getFuseboxEnabled()) { + + mInspectorTarget = + new ReactInstanceManagerInspectorTarget(new InspectorTargetDelegateImpl(this)); } + + return mInspectorTarget; } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java index 47a420587d08..8bc96e4e67ab 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java @@ -893,11 +893,6 @@ private Task getOrCreateStartTask() { @ThreadConfined(UI) private void moveToHostDestroy(@Nullable ReactContext currentContext) { mReactLifecycleStateManager.moveToOnHostDestroy(currentContext); - if (currentContext == null) { - // There's no current context/instance that requires the host inspector - // target to be kept alive, so we can destroy it immediately. - destroyInspectorHostTarget(); - } setCurrentActivity(null); } @@ -1545,6 +1540,16 @@ private Task getOrCreateDestroyTask(final String reason, @Nullable Excepti unregisterInstanceFromInspector(reactInstance); + if (mHostInvalidated) { + // If the host has been invalidated, now that the current context/instance + // has been unregistered, we can safely destroy the host's inspector + // target. + if (mReactHostInspectorTarget != null) { + mReactHostInspectorTarget.close(); + mReactHostInspectorTarget = null; + } + } + // Step 1: Destroy DevSupportManager if (mUseDevSupport) { log(method, "DevSupportManager cleanup"); @@ -1698,20 +1703,13 @@ public void setJsEngineResolutionAlgorithm( private @Nullable ReactHostInspectorTarget getOrCreateReactHostInspectorTarget() { if (mReactHostInspectorTarget == null && InspectorFlags.getFuseboxEnabled()) { + // NOTE: ReactHostInspectorTarget only retains a weak reference to `this`. mReactHostInspectorTarget = new ReactHostInspectorTarget(this); } return mReactHostInspectorTarget; } - @ThreadConfined(UI) - private void destroyInspectorHostTarget() { - if (mReactHostInspectorTarget != null) { - mReactHostInspectorTarget.close(); - mReactHostInspectorTarget = null; - } - } - @ThreadConfined(UI) private void unregisterInstanceFromInspector(final @Nullable ReactInstance reactInstance) { if (reactInstance != null) { @@ -1722,12 +1720,6 @@ private void unregisterInstanceFromInspector(final @Nullable ReactInstance react } reactInstance.unregisterFromInspector(); } - if (mReactLifecycleStateManager.getLifecycleState() == LifecycleState.BEFORE_CREATE) { - // If the host is being destroyed, now that the current context/instance - // has been unregistered, we can safely destroy the host's inspector - // target. - destroyInspectorHostTarget(); - } } @Override diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JWeakRefUtils.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JWeakRefUtils.h new file mode 100644 index 000000000000..6674afb9cfb1 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JWeakRefUtils.h @@ -0,0 +1,23 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include + +namespace facebook::react { + +/** + * Helper for constructing a JWeakReference. \see JWeakReference.h in fbjni. + */ +template +inline jni::local_ref> makeJWeakReference( + jni::alias_ref ref) { + return jni::JWeakReference::newInstance(ref); +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp index 87a8a4aa3d02..15d91ee2a76d 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.cpp @@ -31,24 +31,22 @@ void ReactInstanceManagerInspectorTarget::TargetDelegate:: ReactInstanceManagerInspectorTarget::ReactInstanceManagerInspectorTarget( jni::alias_ref jobj, - jni::alias_ref executor, - jni::alias_ref< - ReactInstanceManagerInspectorTarget::TargetDelegate::javaobject> + jni::alias_ref javaExecutor, + jni::alias_ref delegate) - : delegate_(make_global(delegate)) { + : delegate_(make_global(delegate)), + inspectorExecutor_([javaExecutor = + // Use a SafeReleaseJniRef because this lambda may + // be copied to arbitrary threads. + SafeReleaseJniRef(make_global(javaExecutor))]( + auto callback) mutable { + auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(callback)); + javaExecutor->execute(jrunnable); + }) { auto& inspectorFlags = InspectorFlags::getInstance(); if (inspectorFlags.getFuseboxEnabled()) { - inspectorTarget_ = HostTarget::create( - *this, - [javaExecutor = - // Use a SafeReleaseJniRef because this lambda may be copied to - // arbitrary threads. - SafeReleaseJniRef(make_global(executor))](auto callback) mutable { - auto jrunnable = - JNativeRunnable::newObjectCxxArgs(std::move(callback)); - javaExecutor->execute(jrunnable); - }); + inspectorTarget_ = HostTarget::create(*this, inspectorExecutor_); inspectorPageId_ = getInspectorInstance().addPage( "React Native Bridge (Experimental)", @@ -64,18 +62,24 @@ ReactInstanceManagerInspectorTarget::ReactInstanceManagerInspectorTarget( ReactInstanceManagerInspectorTarget::~ReactInstanceManagerInspectorTarget() { if (inspectorPageId_.has_value()) { - getInspectorInstance().removePage(*inspectorPageId_); + // Remove the page (terminating all sessions) and destroy the target, both + // on the inspector queue. + inspectorExecutor_([inspectorPageId = *inspectorPageId_, + inspectorTarget = std::move(inspectorTarget_)]() { + getInspectorInstance().removePage(inspectorPageId); + (void)inspectorTarget; + }); } } jni::local_ref ReactInstanceManagerInspectorTarget::initHybrid( jni::alias_ref jobj, - jni::alias_ref executor, + jni::alias_ref javaExecutor, jni::alias_ref< ReactInstanceManagerInspectorTarget::TargetDelegate::javaobject> delegate) { - return makeCxxInstance(jobj, executor, delegate); + return makeCxxInstance(jobj, javaExecutor, delegate); } void ReactInstanceManagerInspectorTarget::sendDebuggerResumeCommand() { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.h index 7aa4721ef7b1..97be0427ea5e 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/ReactInstanceManagerInspectorTarget.h @@ -39,7 +39,7 @@ class ReactInstanceManagerInspectorTarget static jni::local_ref initHybrid( jni::alias_ref jobj, - jni::alias_ref executor, + jni::alias_ref javaExecutor, jni::alias_ref< ReactInstanceManagerInspectorTarget::TargetDelegate::javaobject> delegate); @@ -61,14 +61,13 @@ class ReactInstanceManagerInspectorTarget ReactInstanceManagerInspectorTarget( jni::alias_ref jobj, - jni::alias_ref executor, - jni::alias_ref< - ReactInstanceManagerInspectorTarget::TargetDelegate::javaobject> + jni::alias_ref javaExecutor, + jni::alias_ref delegate); - jni::global_ref< - ReactInstanceManagerInspectorTarget::TargetDelegate::javaobject> + jni::global_ref delegate_; + jsinspector_modern::VoidExecutor inspectorExecutor_; std::shared_ptr inspectorTarget_; std::optional inspectorPageId_; }; diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp index d65677be3510..c1335b40a614 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp @@ -8,6 +8,7 @@ #include "JReactHostInspectorTarget.h" #include #include +#include #include using namespace facebook::jni; @@ -15,23 +16,20 @@ using namespace facebook::react::jsinspector_modern; namespace facebook::react { JReactHostInspectorTarget::JReactHostInspectorTarget( - alias_ref reactHostImpl, + alias_ref reactHostImpl, alias_ref executor) - : javaReactHostImpl_(make_global(reactHostImpl)), - javaExecutor_(make_global(executor)) { + : javaReactHostImpl_(make_global(makeJWeakReference(reactHostImpl))), + inspectorExecutor_([javaExecutor = + // Use a SafeReleaseJniRef because this lambda may + // be copied to arbitrary threads. + SafeReleaseJniRef(make_global(executor))]( + std::function&& callback) mutable { + auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(callback)); + javaExecutor->execute(jrunnable); + }) { auto& inspectorFlags = InspectorFlags::getInstance(); if (inspectorFlags.getFuseboxEnabled()) { - inspectorTarget_ = HostTarget::create( - *this, - [javaExecutor = - // Use a SafeReleaseJniRef because this lambda may be copied to - // arbitrary threads. - SafeReleaseJniRef(javaExecutor_)]( - std::function&& callback) mutable { - auto jrunnable = - JNativeRunnable::newObjectCxxArgs(std::move(callback)); - javaExecutor->execute(jrunnable); - }); + inspectorTarget_ = HostTarget::create(*this, inspectorExecutor_); inspectorPageId_ = getInspectorInstance().addPage( "React Native Bridgeless (Experimental)", @@ -51,16 +49,22 @@ JReactHostInspectorTarget::JReactHostInspectorTarget( JReactHostInspectorTarget::~JReactHostInspectorTarget() { if (inspectorPageId_.has_value()) { - getInspectorInstance().removePage(*inspectorPageId_); + // Remove the page (terminating all sessions) and destroy the target, both + // on the inspector queue. + inspectorExecutor_([inspectorPageId = *inspectorPageId_, + inspectorTarget = std::move(inspectorTarget_)]() { + getInspectorInstance().removePage(inspectorPageId); + (void)inspectorTarget; + }); } } local_ref JReactHostInspectorTarget::initHybrid( alias_ref self, - jni::alias_ref reactHostImpl, - jni::alias_ref executor) { - return makeCxxInstance(reactHostImpl, executor); + jni::alias_ref reactHostImpl, + jni::alias_ref javaExecutor) { + return makeCxxInstance(reactHostImpl, javaExecutor); } void JReactHostInspectorTarget::sendDebuggerResumeCommand() { @@ -90,12 +94,16 @@ JReactHostInspectorTarget::getMetadata() { } void JReactHostInspectorTarget::onReload(const PageReloadRequest& request) { - javaReactHostImpl_->reload("CDP Page.reload"); + if (auto javaReactHostImplStrong = javaReactHostImpl_->get()) { + javaReactHostImplStrong->reload("CDP Page.reload"); + } } void JReactHostInspectorTarget::onSetPausedInDebuggerMessage( const OverlaySetPausedInDebuggerMessageRequest& request) { - javaReactHostImpl_->setPausedInDebuggerMessage(request.message); + if (auto javaReactHostImplStrong = javaReactHostImpl_->get()) { + javaReactHostImplStrong->setPausedInDebuggerMessage(request.message); + } } HostTarget* JReactHostInspectorTarget::getInspectorTarget() { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h index aa6a250295f0..b68ecd735ae0 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h @@ -49,8 +49,8 @@ class JReactHostInspectorTarget static jni::local_ref initHybrid( jni::alias_ref jThis, - jni::alias_ref reactHost, - jni::alias_ref); + jni::alias_ref reactHost, + jni::alias_ref javaExecutor); static void registerNatives(); void sendDebuggerResumeCommand(); @@ -65,10 +65,13 @@ class JReactHostInspectorTarget private: JReactHostInspectorTarget( - jni::alias_ref reactHostImpl, - jni::alias_ref executor); - jni::global_ref javaReactHostImpl_; - jni::global_ref javaExecutor_; + jni::alias_ref reactHostImpl, + jni::alias_ref javaExecutor); + // This weak reference breaks the cycle between the C++ HostTarget and the + // Java ReactHostImpl, preventing memory leaks in apps that create multiple + // ReactHostImpls over time. + jni::global_ref> javaReactHostImpl_; + jsinspector_modern::VoidExecutor inspectorExecutor_; std::shared_ptr inspectorTarget_; std::optional inspectorPageId_; From 00032b3e23ca821fc2707b9ff1130b13efa229a7 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Mon, 24 Jun 2024 10:35:20 -0700 Subject: [PATCH 2/2] Reject debugger connections to unknown page IDs Differential Revision: D58954759 --- .../InspectorProxyCdpTransport-test.js | 118 ++++++++++++++++++ .../src/inspector-proxy/Device.js | 30 +++-- 2 files changed, 136 insertions(+), 12 deletions(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js index 5c3536be64ae..5e2396edb0f7 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js @@ -268,5 +268,123 @@ describe.each(['HTTP', 'HTTPS'])( debugger2?.close(); } }); + + test('debugger connection to a nonexistent page is rejected', async () => { + let device, debugger_; + try { + device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + // Set up a page. + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ]); + let pageList: Array = []; + await until(async () => { + pageList = (await fetchJson( + `${serverRef.serverBaseUrl}/json`, + // $FlowIgnore[unclear-type] + ): any); + expect(pageList).toHaveLength(1); + }); + const [{webSocketDebuggerUrl}] = pageList; + + // Connect the debugger to a nonexistent page. + debugger_ = await createDebuggerMock( + webSocketDebuggerUrl.replaceAll('page1', 'some-other-id'), + autoCleanup.signal, + ); + + // The debugger gets disconnected automatically. + await until(async () => { + expect([ + // CLOSING + 3, + // CLOSED + 4, + ]).toContain(debugger_.socket.readyState); + }); + + expect(device.connect).not.toHaveBeenCalled(); + expect(device.disconnect).not.toHaveBeenCalled(); + } finally { + device?.close(); + debugger_?.close(); + } + }); + + test('debugger connection to a nonexistent page does not kill the current debugger connection', async () => { + let device, debugger1, debugger2; + try { + device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + // Set up a page. + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + }, + ]); + let pageList: Array = []; + await until(async () => { + pageList = (await fetchJson( + `${serverRef.serverBaseUrl}/json`, + // $FlowIgnore[unclear-type] + ): any); + expect(pageList).toHaveLength(1); + }); + const [{webSocketDebuggerUrl}] = pageList; + + // Connect the first debugger. + debugger1 = await createDebuggerMock( + webSocketDebuggerUrl, + autoCleanup.signal, + ); + + // Connect a second debugger to a nonexistent page. + debugger2 = await createDebuggerMock( + webSocketDebuggerUrl.replaceAll('page1', 'some-other-id'), + autoCleanup.signal, + ); + + // The second debugger gets disconnected automatically. + await until(async () => { + expect([ + // CLOSING + 3, + // CLOSED + 4, + ]).toContain(debugger2.socket.readyState); + }); + + // We can still send messages through the first debugger. + await sendFromDebuggerToTarget(debugger1, device, 'page1', { + method: 'Runtime.enable', + id: 0, + }); + + expect(device.connect).toHaveBeenCalledWith({ + event: 'connect', + payload: { + pageId: 'page1', + }, + }); + expect(device.disconnect).not.toHaveBeenCalled(); + } finally { + device?.close(); + debugger1?.close(); + debugger2?.close(); + } + }); }, ); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index 7be749d07cd9..4d058de8dd20 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -308,17 +308,30 @@ export default class Device { userAgent: string | null, }>, ) { + const page: ?Page = + pageId === REACT_NATIVE_RELOADABLE_PAGE_ID + ? this.#createSyntheticPage() + : this.#pages.get(pageId); + + if (!page) { + debug( + `Got new debugger connection for page ${pageId} of ${this.#name}, but no such page exists`, + ); + socket.close(); + return; + } + // Clear any commands we were waiting on. this.#deviceEventReporter?.logDisconnection('debugger'); + // Disconnect current debugger if we already have debugger connected. + this.#terminateDebuggerConnection(); + this.#deviceEventReporter?.logConnection('debugger', { pageId, frontendUserAgent: metadata.userAgent, }); - // Disconnect current debugger if we already have debugger connected. - this.#terminateDebuggerConnection(); - const debuggerInfo = { socket, prependedFilePrefix: false, @@ -327,18 +340,11 @@ export default class Device { customHandler: null, }; - // TODO(moti): Handle null case explicitly, e.g. refuse to connect to - // unknown pages. - const page: ?Page = - pageId === REACT_NATIVE_RELOADABLE_PAGE_ID - ? this.#createSyntheticPage() - : this.#pages.get(pageId); - this.#debuggerConnection = debuggerInfo; debug(`Got new debugger connection for page ${pageId} of ${this.#name}`); - if (page && this.#debuggerConnection && this.#createCustomMessageHandler) { + if (this.#debuggerConnection && this.#createCustomMessageHandler) { this.#debuggerConnection.customHandler = this.#createCustomMessageHandler( { page, @@ -405,7 +411,7 @@ export default class Device { return; } - if (!page || !this.#pageHasCapability(page, 'nativeSourceCodeFetching')) { + if (!this.#pageHasCapability(page, 'nativeSourceCodeFetching')) { processedReq = this.#interceptClientMessageForSourceFetching( debuggerRequest, debuggerInfo,