Bug 1382861 - 1. Replace jni::AbstractCall with nsIRunnable; r=esawin
authorJim Chen <nchen@mozilla.com>
Tue, 25 Jul 2017 17:25:58 -0400
changeset 419694 9c322ac1b41d89d2f952db9fc49aa0af3c03a13b
parent 419693 1d844f6fdaf39108dea8aa3fd280f5cc654e1489
child 419695 9e9270da4f55d22a25e51a07d411d7216687fb77
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1382861
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1382861 - 1. Replace jni::AbstractCall with nsIRunnable; r=esawin The native binding code used `jni::AbstractCall` as the interface between `ProxyNativeCall` and `DispatchToGeckoPriorityQueue`. However, we already make use of `nsIRunnable` for dispatching to the XPCOM queue, so we should just use `nsIRunnable` for the priority queue as well. MozReview-Commit-ID: KmuNMZZkXX3
widget/android/jni/Natives.h
widget/android/jni/Utils.cpp
widget/android/jni/Utils.h
--- a/widget/android/jni/Natives.h
+++ b/widget/android/jni/Natives.h
@@ -347,17 +347,17 @@ struct ProxyArg<Ref<C, T>>
 template<typename C> struct ProxyArg<const C&> : ProxyArg<C> {};
 template<> struct ProxyArg<StringParam> : ProxyArg<String::Ref> {};
 template<class C> struct ProxyArg<LocalRef<C>> : ProxyArg<typename C::Ref> {};
 
 // ProxyNativeCall implements the functor object that is passed to OnNativeCall
 template<class Impl, class Owner, bool IsStatic,
          bool HasThisArg /* has instance/class local ref in the call */,
          typename... Args>
-class ProxyNativeCall : public AbstractCall
+class ProxyNativeCall
 {
     // "this arg" refers to the Class::LocalRef (for static methods) or
     // Owner::LocalRef (for instance methods) that we optionally (as indicated
     // by HasThisArg) pass into the destination C++ function.
     typedef typename mozilla::Conditional<IsStatic,
             Class, Owner>::Type ThisArgClass;
     typedef typename mozilla::Conditional<IsStatic,
             jclass, jobject>::Type ThisArgJNIType;
@@ -470,17 +470,17 @@ public:
     bool IsTarget(NativeCallType call) const { return call == mNativeCall; }
     template<typename T> bool IsTarget(T&&) const { return false; }
 
     // Redirect the call to another function / class member with the same
     // signature as the original target. Crash if given a wrong signature.
     void SetTarget(NativeCallType call) { mNativeCall = call; }
     template<typename T> void SetTarget(T&&) const { MOZ_CRASH(); }
 
-    void operator()() override
+    void operator()()
     {
         JNIEnv* const env = GetEnvForThread();
         typename ThisArgClass::LocalRef thisArg(env, mThisArg);
         Call<IsStatic, HasThisArg>(
                 thisArg, typename IndexSequenceFor<Args...>::Type());
 
         // Clear all saved global refs. We do this after the call is invoked,
         // and not inside the destructor because we already have a JNIEnv here,
@@ -509,35 +509,39 @@ struct Dispatcher
              typename ThisArg, typename... ProxyArgs>
     static typename EnableIf<
             Traits::dispatchTarget == DispatchTarget::GECKO_PRIORITY, void>::Type
     Run(ThisArg thisArg, ProxyArgs&&... args)
     {
         // For a static method, do not forward the "this arg" (i.e. the class
         // local ref) if the implementation does not request it. This saves us
         // a pair of calls to add/delete global ref.
-        DispatchToGeckoPriorityQueue(MakeUnique<ProxyNativeCall<
-                Impl, typename Traits::Owner, IsStatic, HasThisArg,
-                Args...>>(HasThisArg || !IsStatic ? thisArg : nullptr,
-                          Forward<ProxyArgs>(args)...));
+        auto proxy = ProxyNativeCall<Impl, typename Traits::Owner, IsStatic,
+                                     HasThisArg, Args...>(
+                (HasThisArg || !IsStatic) ? thisArg : nullptr,
+                Forward<ProxyArgs>(args)...);
+        DispatchToGeckoPriorityQueue(
+                NS_NewRunnableFunction("PriorityNativeCall", Move(proxy)));
     }
 
     template<class Traits, bool IsStatic = Traits::isStatic,
              typename ThisArg, typename... ProxyArgs>
     static typename EnableIf<
             Traits::dispatchTarget == DispatchTarget::GECKO, void>::Type
     Run(ThisArg thisArg, ProxyArgs&&... args)
     {
         // For a static method, do not forward the "this arg" (i.e. the class
         // local ref) if the implementation does not request it. This saves us
         // a pair of calls to add/delete global ref.
-        NS_DispatchToMainThread(NS_NewRunnableFunction("ProxyNativeCall", ProxyNativeCall<
-                Impl, typename Traits::Owner, IsStatic, HasThisArg,
-                Args...>(HasThisArg || !IsStatic ? thisArg : nullptr,
-                          Forward<ProxyArgs>(args)...)));
+        auto proxy = ProxyNativeCall<Impl, typename Traits::Owner, IsStatic,
+                                     HasThisArg, Args...>(
+                (HasThisArg || !IsStatic) ? thisArg : nullptr,
+                Forward<ProxyArgs>(args)...);
+        NS_DispatchToMainThread(
+                NS_NewRunnableFunction("GeckoNativeCall", Move(proxy)));
     }
 
     template<class Traits, bool IsStatic = false, typename... ProxyArgs>
     static typename EnableIf<
             Traits::dispatchTarget == DispatchTarget::CURRENT, void>::Type
     Run(ProxyArgs&&... args)
     {
         MOZ_CRASH("Unreachable code");
--- a/widget/android/jni/Utils.cpp
+++ b/widget/android/jni/Utils.cpp
@@ -283,34 +283,27 @@ jclass GetClassRef(JNIEnv* aEnv, const c
             "Does the class require a newer API version? "
             "Or did ProGuard optimize away something it shouldn't have?",
             aClassName);
     aEnv->ExceptionDescribe();
     MOZ_CRASH("Cannot find JNI class");
     return nullptr;
 }
 
-void DispatchToGeckoPriorityQueue(UniquePtr<AbstractCall>&& aCall)
+void DispatchToGeckoPriorityQueue(already_AddRefed<nsIRunnable> aCall)
 {
-    class AbstractCallEvent : public nsAppShell::Event
+    class RunnableEvent : public nsAppShell::Event
     {
-        UniquePtr<AbstractCall> mCall;
-
+        nsCOMPtr<nsIRunnable> mCall;
     public:
-        AbstractCallEvent(UniquePtr<AbstractCall>&& aCall)
-            : mCall(Move(aCall))
-        {}
-
-        void Run() override
-        {
-            (*mCall)();
-        }
+        RunnableEvent(already_AddRefed<nsIRunnable> aCall) : mCall(aCall) {}
+        void Run() override { NS_ENSURE_SUCCESS_VOID(mCall->Run()); }
     };
 
-    nsAppShell::PostEvent(MakeUnique<AbstractCallEvent>(Move(aCall)));
+    nsAppShell::PostEvent(MakeUnique<RunnableEvent>(Move(aCall)));
 }
 
 bool IsFennec()
 {
     return sIsFennec;
 }
 
 int GetAPIVersion() {
--- a/widget/android/jni/Utils.h
+++ b/widget/android/jni/Utils.h
@@ -1,13 +1,15 @@
 #ifndef mozilla_jni_Utils_h__
 #define mozilla_jni_Utils_h__
 
 #include <jni.h>
 
+#include "nsIRunnable.h"
+
 #include "mozilla/UniquePtr.h"
 
 #if defined(DEBUG) || !defined(RELEASE_OR_BETA)
 #define MOZ_CHECK_JNI
 #endif
 
 #ifdef MOZ_CHECK_JNI
 #include <pthread.h>
@@ -127,23 +129,17 @@ bool ReportException(JNIEnv* aEnv, jthro
 
 
 uintptr_t GetNativeHandle(JNIEnv* env, jobject instance);
 
 void SetNativeHandle(JNIEnv* env, jobject instance, uintptr_t handle);
 
 jclass GetClassRef(JNIEnv* aEnv, const char* aClassName);
 
-struct AbstractCall
-{
-    virtual ~AbstractCall() {}
-    virtual void operator()() = 0;
-};
-
-void DispatchToGeckoPriorityQueue(UniquePtr<AbstractCall>&& aCall);
+void DispatchToGeckoPriorityQueue(already_AddRefed<nsIRunnable> aCall);
 
 /**
  * Returns whether Gecko is running in a Fennec environment, as determined by
  * the presence of the GeckoApp class.
  */
 bool IsFennec();
 
 int GetAPIVersion();