Revert D4840716: Refactor interfaces

Differential Revision: D4840716

fbshipit-source-id: 1b6a6050d78ccbbd3c817621df1c1c989594fdb1

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 = [

ReactAndroid/src/main/jni/xreact/jni/JInspector.cpp

@@ -1,25 +1,24 @@
 // Copyright 2004-present Facebook. All Rights Reserved.
 
 #include "JInspector.h"
-#include <jschelpers/JavaScriptCore.h>
 
-#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<JRemoteConnection::javaobject> 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<ILocalConnection> connection)
+JLocalConnection::JLocalConnection(std::unique_ptr<Inspector::LocalConnection> 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::javaobject> JInspector::instance(jni::alias_ref<jclass>) {
-  static auto instance = jni::make_global(newObjectCxxArgs(getInspectorInstance()/*&Inspector::instance()*/));
+  static auto instance = jni::make_global(newObjectCxxArgs(&Inspector::instance()));
   return instance;
 }
 
 jni::local_ref<jni::JArrayClass<JPage::javaobject>> JInspector::getPages() {
-  std::vector<InspectorPage> pages = inspector_->getPages();
+  std::vector<Inspector::Page> pages = inspector_->getPages();
   auto array = jni::JArrayClass<JPage::javaobject>::newArray(pages.size());
   for (size_t i = 0; i < pages.size(); i++) {
     (*array)[i] = JPage::create(pages[i].id, pages[i].title);

ReactAndroid/src/main/jni/xreact/jni/JInspector.h

@@ -2,9 +2,9 @@
 
 #pragma once
 
-#ifdef WITH_FBJSCEXTENSIONS
+#ifdef WITH_INSPECTOR
 
-#include <jschelpers/InspectorInterfaces.h>
+#include <inspector/Inspector.h>
 
 #include <fb/fbjni.h>
 #include <folly/Memory.h>
@@ -31,14 +31,14 @@ class JLocalConnection : public jni::HybridClass<JLocalConnection> {
 public:
   static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/Inspector$LocalConnection;";
 
-  JLocalConnection(std::unique_ptr<ILocalConnection> connection);
+  JLocalConnection(std::unique_ptr<Inspector::LocalConnection> connection);
 
   void sendMessage(std::string message);
   void disconnect();
 
   static void registerNatives();
 private:
-  std::unique_ptr<ILocalConnection> connection_;
+  std::unique_ptr<Inspector::LocalConnection> connection_;
 };
 
 class JInspector : public jni::HybridClass<JInspector> {
@@ -54,9 +54,9 @@ public:
 private:
   friend HybridBase;
 
-  JInspector(IInspector* inspector) : inspector_(inspector) {}
+  JInspector(Inspector* inspector) : inspector_(inspector) {}
 
-  IInspector* inspector_;
+  Inspector* inspector_;
 };
 
 }

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
 

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 = [

ReactCommon/cxxreact/JSCExecutor.cpp

@@ -18,7 +18,10 @@
 
 #include <jschelpers/JSCHelpers.h>
 #include <jschelpers/Value.h>
-#include <jschelpers/InspectorInterfaces.h>
+
+#ifdef WITH_INSPECTOR
+#include <inspector/Inspector.h>
+#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)) {
+  #ifdef WITH_INSPECTOR
-    IInspector* pInspector = JSC_JSInspectorGetInstance(true);
+  Inspector::instance().registerGlobalContext("main", m_context);
-    pInspector->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)) {
+  #ifdef WITH_INSPECTOR
-    IInspector* pInspector = JSC_JSInspectorGetInstance(true);
+  Inspector::instance().unregisterGlobalContext(m_context);
-    pInspector->unregisterGlobalContext(m_context);
+  #endif
-  }
 
   JSC_JSGlobalContextRelease(m_context);
   m_context = nullptr;

ReactCommon/jschelpers/BUCK

@@ -1,5 +1,4 @@
 EXPORTED_HEADERS = [
-    "InspectorInterfaces.h",
     "JavaScriptCore.h",
     "JSCHelpers.h",
     "JSCWrapper.h",

ReactCommon/jschelpers/InspectorInterfaces.h

@@ -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 <memory>
-#include <string>
-#include <vector>
-#include <JavaScriptCore/JSBase.h>
-
-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<InspectorPage> getPages() const = 0;
-  virtual std::unique_ptr<ILocalConnection> connect(int pageId, std::unique_ptr<IRemoteConnection> remote) = 0;
-};
-
-}
-}

ReactCommon/jschelpers/JSCWrapper.h

@@ -15,29 +15,19 @@
 #include <jsc_stringref.h>
 #endif
 
-#include <jschelpers/InspectorInterfaces.h>
-
-#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 <objc/objc.h>
 #import <JavaScriptCore/JSStringRefCF.h>
 #import <string>
 
+// 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

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)

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,