From 75ca46e332269da3da3380d54bad2637bd7f0806 Mon Sep 17 00:00:00 2001 From: Chris Hopman Date: Fri, 5 Feb 2016 18:09:24 -0800 Subject: [PATCH] Inject some behavior from react/jni/ to react/ Summary: So, this makes it so a set of behaviors that require accessing java can be injected from the jni/ folder. The behaviors are logging, perf logging, log markers and loading script from assets. I'd argue that these should all actually be encapsulated by interfaces that are passed to the JSCExecutor/others (and I'd say that's regardless of whether they are injected from jni/ or not), but I wanted to stick to the least disruptive pattern for these changes. public Reviewed By: astreet Differential Revision: D2905168 fb-gh-sync-id: 7c8c16cb77b8fc3d42750dacc6574259ad512ac2 --- ReactAndroid/src/main/jni/react/BUCK | 6 +-- .../src/main/jni/react/JSCExecutor.cpp | 51 +++---------------- .../src/main/jni/react/JSCWebWorker.cpp | 3 +- ReactAndroid/src/main/jni/react/Platform.cpp | 24 +++++++++ ReactAndroid/src/main/jni/react/Platform.h | 39 ++++++++++++++ ReactAndroid/src/main/jni/react/jni/BUCK | 5 ++ .../jni/react/{ => jni}/JSCPerfLogging.cpp | 4 +- .../main/jni/react/{ => jni}/JSCPerfLogging.h | 0 .../src/main/jni/react/jni/JSLoader.cpp | 6 +-- .../src/main/jni/react/jni/JSLoader.h | 6 +-- .../src/main/jni/react/jni/JSLogging.cpp | 36 +++++++++++++ .../src/main/jni/react/jni/JSLogging.h | 15 ++++++ .../src/main/jni/react/jni/OnLoad.cpp | 21 ++++++++ 13 files changed, 160 insertions(+), 56 deletions(-) create mode 100644 ReactAndroid/src/main/jni/react/Platform.cpp create mode 100644 ReactAndroid/src/main/jni/react/Platform.h rename ReactAndroid/src/main/jni/react/{ => jni}/JSCPerfLogging.cpp (99%) rename ReactAndroid/src/main/jni/react/{ => jni}/JSCPerfLogging.h (100%) create mode 100644 ReactAndroid/src/main/jni/react/jni/JSLogging.cpp create mode 100644 ReactAndroid/src/main/jni/react/jni/JSLogging.h diff --git a/ReactAndroid/src/main/jni/react/BUCK b/ReactAndroid/src/main/jni/react/BUCK index 10b22bdcd..aebe0af68 100644 --- a/ReactAndroid/src/main/jni/react/BUCK +++ b/ReactAndroid/src/main/jni/react/BUCK @@ -46,15 +46,14 @@ react_library( 'JSCExecutor.cpp', 'JSCTracing.cpp', 'JSCMemory.cpp', - 'JSCPerfLogging.cpp', 'JSCLegacyProfiler.cpp', 'JSCWebWorker.cpp', + 'Platform.cpp', ], headers = [ 'JSCTracing.h', - 'JSCPerfLogging.h', 'JSCLegacyProfiler.h', - 'JSCMemory.h' + 'JSCMemory.h', ], exported_headers = [ 'Bridge.h', @@ -65,6 +64,7 @@ react_library( 'MethodCall.h', 'JSModulesUnbundle.h', 'Value.h', + 'Platform.h', ], compiler_flags = [ '-Wall', diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp index f62ec6e56..ae1aa7a4f 100644 --- a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp +++ b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp @@ -15,6 +15,7 @@ #include "jni/JMessageQueueThread.h" #include "jni/OnLoad.h" #include +#include "Platform.h" #ifdef WITH_JSC_EXTRA_TRACING #include @@ -31,9 +32,6 @@ using fbsystrace::FbSystraceSection; #endif -// Add native performance markers support -#include - #ifdef WITH_FB_MEMORY_PROFILING #include #endif @@ -59,13 +57,6 @@ static JSValueRef nativeFlushQueueImmediate( size_t argumentCount, const JSValueRef arguments[], JSValueRef *exception); -static JSValueRef nativeLoggingHook( - JSContextRef ctx, - JSObjectRef function, - JSObjectRef thisObject, - size_t argumentCount, - const JSValueRef arguments[], - JSValueRef *exception); static JSValueRef nativePerformanceNow( JSContextRef ctx, JSObjectRef function, @@ -103,12 +94,13 @@ JSCExecutor::JSCExecutor(FlushImmediateCallback cb) : m_messageQueueThread = JMessageQueueThread::currentMessageQueueThread(); s_globalContextRefToJSCExecutor[m_context] = this; installGlobalFunction(m_context, "nativeFlushQueueImmediate", nativeFlushQueueImmediate); - installGlobalFunction(m_context, "nativeLoggingHook", nativeLoggingHook); installGlobalFunction(m_context, "nativePerformanceNow", nativePerformanceNow); installGlobalFunction(m_context, "nativeStartWorker", nativeStartWorker); installGlobalFunction(m_context, "nativePostMessageToWorker", nativePostMessageToWorker); installGlobalFunction(m_context, "nativeTerminateWorker", nativeTerminateWorker); + installGlobalFunction(m_context, "nativeLoggingHook", JSLogging::nativeHook); + #ifdef WITH_FB_JSC_TUNING configureJSCForAndroid(); #endif @@ -116,7 +108,7 @@ JSCExecutor::JSCExecutor(FlushImmediateCallback cb) : #ifdef WITH_JSC_EXTRA_TRACING addNativeTracingHooks(m_context); addNativeProfilingHooks(m_context); - addNativePerfLoggingHooks(m_context); + PerfLogging::installNativeHooks(m_context); #endif #ifdef WITH_FB_MEMORY_PROFILING @@ -164,17 +156,9 @@ std::string JSCExecutor::getDeviceCacheDir(){ void JSCExecutor::executeApplicationScript( const std::string& script, const std::string& sourceURL) { - JNIEnv* env = Environment::current(); - jclass markerClass = env->FindClass("com/facebook/react/bridge/ReactMarker"); - jmethodID logMarkerMethod = facebook::react::getLogMarkerMethod(); - jstring startStringMarker = env->NewStringUTF("executeApplicationScript_startStringConvert"); - jstring endStringMarker = env->NewStringUTF("executeApplicationScript_endStringConvert"); - - env->CallStaticVoidMethod(markerClass, logMarkerMethod, startStringMarker); + ReactMarker::logMarker("executeApplicationScript_startStringConvert"); String jsScript = String::createExpectingAscii(script); - env->CallStaticVoidMethod(markerClass, logMarkerMethod, endStringMarker); - env->DeleteLocalRef(startStringMarker); - env->DeleteLocalRef(endStringMarker); + ReactMarker::logMarker("executeApplicationScript_endStringConvert"); String jsSourceURL(sourceURL.c_str()); #ifdef WITH_FBSYSTRACE @@ -504,29 +488,6 @@ JSValueRef JSCExecutor::nativeTerminateWorker( return JSValueMakeUndefined(ctx); } -static JSValueRef nativeLoggingHook( - JSContextRef ctx, - JSObjectRef function, - JSObjectRef thisObject, - size_t argumentCount, - const JSValueRef arguments[], JSValueRef *exception) { - android_LogPriority logLevel = ANDROID_LOG_DEBUG; - if (argumentCount > 1) { - int level = (int) JSValueToNumber(ctx, arguments[1], NULL); - // The lowest log level we get from JS is 0. We shift and cap it to be - // in the range the Android logging method expects. - logLevel = std::min( - static_cast(level + ANDROID_LOG_DEBUG), - ANDROID_LOG_FATAL); - } - if (argumentCount > 0) { - JSStringRef jsString = JSValueToStringCopy(ctx, arguments[0], NULL); - String message = String::adopt(jsString); - FBLOG_PRI(logLevel, "ReactNativeJS", "%s", message.str().c_str()); - } - return JSValueMakeUndefined(ctx); -} - static JSValueRef nativePerformanceNow( JSContextRef ctx, JSObjectRef function, diff --git a/ReactAndroid/src/main/jni/react/JSCWebWorker.cpp b/ReactAndroid/src/main/jni/react/JSCWebWorker.cpp index 961ac8269..27ba10114 100644 --- a/ReactAndroid/src/main/jni/react/JSCWebWorker.cpp +++ b/ReactAndroid/src/main/jni/react/JSCWebWorker.cpp @@ -16,6 +16,7 @@ #include "jni/JMessageQueueThread.h" #include "jni/JSLoader.h" #include "jni/WebWorkers.h" +#include "Platform.h" #include "Value.h" #include @@ -109,7 +110,7 @@ void JSCWebWorker::initJSVMAndLoadScript() { s_globalContextRefToJSCWebWorker[context_] = this; // TODO(9604438): Protect against script does not exist - std::string script = loadScriptFromAssets(scriptName_); + std::string script = WebWorkerUtil::loadScriptFromAssets(scriptName_); evaluateScript(context_, String(script.c_str()), String(scriptName_.c_str())); installGlobalFunction(context_, "postMessage", nativePostMessage); diff --git a/ReactAndroid/src/main/jni/react/Platform.cpp b/ReactAndroid/src/main/jni/react/Platform.cpp new file mode 100644 index 000000000..be5caf546 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/Platform.cpp @@ -0,0 +1,24 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "Platform.h" + +namespace facebook { +namespace react { + +namespace ReactMarker { +LogMarker logMarker; +}; + +namespace WebWorkerUtil { +LoadScriptFromAssets loadScriptFromAssets; +}; + +namespace PerfLogging { +InstallNativeHooks installNativeHooks; +} + +namespace JSLogging { +JSCNativeHook nativeHook = nullptr; +} + +} } diff --git a/ReactAndroid/src/main/jni/react/Platform.h b/ReactAndroid/src/main/jni/react/Platform.h new file mode 100644 index 000000000..3de8b4b6b --- /dev/null +++ b/ReactAndroid/src/main/jni/react/Platform.h @@ -0,0 +1,39 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include +#include +#include + +#include + +namespace facebook { +namespace react { + +namespace ReactMarker { +using LogMarker = std::function; +extern LogMarker logMarker; +}; + +namespace WebWorkerUtil { +using LoadScriptFromAssets = std::function; +extern LoadScriptFromAssets loadScriptFromAssets; +}; + +namespace PerfLogging { +using InstallNativeHooks = std::function; +extern InstallNativeHooks installNativeHooks; +} + +namespace JSLogging { + using JSCNativeHook = JSValueRef (*) ( + JSContextRef ctx, + JSObjectRef function, + JSObjectRef thisObject, + size_t argumentCount, + const JSValueRef arguments[], JSValueRef *exception); + extern JSCNativeHook nativeHook; +} + +} } diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index d29ea7dc6..32d3f3f90 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -5,6 +5,7 @@ SUPPORTED_PLATFORMS = '^android-(armv7|x86)$' DEPS = [ '//native/jni:jni', + '//native/third-party/android-ndk:android', '//xplat/folly:molly', ] @@ -28,14 +29,18 @@ jni_library( supported_platforms_regex = SUPPORTED_PLATFORMS, srcs = [ 'JMessageQueueThread.cpp', + 'JSCPerfLogging.cpp', 'JSLoader.cpp', 'NativeArray.cpp', 'OnLoad.cpp', 'ProxyExecutor.cpp', + 'JSLogging.cpp', ], headers = [ 'JSLoader.h', 'ProxyExecutor.h', + 'JSCPerfLogging.h', + 'JSLogging.h', ], exported_headers = [ 'JMessageQueueThread.h', diff --git a/ReactAndroid/src/main/jni/react/JSCPerfLogging.cpp b/ReactAndroid/src/main/jni/react/jni/JSCPerfLogging.cpp similarity index 99% rename from ReactAndroid/src/main/jni/react/JSCPerfLogging.cpp rename to ReactAndroid/src/main/jni/react/jni/JSCPerfLogging.cpp index 3700b14c0..d480a8833 100644 --- a/ReactAndroid/src/main/jni/react/JSCPerfLogging.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JSCPerfLogging.cpp @@ -1,8 +1,10 @@ // Copyright 2004-present Facebook. All Rights Reserved. -#include "JSCHelpers.h" +#include "JSCPerfLogging.h" + #include #include +#include using namespace facebook::jni; diff --git a/ReactAndroid/src/main/jni/react/JSCPerfLogging.h b/ReactAndroid/src/main/jni/react/jni/JSCPerfLogging.h similarity index 100% rename from ReactAndroid/src/main/jni/react/JSCPerfLogging.h rename to ReactAndroid/src/main/jni/react/jni/JSCPerfLogging.h diff --git a/ReactAndroid/src/main/jni/react/jni/JSLoader.cpp b/ReactAndroid/src/main/jni/react/jni/JSLoader.cpp index 24937d51b..a96d0c3de 100644 --- a/ReactAndroid/src/main/jni/react/jni/JSLoader.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JSLoader.cpp @@ -21,7 +21,7 @@ static jclass gApplicationHolderClass; static jmethodID gGetApplicationMethod; static jmethodID gGetAssetManagerMethod; -std::string loadScriptFromAssets(std::string assetName) { +std::string loadScriptFromAssets(const std::string& assetName) { JNIEnv *env = jni::Environment::current(); jobject application = env->CallStaticObjectMethod( gApplicationHolderClass, @@ -32,7 +32,7 @@ std::string loadScriptFromAssets(std::string assetName) { std::string loadScriptFromAssets( AAssetManager *manager, - std::string assetName) { + const std::string& assetName) { #ifdef WITH_FBSYSTRACE FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "reactbridge_jni_loadScriptFromAssets", "assetName", assetName); @@ -59,7 +59,7 @@ std::string loadScriptFromAssets( return ""; } -std::string loadScriptFromFile(std::string fileName) { +std::string loadScriptFromFile(const std::string& fileName) { #ifdef WITH_FBSYSTRACE FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "reactbridge_jni_loadScriptFromFile", "fileName", fileName); diff --git a/ReactAndroid/src/main/jni/react/jni/JSLoader.h b/ReactAndroid/src/main/jni/react/jni/JSLoader.h index 91aa137c9..8b716beef 100644 --- a/ReactAndroid/src/main/jni/react/jni/JSLoader.h +++ b/ReactAndroid/src/main/jni/react/jni/JSLoader.h @@ -13,17 +13,17 @@ namespace react { * Helper method for loading a JS script from Android assets without * a reference to an AssetManager. */ -std::string loadScriptFromAssets(std::string assetName); +std::string loadScriptFromAssets(const std::string& assetName); /** * Helper method for loading JS script from android asset */ -std::string loadScriptFromAssets(AAssetManager *assetManager, std::string assetName); +std::string loadScriptFromAssets(AAssetManager *assetManager, const std::string& assetName); /** * Helper method for loading JS script from a file */ -std::string loadScriptFromFile(std::string fileName); +std::string loadScriptFromFile(const std::string& fileName); void registerJSLoaderNatives(); diff --git a/ReactAndroid/src/main/jni/react/jni/JSLogging.cpp b/ReactAndroid/src/main/jni/react/jni/JSLogging.cpp new file mode 100644 index 000000000..7690a5656 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/JSLogging.cpp @@ -0,0 +1,36 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "JSLogging.h" + +#include +#include +#include +#include + +namespace facebook { +namespace react { + +JSValueRef nativeLoggingHook( + JSContextRef ctx, + JSObjectRef function, + JSObjectRef thisObject, + size_t argumentCount, + const JSValueRef arguments[], JSValueRef *exception) { + android_LogPriority logLevel = ANDROID_LOG_DEBUG; + if (argumentCount > 1) { + int level = (int) JSValueToNumber(ctx, arguments[1], NULL); + // The lowest log level we get from JS is 0. We shift and cap it to be + // in the range the Android logging method expects. + logLevel = std::min( + static_cast(level + ANDROID_LOG_DEBUG), + ANDROID_LOG_FATAL); + } + if (argumentCount > 0) { + JSStringRef jsString = JSValueToStringCopy(ctx, arguments[0], NULL); + String message = String::adopt(jsString); + FBLOG_PRI(logLevel, "ReactNativeJS", "%s", message.str().c_str()); + } + return JSValueMakeUndefined(ctx); +} + +}}; diff --git a/ReactAndroid/src/main/jni/react/jni/JSLogging.h b/ReactAndroid/src/main/jni/react/jni/JSLogging.h new file mode 100644 index 000000000..27756b665 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/JSLogging.h @@ -0,0 +1,15 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include + +namespace facebook { +namespace react { +JSValueRef nativeLoggingHook( + JSContextRef ctx, + JSObjectRef function, + JSObjectRef thisObject, + size_t argumentCount, + const JSValueRef arguments[], JSValueRef *exception); +}} diff --git a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index 4abc78f20..41a4044f6 100644 --- a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -15,11 +15,14 @@ #include #include #include +#include #include "JNativeRunnable.h" #include "JSLoader.h" #include "ReadableNativeArray.h" #include "ProxyExecutor.h" #include "OnLoad.h" +#include "JSLogging.h" +#include "JSCPerfLogging.h" #include #ifdef WITH_FBSYSTRACE @@ -561,6 +564,15 @@ static jmethodID gCallbackMethod; static jmethodID gOnBatchCompleteMethod; static jmethodID gLogMarkerMethod; +static void logMarker(const std::string& marker) { + JNIEnv* env = Environment::current(); + jclass markerClass = env->FindClass("com/facebook/react/bridge/ReactMarker"); + jstring jmarker = env->NewStringUTF(marker.c_str()); + env->CallStaticVoidMethod(markerClass, gLogMarkerMethod, jmarker); + env->DeleteLocalRef(markerClass); + env->DeleteLocalRef(jmarker); +} + static void makeJavaCall(JNIEnv* env, jobject callback, MethodCall&& call) { if (call.arguments.isNull()) { return; @@ -801,6 +813,15 @@ jmethodID getLogMarkerMethod() { extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { return initialize(vm, [] { + // Inject some behavior into react/ + ReactMarker::logMarker = bridge::logMarker; + WebWorkerUtil::loadScriptFromAssets = + [] (const std::string& assetName) { + return loadScriptFromAssets(assetName); + }; + PerfLogging::installNativeHooks = addNativePerfLoggingHooks; + JSLogging::nativeHook = nativeLoggingHook; + // get the current env JNIEnv* env = Environment::current();