From 5f37483466b32bb9d09945e25c04b1cb8add9679 Mon Sep 17 00:00:00 2001 From: Martin Kralik Date: Thu, 6 Apr 2017 05:50:28 -0700 Subject: [PATCH] Revert D4840716: Refactor interfaces Differential Revision: D4840716 fbshipit-source-id: 1b6a6050d78ccbbd3c817621df1c1c989594fdb1 --- ReactAndroid/src/main/jni/xreact/jni/BUCK | 1 + .../src/main/jni/xreact/jni/JInspector.cpp | 19 +++---- .../src/main/jni/xreact/jni/JInspector.h | 12 ++-- .../src/main/jni/xreact/jni/OnLoad.cpp | 4 +- ReactCommon/cxxreact/BUCK | 1 + ReactCommon/cxxreact/JSCExecutor.cpp | 29 ++++------ ReactCommon/jschelpers/BUCK | 1 - ReactCommon/jschelpers/InspectorInterfaces.h | 55 ------------------- ReactCommon/jschelpers/JSCWrapper.h | 28 +++------- ReactCommon/jschelpers/JavaScriptCore.h | 4 -- ReactCommon/jschelpers/systemJSCWrapper.cpp | 7 --- 11 files changed, 35 insertions(+), 126 deletions(-) delete mode 100644 ReactCommon/jschelpers/InspectorInterfaces.h diff --git a/ReactAndroid/src/main/jni/xreact/jni/BUCK b/ReactAndroid/src/main/jni/xreact/jni/BUCK index 3f005f5578a..344814039e0 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/BUCK +++ b/ReactAndroid/src/main/jni/xreact/jni/BUCK @@ -39,6 +39,7 @@ cxx_library( preprocessor_flags = [ "-DLOG_TAG=\"ReactNativeJNI\"", "-DWITH_FBSYSTRACE=1", + "-DWITH_INSPECTOR=1", ], soname = "libreactnativejnifb.$(ext)", visibility = [ diff --git a/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp b/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp index 5da21db08ad..cad09b1cc00 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp @@ -1,25 +1,24 @@ // Copyright 2004-present Facebook. All Rights Reserved. #include "JInspector.h" -#include -#ifdef WITH_FBJSCEXTENSIONS +#ifdef WITH_INSPECTOR namespace facebook { namespace react { namespace { -class RemoteConnection : public IRemoteConnection { +class RemoteConnection : public Inspector::RemoteConnection { public: RemoteConnection(jni::alias_ref connection) : connection_(jni::make_global(connection)) {} - virtual void onMessage(std::string message) override { + void onMessage(std::string message) override { connection_->onMessage(message); } - virtual void onDisconnect() override { + void onDisconnect() override { connection_->onDisconnect(); } private: @@ -43,7 +42,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) { @@ -61,17 +60,13 @@ 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(&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 d28523d0935..0b9db509a75 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JInspector.h +++ b/ReactAndroid/src/main/jni/xreact/jni/JInspector.h @@ -2,9 +2,9 @@ #pragma once -#ifdef WITH_FBJSCEXTENSIONS +#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(IInspector* inspector) : inspector_(inspector) {} + JInspector(Inspector* inspector) : inspector_(inspector) {} - IInspector* inspector_; + Inspector* inspector_; }; } diff --git a/ReactAndroid/src/main/jni/xreact/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/xreact/jni/OnLoad.cpp index 6133675dffd..7be58ad9fb1 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/OnLoad.cpp @@ -18,7 +18,7 @@ #include "JCallback.h" #include "JSLogging.h" -#ifdef WITH_FBJSCEXTENSIONS +#ifdef WITH_INSPECTOR #include "JInspector.h" #endif @@ -171,7 +171,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { CxxModuleWrapperBase::registerNatives(); CxxModuleWrapper::registerNatives(); JCallbackImpl::registerNatives(); - #ifdef WITH_FBJSCEXTENSIONS + #ifdef WITH_INSPECTOR JInspector::registerNatives(); #endif diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index 127c5e3d677..65cafc15002 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/ReactCommon/cxxreact/BUCK @@ -39,6 +39,7 @@ if THIS_IS_FBANDROID: '-DWITH_JSC_MEMORY_PRESSURE=1', '-DWITH_REACT_INTERNAL_SETTINGS=1', '-DWITH_FB_MEMORY_PROFILING=1', + '-DWITH_INSPECTOR=1', ], deps = JSC_DEPS, visibility = [ diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 3027200d946..810d0ecfcf5 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -18,7 +18,10 @@ #include #include -#include + +#ifdef WITH_INSPECTOR +#include +#endif #include "JSBundleType.h" #include "Platform.h" @@ -217,16 +220,6 @@ void JSCExecutor::setContextName(const std::string& name) { JSC_JSGlobalContextSetName(m_context, jsName); } -static bool canUseInspector(JSContextRef context) { -#if defined(__APPLE__) - return isCustomJSCPtr(context); -#elif defined(WITH_FBJSCEXTENSIONS) - return true; -#else - return false; -#endif -} - void JSCExecutor::initOnJSVMThread() throw(JSException) { SystraceSection s("JSCExecutor.initOnJSVMThread"); @@ -260,10 +253,9 @@ 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); - if (canUseInspector(m_context)) { - IInspector* pInspector = JSC_JSInspectorGetInstance(true); - pInspector->registerGlobalContext("main", m_context); - } + #ifdef WITH_INSPECTOR + Inspector::instance().registerGlobalContext("main", m_context); + #endif installNativeHook<&JSCExecutor::nativeFlushQueueImmediate>("nativeFlushQueueImmediate"); installNativeHook<&JSCExecutor::nativeCallSyncHook>("nativeCallSyncHook"); @@ -319,10 +311,9 @@ void JSCExecutor::terminateOnJSVMThread() { m_nativeModules.reset(); - if (canUseInspector(m_context)) { - IInspector* pInspector = JSC_JSInspectorGetInstance(true); - pInspector->unregisterGlobalContext(m_context); - } + #ifdef WITH_INSPECTOR + Inspector::instance().unregisterGlobalContext(m_context); + #endif JSC_JSGlobalContextRelease(m_context); m_context = nullptr; diff --git a/ReactCommon/jschelpers/BUCK b/ReactCommon/jschelpers/BUCK index af29360f753..ef759f4ca02 100644 --- a/ReactCommon/jschelpers/BUCK +++ b/ReactCommon/jschelpers/BUCK @@ -1,5 +1,4 @@ EXPORTED_HEADERS = [ - "InspectorInterfaces.h", "JavaScriptCore.h", "JSCHelpers.h", "JSCWrapper.h", diff --git a/ReactCommon/jschelpers/InspectorInterfaces.h b/ReactCommon/jschelpers/InspectorInterfaces.h deleted file mode 100644 index e19eae5d92f..00000000000 --- a/ReactCommon/jschelpers/InspectorInterfaces.h +++ /dev/null @@ -1,55 +0,0 @@ -/** - * 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 736c8407023..c1a6d0cc12a 100644 --- a/ReactCommon/jschelpers/JSCWrapper.h +++ b/ReactCommon/jschelpers/JSCWrapper.h @@ -15,29 +15,19 @@ #include #endif -#include - -#if JSCINTERNAL || (!defined(__APPLE__)) -#define JSC_IMPORT extern "C" -#else -#define JSC_IMPORT extern -#endif - -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 * @@ -124,8 +114,6 @@ 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 f0e4423ff6b..4d849883476 100644 --- a/ReactCommon/jschelpers/JavaScriptCore.h +++ b/ReactCommon/jschelpers/JavaScriptCore.h @@ -182,10 +182,6 @@ 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 a3612483892..80000c3fa9a 100644 --- a/ReactCommon/jschelpers/systemJSCWrapper.cpp +++ b/ReactCommon/jschelpers/systemJSCWrapper.cpp @@ -28,9 +28,6 @@ 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() { @@ -122,10 +119,6 @@ const JSCWrapper* systemJSCWrapper() { (decltype(&JSStartSamplingProfilingOnMainJSCThread)) Unimplemented_JSStartSamplingProfilingOnMainJSCThread, - .JSInspectorGetInstance = - (decltype(&JSInspectorGetInstance)) - Unimplemented_JSInspectorGetInstance, - .configureJSCForIOS = (decltype(&configureJSCForIOS))Unimplemented_configureJSCForIOS,