diff --git a/React/Inspector/RCTInspector.mm b/React/Inspector/RCTInspector.mm index da3e2d5f5..1cc3d1d3a 100644 --- a/React/Inspector/RCTInspector.mm +++ b/React/Inspector/RCTInspector.mm @@ -49,17 +49,9 @@ private: - (instancetype)initWithConnection:(std::unique_ptr)connection; @end -// Only safe to call with Custom JSC. Custom JSC check must occur earlier -// in the stack static IInspector *getInstance() { - static dispatch_once_t onceToken; - static IInspector *s_inspector; - dispatch_once(&onceToken, ^{ - s_inspector = customJSCWrapper()->JSInspectorGetInstance(); - }); - - return s_inspector; + return &facebook::react::getInspectorInstance(); } @implementation RCTInspector diff --git a/ReactAndroid/src/main/jni/react/jni/JInspector.cpp b/ReactAndroid/src/main/jni/react/jni/JInspector.cpp index 62f0cb66d..8e64ce1ad 100644 --- a/ReactAndroid/src/main/jni/react/jni/JInspector.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JInspector.cpp @@ -61,12 +61,8 @@ void JLocalConnection::registerNatives() { }); } -static IInspector* getInspectorInstance() { - return JSC_JSInspectorGetInstance(true /*useCustomJSC*/); -} - jni::global_ref JInspector::instance(jni::alias_ref) { - static auto instance = jni::make_global(newObjectCxxArgs(getInspectorInstance()/*&Inspector::instance()*/)); + static auto instance = jni::make_global(newObjectCxxArgs(&getInspectorInstance())); return instance; } diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index f60e7ca20..b87c15d26 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -215,11 +215,12 @@ namespace facebook { const std::string ownerId = m_jscConfig.getDefault("OwnerIdentity", "unknown").getString(); const std::string appId = m_jscConfig.getDefault("AppIdentity", "unknown").getString(); const std::string deviceId = m_jscConfig.getDefault("DeviceIdentity", "unknown").getString(); - const std::function checkIsInspectedRemote = [&](){ + auto checkIsInspectedRemote = [ownerId, appId, deviceId]() { return isNetworkInspected(ownerId, appId, deviceId); }; - IInspector* pInspector = JSC_JSInspectorGetInstance(true); - pInspector->registerGlobalContext(ownerId, checkIsInspectedRemote, m_context); + + auto& globalInspector = facebook::react::getInspectorInstance(); + JSC_JSGlobalContextEnableDebugger(m_context, globalInspector, ownerId.c_str(), checkIsInspectedRemote); } installNativeHook<&JSCExecutor::nativeFlushQueueImmediate>("nativeFlushQueueImmediate"); @@ -340,8 +341,8 @@ namespace facebook { m_nativeModules.reset(); if (canUseInspector(context)) { - IInspector* pInspector = JSC_JSInspectorGetInstance(true); - pInspector->unregisterGlobalContext(context); + auto &globalInspector = facebook::react::getInspectorInstance(); + JSC_JSGlobalContextDisableDebugger(context, globalInspector); } JSC_JSGlobalContextRelease(context); diff --git a/ReactCommon/cxxreact/JSCExecutor.h b/ReactCommon/cxxreact/JSCExecutor.h index c646677d6..2b85d307d 100644 --- a/ReactCommon/cxxreact/JSCExecutor.h +++ b/ReactCommon/cxxreact/JSCExecutor.h @@ -117,7 +117,7 @@ private: folly::Optional m_callFunctionReturnResultAndFlushedQueueJS; void initOnJSVMThread() throw(JSException); - bool isNetworkInspected(const std::string &owner, const std::string &app, const std::string &device); + static bool isNetworkInspected(const std::string &owner, const std::string &app, const std::string &device); // This method is experimental, and may be modified or removed. Value callFunctionSyncWithValue( const std::string& module, const std::string& method, Value value); diff --git a/ReactCommon/jschelpers/JSCWrapper.h b/ReactCommon/jschelpers/JSCWrapper.h index 7e624844c..ff9d7890f 100644 --- a/ReactCommon/jschelpers/JSCWrapper.h +++ b/ReactCommon/jschelpers/JSCWrapper.h @@ -9,6 +9,7 @@ #pragma once +#include #include #include @@ -32,7 +33,14 @@ namespace react { } } -JSC_IMPORT facebook::react::IInspector* JSInspectorGetInstance(); +JSC_IMPORT void JSGlobalContextEnableDebugger( + JSGlobalContextRef ctx, + facebook::react::IInspector &globalInspector, + const char *title, + const std::function &checkIsInspectedRemote); +JSC_IMPORT void JSGlobalContextDisableDebugger( + JSGlobalContextRef ctx, + facebook::react::IInspector &globalInspector); // This is used to substitute an alternate JSC implementation for // testing. These calls must all be ABI compatible with the standard JSC. @@ -143,7 +151,8 @@ struct JSCWrapper { JSC_WRAPPER_METHOD(JSPokeSamplingProfiler); JSC_WRAPPER_METHOD(JSStartSamplingProfilingOnMainJSCThread); - JSC_WRAPPER_METHOD(JSInspectorGetInstance); + JSC_WRAPPER_METHOD(JSGlobalContextEnableDebugger); + JSC_WRAPPER_METHOD(JSGlobalContextDisableDebugger); JSC_WRAPPER_METHOD(configureJSCForIOS); diff --git a/ReactCommon/jschelpers/JavaScriptCore.h b/ReactCommon/jschelpers/JavaScriptCore.h index ae23a1ac0..9cae11681 100644 --- a/ReactCommon/jschelpers/JavaScriptCore.h +++ b/ReactCommon/jschelpers/JavaScriptCore.h @@ -191,9 +191,13 @@ jsc_poison(JSObjectMakeArrayBufferWithBytesNoCopy JSObjectMakeTypedArray jsc_poison(JSSamplingProfilerEnabled JSPokeSamplingProfiler JSStartSamplingProfilingOnMainJSCThread) -#define JSC_JSInspectorGetInstance(...) __jsc_bool_wrapper(JSInspectorGetInstance, __VA_ARGS__) -// no need to poison JSInspectorGetInstance because it's not defined for System JSC / standard SDK header -// jsc_poison(JSInspectorGetInstance) +#define JSC_JSGlobalContextEnableDebugger(...) __jsc_wrapper(JSGlobalContextEnableDebugger, __VA_ARGS__) +// no need to poison JSGlobalContextEnableDebugger because it's not defined for System JSC / standard SDK header +// jsc_poison(JSGlobalContextEnableDebugger) + +#define JSC_JSGlobalContextDisableDebugger(...) __jsc_wrapper(JSGlobalContextDisableDebugger, __VA_ARGS__) +// no need to poison JSGlobalContextDisableDebugger because it's not defined for System JSC / standard SDK header +// jsc_poison(JSGlobalContextDisableDebugger) #define JSC_configureJSCForIOS(...) __jsc_bool_wrapper(configureJSCForIOS, __VA_ARGS__) diff --git a/ReactCommon/jschelpers/systemJSCWrapper.cpp b/ReactCommon/jschelpers/systemJSCWrapper.cpp index c501f1fa5..e050468b4 100644 --- a/ReactCommon/jschelpers/systemJSCWrapper.cpp +++ b/ReactCommon/jschelpers/systemJSCWrapper.cpp @@ -29,7 +29,8 @@ UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSStringCreateWithUTF8CStringExpectAscii) UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSPokeSamplingProfiler) UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSStartSamplingProfilingOnMainJSCThread) -UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSInspectorGetInstance) +UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSGlobalContextEnableDebugger) +UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSGlobalContextDisableDebugger) UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(configureJSCForIOS) @@ -129,9 +130,12 @@ const JSCWrapper* systemJSCWrapper() { (decltype(&JSStartSamplingProfilingOnMainJSCThread)) Unimplemented_JSStartSamplingProfilingOnMainJSCThread, - .JSInspectorGetInstance = - (decltype(&JSInspectorGetInstance)) - Unimplemented_JSInspectorGetInstance, + .JSGlobalContextEnableDebugger = + (decltype(&JSGlobalContextEnableDebugger)) + Unimplemented_JSGlobalContextEnableDebugger, + .JSGlobalContextDisableDebugger = + (decltype(&JSGlobalContextDisableDebugger)) + Unimplemented_JSGlobalContextDisableDebugger, .configureJSCForIOS = (decltype(&configureJSCForIOS))Unimplemented_configureJSCForIOS, diff --git a/ReactCommon/jsinspector/BUCK b/ReactCommon/jsinspector/BUCK index 690ddfb02..8cd407356 100644 --- a/ReactCommon/jsinspector/BUCK +++ b/ReactCommon/jsinspector/BUCK @@ -20,6 +20,7 @@ rn_xplat_cxx_library( "-fexceptions", "-std=c++1y", ], + fbandroid_preferred_linkage = "shared", visibility = [ "PUBLIC", ], diff --git a/ReactCommon/jsinspector/InspectorInterfaces.cpp b/ReactCommon/jsinspector/InspectorInterfaces.cpp index e065ea33a..212928759 100644 --- a/ReactCommon/jsinspector/InspectorInterfaces.cpp +++ b/ReactCommon/jsinspector/InspectorInterfaces.cpp @@ -9,6 +9,9 @@ #include "InspectorInterfaces.h" +#include +#include + namespace facebook { namespace react { @@ -19,5 +22,80 @@ IDestructible::~IDestructible() { } ILocalConnection::~ILocalConnection() { } IRemoteConnection::~IRemoteConnection() { } +namespace { + +class InspectorImpl : public IInspector { + public: + int addPage(const std::string& title, ConnectFunc connectFunc) override; + void removePage(int pageId) override; + + std::vector getPages() const override; + std::unique_ptr connect( + int pageId, + std::unique_ptr remote) override; + + private: + mutable std::mutex mutex_; + int nextPageId_{1}; + std::unordered_map titles_; + std::unordered_map connectFuncs_; +}; + +int InspectorImpl::addPage(const std::string& title, ConnectFunc connectFunc) { + std::lock_guard lock(mutex_); + + int pageId = nextPageId_++; + titles_[pageId] = title; + connectFuncs_[pageId] = std::move(connectFunc); + + return pageId; } + +void InspectorImpl::removePage(int pageId) { + std::lock_guard lock(mutex_); + + titles_.erase(pageId); + connectFuncs_.erase(pageId); } + +std::vector InspectorImpl::getPages() const { + std::lock_guard lock(mutex_); + + std::vector inspectorPages; + for (auto& it : titles_) { + inspectorPages.push_back(InspectorPage{it.first, it.second}); + } + + return inspectorPages; +} + +std::unique_ptr InspectorImpl::connect( + int pageId, + std::unique_ptr remote) { + IInspector::ConnectFunc connectFunc; + + { + std::lock_guard lock(mutex_); + + auto it = connectFuncs_.find(pageId); + if (it != connectFuncs_.end()) { + connectFunc = it->second; + } + } + + return connectFunc ? connectFunc(std::move(remote)) : nullptr; +} + +} // namespace + +IInspector& getInspectorInstance() { + static InspectorImpl instance; + return instance; +} + +std::unique_ptr makeTestInspectorInstance() { + return std::make_unique(); +} + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/jsinspector/InspectorInterfaces.h b/ReactCommon/jsinspector/InspectorInterfaces.h index 8ae73571f..d137f5a58 100644 --- a/ReactCommon/jsinspector/InspectorInterfaces.h +++ b/ReactCommon/jsinspector/InspectorInterfaces.h @@ -18,7 +18,7 @@ namespace facebook { namespace react { class IDestructible { -public: + public: virtual ~IDestructible() = 0; }; @@ -27,29 +27,52 @@ struct InspectorPage { const std::string title; }; +/// IRemoteConnection allows the VM to send debugger messages to the client. class IRemoteConnection : public IDestructible { -public: + public: virtual ~IRemoteConnection() = 0; virtual void onMessage(std::string message) = 0; virtual void onDisconnect() = 0; }; +/// ILocalConnection allows the client to send debugger messages to the VM. class ILocalConnection : public IDestructible { -public: + public: virtual ~ILocalConnection() = 0; virtual void sendMessage(std::string message) = 0; virtual void disconnect() = 0; }; -// Note: not destructible! +/// IInspector tracks debuggable JavaScript targets (pages). class IInspector { -public: - virtual void registerGlobalContext(const std::string& title, const std::function &checkIsInspectedRemote, void* ctx) = 0; - virtual void unregisterGlobalContext(void* ctx) = 0; + public: + using ConnectFunc = std::function( + std::unique_ptr)>; + /// addPage is called by the VM to add a page to the list of debuggable pages. + virtual int addPage(const std::string& title, ConnectFunc connectFunc) = 0; + + /// removePage is called by the VM to remove a page from the list of + /// debuggable pages. + virtual void removePage(int pageId) = 0; + + /// getPages is called by the client to list all debuggable pages. virtual std::vector getPages() const = 0; - virtual std::unique_ptr connect(int pageId, std::unique_ptr remote) = 0; + + /// connect is called by the client to initiate a debugging session on the + /// given page. + virtual std::unique_ptr connect( + int pageId, + std::unique_ptr remote) = 0; }; -} -} +/// getInspectorInstance retrieves the singleton inspector that tracks all +/// debuggable pages in this process. +extern IInspector& getInspectorInstance(); + +/// makeTestInspectorInstance creates an independent inspector instance that +/// should only be used in tests. +extern std::unique_ptr makeTestInspectorInstance(); + +} // namespace react +} // namespace facebook