From ac74d2a38b000749d73c4b660cf4dfcb0c940832 Mon Sep 17 00:00:00 2001 From: Theo Yaung Date: Tue, 2 May 2017 21:29:04 -0700 Subject: [PATCH] Refactor interfaces Reviewed By: johnislarry Differential Revision: D4844786 fbshipit-source-id: f348b8d5078643636343f6ea099b200f519fbc40 --- .../src/main/jni/xreact/jni/JInspector.cpp | 17 ++++-- .../src/main/jni/xreact/jni/JInspector.h | 10 ++-- ReactCommon/cxxreact/BUCK | 9 ++- ReactCommon/cxxreact/JSCExecutor.cpp | 30 +++++++--- ReactCommon/jschelpers/BUCK | 1 + ReactCommon/jschelpers/InspectorInterfaces.h | 55 +++++++++++++++++++ ReactCommon/jschelpers/JSCWrapper.h | 33 ++++++++--- ReactCommon/jschelpers/JavaScriptCore.h | 4 ++ ReactCommon/jschelpers/systemJSCWrapper.cpp | 7 +++ 9 files changed, 139 insertions(+), 27 deletions(-) create mode 100644 ReactCommon/jschelpers/InspectorInterfaces.h diff --git a/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp b/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp index cad09b1cc00..62f0cb66d2a 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp @@ -1,6 +1,7 @@ // Copyright 2004-present Facebook. All Rights Reserved. #include "JInspector.h" +#include #ifdef WITH_INSPECTOR @@ -9,16 +10,16 @@ namespace react { namespace { -class RemoteConnection : public Inspector::RemoteConnection { +class RemoteConnection : public IRemoteConnection { public: RemoteConnection(jni::alias_ref connection) : connection_(jni::make_global(connection)) {} - void onMessage(std::string message) override { + virtual void onMessage(std::string message) override { connection_->onMessage(message); } - void onDisconnect() override { + virtual void onDisconnect() override { connection_->onDisconnect(); } private: @@ -42,7 +43,7 @@ void JRemoteConnection::onDisconnect() const { method(self()); } -JLocalConnection::JLocalConnection(std::unique_ptr connection) +JLocalConnection::JLocalConnection(std::unique_ptr connection) : connection_(std::move(connection)) {} void JLocalConnection::sendMessage(std::string message) { @@ -60,13 +61,17 @@ 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(&Inspector::instance())); + static auto instance = jni::make_global(newObjectCxxArgs(getInspectorInstance()/*&Inspector::instance()*/)); return instance; } jni::local_ref> JInspector::getPages() { - std::vector pages = inspector_->getPages(); + std::vector pages = inspector_->getPages(); auto array = jni::JArrayClass::newArray(pages.size()); for (size_t i = 0; i < pages.size(); i++) { (*array)[i] = JPage::create(pages[i].id, pages[i].title); diff --git a/ReactAndroid/src/main/jni/xreact/jni/JInspector.h b/ReactAndroid/src/main/jni/xreact/jni/JInspector.h index 0b9db509a75..fa703bbf439 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JInspector.h +++ b/ReactAndroid/src/main/jni/xreact/jni/JInspector.h @@ -4,7 +4,7 @@ #ifdef WITH_INSPECTOR -#include +#include #include #include @@ -31,14 +31,14 @@ class JLocalConnection : public jni::HybridClass { public: static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/Inspector$LocalConnection;"; - JLocalConnection(std::unique_ptr connection); + JLocalConnection(std::unique_ptr connection); void sendMessage(std::string message); void disconnect(); static void registerNatives(); private: - std::unique_ptr connection_; + std::unique_ptr connection_; }; class JInspector : public jni::HybridClass { @@ -54,9 +54,9 @@ public: private: friend HybridBase; - JInspector(Inspector* inspector) : inspector_(inspector) {} + JInspector(IInspector* inspector) : inspector_(inspector) {} - Inspector* inspector_; + IInspector* inspector_; }; } diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index 12f3976f2d2..032fee2f0c1 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/ReactCommon/cxxreact/BUCK @@ -3,6 +3,7 @@ CXX_LIBRARY_COMPILER_FLAGS = [] REACT_LIBRARY_EXTRA_COMPILER_FLAGS = [] if THIS_IS_FBOBJC: + include_defs("xplat//configurations/buck/apple/dependency_aggregator_defs") inherited_buck_flags = STATIC_LIBRARY_IOS_FLAGS CXX_LIBRARY_COMPILER_FLAGS = inherited_buck_flags.get_flag_value('compiler_flags') REACT_LIBRARY_EXTRA_COMPILER_FLAGS = ['-Wno-shadow', '-Wno-missing-prototypes', '-Wno-global-constructors'] @@ -62,6 +63,12 @@ if THIS_IS_FBANDROID: ) if THIS_IS_FBOBJC: + + if should_use_dependency_aggregation(): + INSPECTOR_FLAGS = [] + else: + INSPECTOR_FLAGS = [ '-DWITH_INSPECTOR=1', ] + def react_library(**kwargs): fb_apple_library( name = 'bridge', @@ -75,7 +82,7 @@ if THIS_IS_FBOBJC: ], **kwargs_add( kwargs, - preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS, + preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS + INSPECTOR_FLAGS, deps = [ '//xplat/folly:molly', ], diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 4c180a538cd..8c86db591b4 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -20,7 +20,7 @@ #include #ifdef WITH_INSPECTOR -#include +#include #endif #include "JSBundleType.h" @@ -171,6 +171,16 @@ void JSCExecutor::setContextName(const std::string& name) { JSC_JSGlobalContextSetName(m_context, jsName); } +#ifdef WITH_INSPECTOR +static bool canUseInspector(JSContextRef context) { +#if defined(__APPLE__) + return isCustomJSCPtr(context); // WITH_INSPECTOR && Apple +#else + return true; // WITH_INSPECTOR && Android +#endif +} +#endif + void JSCExecutor::initOnJSVMThread() throw(JSException) { SystraceSection s("JSCExecutor.initOnJSVMThread"); @@ -204,9 +214,12 @@ void JSCExecutor::initOnJSVMThread() throw(JSException) { // Add a pointer to ourselves so we can retrieve it later in our hooks Object::getGlobalObject(m_context).setPrivate(this); - #ifdef WITH_INSPECTOR - Inspector::instance().registerGlobalContext("main", m_context); - #endif +#ifdef WITH_INSPECTOR + if (canUseInspector(m_context)) { + IInspector* pInspector = JSC_JSInspectorGetInstance(true); + pInspector->registerGlobalContext("main", m_context); + } +#endif installNativeHook<&JSCExecutor::nativeFlushQueueImmediate>("nativeFlushQueueImmediate"); installNativeHook<&JSCExecutor::nativeCallSyncHook>("nativeCallSyncHook"); @@ -247,9 +260,12 @@ void JSCExecutor::initOnJSVMThread() throw(JSException) { void JSCExecutor::terminateOnJSVMThread() { m_nativeModules.reset(); - #ifdef WITH_INSPECTOR - Inspector::instance().unregisterGlobalContext(m_context); - #endif +#ifdef WITH_INSPECTOR + if (canUseInspector(m_context)) { + IInspector* pInspector = JSC_JSInspectorGetInstance(true); + pInspector->unregisterGlobalContext(m_context); + } +#endif JSC_JSGlobalContextRelease(m_context); m_context = nullptr; diff --git a/ReactCommon/jschelpers/BUCK b/ReactCommon/jschelpers/BUCK index ef759f4ca02..af29360f753 100644 --- a/ReactCommon/jschelpers/BUCK +++ b/ReactCommon/jschelpers/BUCK @@ -1,4 +1,5 @@ EXPORTED_HEADERS = [ + "InspectorInterfaces.h", "JavaScriptCore.h", "JSCHelpers.h", "JSCWrapper.h", diff --git a/ReactCommon/jschelpers/InspectorInterfaces.h b/ReactCommon/jschelpers/InspectorInterfaces.h new file mode 100644 index 00000000000..e19eae5d92f --- /dev/null +++ b/ReactCommon/jschelpers/InspectorInterfaces.h @@ -0,0 +1,55 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#pragma once + +#include +#include +#include +#include + +namespace facebook { +namespace react { + +class IDestructible { +public: + virtual ~IDestructible() = 0; +}; + +struct InspectorPage { + const int id; + const std::string title; +}; + +class IRemoteConnection : public IDestructible { +public: + virtual ~IRemoteConnection() = 0; + virtual void onMessage(std::string message) = 0; + virtual void onDisconnect() = 0; +}; + +class ILocalConnection : public IDestructible { +public: + virtual ~ILocalConnection() = 0; + virtual void sendMessage(std::string message) = 0; + virtual void disconnect() = 0; +}; + +// Note: not destructible! +class IInspector { +public: + virtual void registerGlobalContext(std::string title, JSGlobalContextRef ctx) = 0; + virtual void unregisterGlobalContext(JSGlobalContextRef ctx) = 0; + + virtual std::vector getPages() const = 0; + virtual std::unique_ptr connect(int pageId, std::unique_ptr remote) = 0; +}; + +} +} diff --git a/ReactCommon/jschelpers/JSCWrapper.h b/ReactCommon/jschelpers/JSCWrapper.h index c1a6d0cc12a..03d2f3bdad9 100644 --- a/ReactCommon/jschelpers/JSCWrapper.h +++ b/ReactCommon/jschelpers/JSCWrapper.h @@ -9,25 +9,40 @@ #pragma once +#include #include #if WITH_FBJSCEXTENSIONS #include #endif +#if defined(JSCINTERNAL) || (!defined(__APPLE__)) +#define JSC_IMPORT extern "C" +#else +#define JSC_IMPORT extern +#endif + +namespace facebook { +namespace react { + class IInspector; +} +} + +JSC_IMPORT facebook::react::IInspector* JSInspectorGetInstance(); + +// This is used to substitute an alternate JSC implementation for +// testing. These calls must all be ABI compatible with the standard JSC. +JSC_IMPORT void configureJSCForIOS(std::string); // TODO: replace with folly::dynamic once supported +JSC_IMPORT JSValueRef JSEvaluateBytecodeBundle(JSContextRef, JSObjectRef, int, JSStringRef, JSValueRef*); +JSC_IMPORT bool JSSamplingProfilerEnabled(); +JSC_IMPORT void JSStartSamplingProfilingOnMainJSCThread(JSGlobalContextRef); +JSC_IMPORT JSValueRef JSPokeSamplingProfiler(JSContextRef); + #if defined(__APPLE__) #import #import #import -// This is used to substitute an alternate JSC implementation for -// testing. These calls must all be ABI compatible with the standard JSC. -extern void configureJSCForIOS(std::string); // TODO: replace with folly::dynamic once supported -extern JSValueRef JSEvaluateBytecodeBundle(JSContextRef, JSObjectRef, int, JSStringRef, JSValueRef*); -extern bool JSSamplingProfilerEnabled(); -extern void JSStartSamplingProfilingOnMainJSCThread(JSGlobalContextRef); -extern JSValueRef JSPokeSamplingProfiler(JSContextRef); - /** * JSNoBytecodeFileFormatVersion * @@ -114,6 +129,8 @@ struct JSCWrapper { JSC_WRAPPER_METHOD(JSPokeSamplingProfiler); JSC_WRAPPER_METHOD(JSStartSamplingProfilingOnMainJSCThread); + JSC_WRAPPER_METHOD(JSInspectorGetInstance); + JSC_WRAPPER_METHOD(configureJSCForIOS); // Objective-C API diff --git a/ReactCommon/jschelpers/JavaScriptCore.h b/ReactCommon/jschelpers/JavaScriptCore.h index 4d849883476..f0e4423ff6b 100644 --- a/ReactCommon/jschelpers/JavaScriptCore.h +++ b/ReactCommon/jschelpers/JavaScriptCore.h @@ -182,6 +182,10 @@ jsc_poison(JSObjectMakeArrayBufferWithBytesNoCopy JSObjectMakeTypedArray jsc_poison(JSSamplingProfilerEnabled JSPokeSamplingProfiler JSStartSamplingProfilingOnMainJSCThread) +#define JSC_JSInspectorGetInstance(...) __jsc_bool_wrapper(JSInspectorGetInstance, __VA_ARGS__) +jsc_poison(JSInspectorGetInstance) + + #define JSC_configureJSCForIOS(...) __jsc_bool_wrapper(configureJSCForIOS, __VA_ARGS__) jsc_poison(configureJSCForIOS) diff --git a/ReactCommon/jschelpers/systemJSCWrapper.cpp b/ReactCommon/jschelpers/systemJSCWrapper.cpp index 80000c3fa9a..a3612483892 100644 --- a/ReactCommon/jschelpers/systemJSCWrapper.cpp +++ b/ReactCommon/jschelpers/systemJSCWrapper.cpp @@ -28,6 +28,9 @@ UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSStringCreateWithUTF8CStringExpectAscii) #endif UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSPokeSamplingProfiler) UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSStartSamplingProfilingOnMainJSCThread) + +UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(JSInspectorGetInstance) + UNIMPLEMENTED_SYSTEM_JSC_FUNCTION(configureJSCForIOS) bool JSSamplingProfilerEnabled() { @@ -119,6 +122,10 @@ const JSCWrapper* systemJSCWrapper() { (decltype(&JSStartSamplingProfilingOnMainJSCThread)) Unimplemented_JSStartSamplingProfilingOnMainJSCThread, + .JSInspectorGetInstance = + (decltype(&JSInspectorGetInstance)) + Unimplemented_JSInspectorGetInstance, + .configureJSCForIOS = (decltype(&configureJSCForIOS))Unimplemented_configureJSCForIOS,