From e774d9991abe29ae5827a854fa64188c7058aa86 Mon Sep 17 00:00:00 2001
From: Pieter De Baets <pieterdb@fb.com>
Date: Wed, 22 Feb 2017 13:44:31 -0800
Subject: [PATCH] Cleanup fbjni usage in JSCPerfLogging

Reviewed By: AaaChiuuu

Differential Revision: D4572148

fbshipit-source-id: acb5494dc094204db25928e456f2d3eda8ce5ae5
---
 .../main/jni/xreact/jni/JSCPerfLogging.cpp    | 108 +++++-------------
 .../src/main/jni/xreact/jni/JSCPerfLogging.h  |   1 +
 2 files changed, 32 insertions(+), 77 deletions(-)

diff --git a/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.cpp b/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.cpp
index f5e7ba021..d2c9973ec 100644
--- a/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.cpp
+++ b/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.cpp
@@ -9,38 +9,27 @@
 
 using namespace facebook::jni;
 
-struct _jqplProvider : _jobject {};
-using jqplProvider = _jqplProvider*;
+namespace facebook { namespace react {
 
-struct _jqpl : _jobject {};
-using jqpl = _jqpl*;
-
-namespace facebook { namespace jni {
-
-template<>
-class JObjectWrapper<jqpl> : public JObjectWrapper<jobject> {
-
- public:
-  static constexpr const char* kJavaDescriptor = "Lcom/facebook/quicklog/QuickPerformanceLogger;";
-
-  using JObjectWrapper<jobject>::JObjectWrapper;
+struct JQuickPerformanceLogger : JavaClass<JQuickPerformanceLogger> {
+  static auto constexpr kJavaDescriptor = "Lcom/facebook/quicklog/QuickPerformanceLogger;";
 
   void markerStart(int markerId, int instanceKey, long timestamp) {
     static auto markerStartMethod =
-      qplClass()->getMethod<void(jint, jint, jlong)>("markerStart");
-    markerStartMethod(this_, markerId, instanceKey, timestamp);
+      javaClassStatic()->getMethod<void(jint, jint, jlong)>("markerStart");
+    markerStartMethod(self(), markerId, instanceKey, timestamp);
   }
 
   void markerEnd(int markerId, int instanceKey, short actionId, long timestamp) {
     static auto markerEndMethod =
-      qplClass()->getMethod<void(jint, jint, jshort, jlong)>("markerEnd");
-    markerEndMethod(this_, markerId, instanceKey, actionId, timestamp);
+      javaClassStatic()->getMethod<void(jint, jint, jshort, jlong)>("markerEnd");
+    markerEndMethod(self(), markerId, instanceKey, actionId, timestamp);
   }
 
   void markerTag(int markerId, int instanceKey, alias_ref<jstring> tag) {
     static auto markerTagMethod =
-      qplClass()->getMethod<void(jint, jint, alias_ref<jstring>)>("markerTag");
-    markerTagMethod(this_, markerId, instanceKey, tag);
+      javaClassStatic()->getMethod<void(jint, jint, alias_ref<jstring>)>("markerTag");
+    markerTagMethod(self(), markerId, instanceKey, tag);
   }
 
   void markerAnnotate(
@@ -48,86 +37,54 @@ class JObjectWrapper<jqpl> : public JObjectWrapper<jobject> {
       int instanceKey,
       alias_ref<jstring> key,
       alias_ref<jstring> value) {
-    static auto markerAnnotateMethod =
-      qplClass()->getMethod<void(
-      jint,
-      jint,
-      alias_ref<jstring>,
-      alias_ref<jstring>)>("markerAnnotate");
-    markerAnnotateMethod(this_, markerId, instanceKey, key, value);
+    static auto markerAnnotateMethod = javaClassStatic()->
+      getMethod<void(jint, jint, alias_ref<jstring>, alias_ref<jstring>)>("markerAnnotate");
+    markerAnnotateMethod(self(), markerId, instanceKey, key, value);
   }
 
   void markerNote(int markerId, int instanceKey, short actionId, long timestamp) {
     static auto markerNoteMethod =
-      qplClass()->getMethod<void(jint, jint, jshort, jlong)>("markerNote");
-    markerNoteMethod(this_, markerId, instanceKey, actionId, timestamp);
+      javaClassStatic()->getMethod<void(jint, jint, jshort, jlong)>("markerNote");
+    markerNoteMethod(self(), markerId, instanceKey, actionId, timestamp);
   }
 
   void markerCancel(int markerId, int instanceKey) {
     static auto markerCancelMethod =
-      qplClass()->getMethod<void(jint, jint)>("markerCancel");
-    markerCancelMethod(this_, markerId, instanceKey);
+      javaClassStatic()->getMethod<void(jint, jint)>("markerCancel");
+    markerCancelMethod(self(), markerId, instanceKey);
   }
 
   int64_t currentMonotonicTimestamp() {
     static auto currentTimestampMethod =
-      qplClass()->getMethod<jlong()>("currentMonotonicTimestamp");
-    return currentTimestampMethod(this_);
-  }
-
- private:
-
-  static alias_ref<jclass> qplClass() {
-    static auto cls = findClassStatic("com/facebook/quicklog/QuickPerformanceLogger");
-    return cls;
-  }
-
-};
-using JQuickPerformanceLogger = JObjectWrapper<jqpl>;
-
-
-template<>
-class JObjectWrapper<jqplProvider> : public JObjectWrapper<jobject> {
- public:
-  static constexpr const char* kJavaDescriptor =
-    "Lcom/facebook/quicklog/QuickPerformanceLoggerProvider;";
-
-  using JObjectWrapper<jobject>::JObjectWrapper;
-
-  static global_ref<jqpl> get() {
-    static auto getQPLInstMethod = qplProviderClass()->getStaticMethod<jqpl()>("getQPLInstance");
-    static global_ref<jqpl> theQpl = make_global(getQPLInstMethod(qplProviderClass().get()));
-    return theQpl;
-  }
-
-  static bool check() {
-    static auto getQPLInstMethod = qplProviderClass()->getStaticMethod<jqpl()>("getQPLInstance");
-    auto theQpl = getQPLInstMethod(qplProviderClass().get());
-    return (theQpl.get() != nullptr);
-  }
-
- private:
-
-  static alias_ref<jclass> qplProviderClass() {
-    static auto cls = findClassStatic("com/facebook/quicklog/QuickPerformanceLoggerProvider");
-    return cls;
+      javaClassStatic()->getMethod<jlong()>("currentMonotonicTimestamp");
+    return currentTimestampMethod(self());
   }
 };
-using JQuickPerformanceLoggerProvider = JObjectWrapper<jqplProvider>;
 
-}}
+struct JQuickPerformanceLoggerProvider : JavaClass<JQuickPerformanceLoggerProvider> {
+  static auto constexpr kJavaDescriptor = "Lcom/facebook/quicklog/QuickPerformanceLoggerProvider;";
+
+  static alias_ref<JQuickPerformanceLogger::javaobject> get() {
+    static auto getQPLInstMethod =
+      javaClassStatic()->getStaticMethod<JQuickPerformanceLogger::javaobject()>("getQPLInstance");
+    static auto logger = make_global(getQPLInstMethod(javaClassStatic()));
+    return logger;
+  }
+};
 
 static bool isReady() {
   static bool ready = false;
   if (!ready) {
     try {
+      // TODO: findClassStatic only does the lookup once. If we can't find
+      // QuickPerformanceLoggerProvider the first time we call this, we will always fail here.
       findClassStatic("com/facebook/quicklog/QuickPerformanceLoggerProvider");
     } catch(...) {
       // Swallow this exception - we don't want to crash the app, an error is enough.
       FBLOGE("Calling QPL from JS before class has been loaded in Java. Ignored.");
       return false;
     }
-    if (JQuickPerformanceLoggerProvider::check()) {
+    if (JQuickPerformanceLoggerProvider::get()) {
       ready = true;
     } else {
       FBLOGE("Calling QPL from JS before it has been initialized in Java. Ignored.");
@@ -294,9 +251,6 @@ static JSValueRef nativeQPLTimestamp(
   return JSValueMakeNumber(ctx, timestamp);
 }
 
-namespace facebook {
-namespace react {
-
 void addNativePerfLoggingHooks(JSGlobalContextRef ctx) {
   installGlobalFunction(ctx, "nativeQPLMarkerStart", nativeQPLMarkerStart);
   installGlobalFunction(ctx, "nativeQPLMarkerEnd", nativeQPLMarkerEnd);
diff --git a/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.h b/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.h
index 256755b1f..241e716b8 100644
--- a/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.h
+++ b/ReactAndroid/src/main/jni/xreact/jni/JSCPerfLogging.h
@@ -3,6 +3,7 @@
 #pragma once
 
 #include <JavaScriptCore/JSContextRef.h>
+
 namespace facebook {
 namespace react {