From 36628f0198eee0e7bc4112076a2b6860c5e05ea8 Mon Sep 17 00:00:00 2001 From: Jim Chen Date: Fri, 12 Aug 2016 23:15:52 -0400 Subject: [PATCH] Bug 1292323 - Implement JNI thread checking and dispatching; r=snorp Implement checking the calling thread of a JNI call based on the calledFrom attribute set in WrapForJNI. Also implement automatic call dispatching based on the dispatchTo attribute set in WrapForJNI. This eliminates the use of UsesNativeCallProxy and UsesGeckoThreadProxy. --- widget/android/jni/Accessors.h | 10 ++- widget/android/jni/Natives.h | 146 +++++++++++++++++---------------- widget/android/jni/Refs.h | 37 --------- widget/android/jni/Utils.cpp | 24 +++++- widget/android/jni/Utils.h | 80 ++++++++++++++++-- widget/android/jni/moz.build | 1 + widget/android/nsAppShell.h | 13 --- 7 files changed, 182 insertions(+), 129 deletions(-) diff --git a/widget/android/jni/Accessors.h b/widget/android/jni/Accessors.h index fbb439f9eb9d..fe2cbc9d4c8b 100644 --- a/widget/android/jni/Accessors.h +++ b/widget/android/jni/Accessors.h @@ -39,7 +39,7 @@ class Accessor static void GetNsresult(JNIEnv* env, nsresult* rv) { if (env->ExceptionCheck()) { -#ifdef DEBUG +#ifdef MOZ_CHECK_JNI env->ExceptionDescribe(); #endif env->ExceptionClear(); @@ -77,6 +77,10 @@ protected: static void BeginAccess(const Context& ctx) { + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatching not supported for method call"); + if (sID) { return; } @@ -196,6 +200,10 @@ private: static void BeginAccess(const Context& ctx) { + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatching not supported for field access"); + if (sID) { return; } diff --git a/widget/android/jni/Natives.h b/widget/android/jni/Natives.h index 0181e5b2ad85..9bbf1950525f 100644 --- a/widget/android/jni/Natives.h +++ b/widget/android/jni/Natives.h @@ -178,16 +178,15 @@ struct NativePtr using namespace detail; /** - * For C++ classes whose native methods all return void, they can choose to - * have the native calls go through a proxy by inheriting from - * mozilla::jni::UsesNativeCallProxy, and overriding the OnNativeCall member. - * Subsequently, every native call is automatically wrapped in a functor - * object, and the object is passed to OnNativeCall. The OnNativeCall - * implementation can choose to invoke the call, save it, dispatch it to a - * different thread, etc. Each copy of functor may only be invoked once. + * For JNI native methods that are dispatched to a proxy, i.e. using + * @WrapForJNI(dispatchTo = "proxy"), the implementing C++ class must provide a + * OnNativeCall member. Subsequently, every native call is automatically + * wrapped in a functor object, and the object is passed to OnNativeCall. The + * OnNativeCall implementation can choose to invoke the call, save it, dispatch + * it to a different thread, etc. Each copy of functor may only be invoked + * once. * * class MyClass : public MyJavaClass::Natives - * , public mozilla::jni::UsesNativeCallProxy * { * // ... * @@ -209,16 +208,6 @@ using namespace detail; * }; */ -struct UsesNativeCallProxy -{ - template - static void OnNativeCall(Functor&& call) - { - // The default behavior is to invoke the call right away. - call(); - } -}; - namespace detail { template @@ -263,16 +252,12 @@ template struct ProxyArg : ProxyArg {}; template<> struct ProxyArg : ProxyArg {}; template struct ProxyArg> : ProxyArg {}; -// ProxyNativeCall implements the functor object that is passed to -// UsesNativeCallProxy::OnNativeCall +// ProxyNativeCall implements the functor object that is passed to OnNativeCall template -class ProxyNativeCall +class ProxyNativeCall : public AbstractCall { - template - friend class detail::NativeStubImpl; - // "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. @@ -298,15 +283,6 @@ class ProxyNativeCall // Saved arguments. mozilla::Tuple::Type...> mArgs; - ProxyNativeCall(NativeCallType nativeCall, - JNIEnv* env, - ThisArgJNIType thisArg, - typename ProxyArg::JNIType... args) - : mNativeCall(nativeCall) - , mThisArg(env, ThisArgClass::Ref::From(thisArg)) - , mArgs(ProxyArg::From(env, args)...) - {} - // We cannot use IsStatic and HasThisArg directly (without going through // extra hoops) because GCC complains about invalid overloads, so we use // another pair of template parameters, Static and ThisArg. @@ -363,6 +339,15 @@ public: static const bool isStatic = IsStatic; + ProxyNativeCall(NativeCallType nativeCall, + JNIEnv* env, + ThisArgJNIType thisArg, + typename ProxyArg::JNIType... args) + : mNativeCall(nativeCall) + , mThisArg(env, ThisArgClass::Ref::From(thisArg)) + , mArgs(ProxyArg::From(env, args)...) + {} + ProxyNativeCall(ProxyNativeCall&&) = default; ProxyNativeCall(const ProxyNativeCall&) = default; @@ -381,7 +366,7 @@ public: void SetTarget(NativeCallType call) { mNativeCall = call; } template void SetTarget(T&&) const { MOZ_CRASH(); } - void operator()() + void operator()() override { JNIEnv* const env = GetEnvForThread(); typename ThisArgClass::LocalRef thisArg(env, mThisArg); @@ -397,22 +382,28 @@ public: } }; -// We can only use Impl::OnNativeCall when Impl is derived from -// UsesNativeCallProxy, otherwise it's a compile error. Therefore, the real -// Dispatch function is conditional on UsesNativeCallProxy being a base class -// of Impl. Otherwise, the dummy Dispatch function below that is chosen during -// overload resolution. Because Dispatch is called with an rvalue, the && -// version is always chosen before the const& version, if possible. - -template -typename mozilla::EnableIf< - mozilla::IsBaseOf::value, void>::Type -Dispatch(ProxyNativeCall&& call) +template +typename mozilla::EnableIf::Type +Dispatch(UniquePtr>&& call) { - Impl::OnNativeCall(mozilla::Move(call)); + Impl::OnNativeCall(Move(*call)); } -template +template +typename mozilla::EnableIf::Type +Dispatch(UniquePtr>&& call) +{ + DispatchToGeckoThread(Move(call)); +} + +// The dummy Dispatch function below is chosen during overload resolution if +// none of the above conditional overloads apply. Because Dispatch is called +// with an rvalue, the && version is always chosen before the const& version, +// if possible. + +template void Dispatch(const T&) {} } // namespace detail @@ -452,8 +443,9 @@ public: static MOZ_JNICALL ReturnJNIType Wrap(JNIEnv* env, jobject instance, typename TypeAdapter::JNIType... args) { - static_assert(!mozilla::IsBaseOf::value, - "Native call proxy only supports void return type"); + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatched calls must have void return type"); Impl* const impl = NativePtr::Get(env, instance); if (!impl) { @@ -468,8 +460,9 @@ public: static MOZ_JNICALL ReturnJNIType Wrap(JNIEnv* env, jobject instance, typename TypeAdapter::JNIType... args) { - static_assert(!mozilla::IsBaseOf::value, - "Native call proxy only supports void return type"); + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatched calls must have void return type"); Impl* const impl = NativePtr::Get(env, instance); if (!impl) { @@ -496,10 +489,12 @@ public: static MOZ_JNICALL void Wrap(JNIEnv* env, jobject instance, typename TypeAdapter::JNIType... args) { - if (mozilla::IsBaseOf::value) { - Dispatch(ProxyNativeCall< + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + + if (Traits::dispatchTarget != DispatchTarget::CURRENT) { + Dispatch(MakeUnique(Method, env, instance, args...)); + Args...>>(Method, env, instance, args...)); return; } Impl* const impl = NativePtr::Get(env, instance); @@ -514,10 +509,12 @@ public: static MOZ_JNICALL void Wrap(JNIEnv* env, jobject instance, typename TypeAdapter::JNIType... args) { - if (mozilla::IsBaseOf::value) { - Dispatch(ProxyNativeCall< + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + + if (Traits::dispatchTarget != DispatchTarget::CURRENT) { + Dispatch(MakeUnique(Method, env, instance, args...)); + Args...>>(Method, env, instance, args...)); return; } Impl* const impl = NativePtr::Get(env, instance); @@ -533,11 +530,14 @@ public: template static MOZ_JNICALL void Wrap(JNIEnv* env, jobject instance) { - if (mozilla::IsBaseOf::value) { + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + + if (Traits::dispatchTarget != DispatchTarget::CURRENT) { auto cls = Class::LocalRef::Adopt( env, env->GetObjectClass(instance)); - Dispatch(ProxyNativeCall( + Dispatch(MakeUnique>( DisposeNative, env, cls.Get(), instance)); return; } @@ -561,8 +561,9 @@ public: static MOZ_JNICALL ReturnJNIType Wrap(JNIEnv* env, jclass, typename TypeAdapter::JNIType... args) { - static_assert(!mozilla::IsBaseOf::value, - "Native call proxy only supports void return type"); + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatched calls must have void return type"); return TypeAdapter::FromNative(env, (*Method)(TypeAdapter::ToNative(env, args)...)); @@ -573,8 +574,9 @@ public: static MOZ_JNICALL ReturnJNIType Wrap(JNIEnv* env, jclass cls, typename TypeAdapter::JNIType... args) { - static_assert(!mozilla::IsBaseOf::value, - "Native call proxy only supports void return type"); + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + static_assert(Traits::dispatchTarget == DispatchTarget::CURRENT, + "Dispatched calls must have void return type"); auto clazz = Class::LocalRef::Adopt(env, cls); const auto res = TypeAdapter::FromNative(env, @@ -597,10 +599,12 @@ public: static MOZ_JNICALL void Wrap(JNIEnv* env, jclass cls, typename TypeAdapter::JNIType... args) { - if (mozilla::IsBaseOf::value) { - Dispatch(ProxyNativeCall< + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + + if (Traits::dispatchTarget != DispatchTarget::CURRENT) { + Dispatch(MakeUnique(Method, env, cls, args...)); + Args...>>(Method, env, cls, args...)); return; } (*Method)(TypeAdapter::ToNative(env, args)...); @@ -611,10 +615,12 @@ public: static MOZ_JNICALL void Wrap(JNIEnv* env, jclass cls, typename TypeAdapter::JNIType... args) { - if (mozilla::IsBaseOf::value) { - Dispatch(ProxyNativeCall< + MOZ_ASSERT_JNI_THREAD(Traits::callingThread); + + if (Traits::dispatchTarget != DispatchTarget::CURRENT) { + Dispatch(MakeUnique(Method, env, cls, args...)); + Args...>>(Method, env, cls, args...)); return; } auto clazz = Class::LocalRef::Adopt(env, cls); diff --git a/widget/android/jni/Refs.h b/widget/android/jni/Refs.h index 7ac6e82cfc50..139494c48613 100644 --- a/widget/android/jni/Refs.h +++ b/widget/android/jni/Refs.h @@ -25,43 +25,6 @@ template class GlobalRef; template class DependentRef; -// How exception during a JNI call should be treated. -enum class ExceptionMode -{ - // Abort on unhandled excepion (default). - ABORT, - // Ignore the exception and return to caller. - IGNORE, - // Catch any exception and return a nsresult. - NSRESULT, -}; - -// Thread that a particular JNI call is allowed on. -enum class CallingThread -{ - // Can be called from any thread (default). - ANY, - // Can be called from the Gecko thread. - GECKO, - // Can be called from the Java UI thread. - UI, -}; - -// If and where a JNI call will be dispatched. -enum class DispatchTarget -{ - // Call happens synchronously on the calling thread (default). - CURRENT, - // Call happens synchronously on the calling thread, but the call is - // wrapped in a function object and is passed thru UsesNativeCallProxy. - // Method must return void. - PROXY, - // Call is dispatched asynchronously on the Gecko thread. Method must - // return void. - GECKO, -}; - - // Class to hold the native types of a method's arguments. // For example, if a method has signature (ILjava/lang/String;)V, // its arguments class would be jni::Args diff --git a/widget/android/jni/Utils.cpp b/widget/android/jni/Utils.cpp index 6d4e08b1ac2c..d2d4879c5871 100644 --- a/widget/android/jni/Utils.cpp +++ b/widget/android/jni/Utils.cpp @@ -7,6 +7,7 @@ #include "AndroidBridge.h" #include "GeneratedJNIWrappers.h" +#include "nsAppShell.h" #ifdef MOZ_CRASHREPORTER #include "nsExceptionHandler.h" @@ -143,7 +144,7 @@ bool HandleUncaughtException(JNIEnv* aEnv) return false; } -#ifdef DEBUG +#ifdef MOZ_CHECK_JNI aEnv->ExceptionDescribe(); #endif @@ -210,5 +211,26 @@ jclass GetClassGlobalRef(JNIEnv* aEnv, const char* aClassName) return AndroidBridge::GetClassGlobalRef(aEnv, aClassName); } + +void DispatchToGeckoThread(UniquePtr&& aCall) +{ + class AbstractCallEvent : public nsAppShell::Event + { + UniquePtr mCall; + + public: + AbstractCallEvent(UniquePtr&& aCall) + : mCall(Move(aCall)) + {} + + void Run() override + { + (*mCall)(); + } + }; + + nsAppShell::PostEvent(MakeUnique(Move(aCall))); +} + } // jni } // mozilla diff --git a/widget/android/jni/Utils.h b/widget/android/jni/Utils.h index fe5ffab25a69..7739f8c788e5 100644 --- a/widget/android/jni/Utils.h +++ b/widget/android/jni/Utils.h @@ -3,14 +3,59 @@ #include +#include "mozilla/UniquePtr.h" + #if defined(DEBUG) || !defined(RELEASE_BUILD) +#define MOZ_CHECK_JNI +#endif + +#ifdef MOZ_CHECK_JNI +#include #include "mozilla/Assertions.h" +#include "APKOpen.h" #include "MainThreadUtils.h" #endif namespace mozilla { namespace jni { +// How exception during a JNI call should be treated. +enum class ExceptionMode +{ + // Abort on unhandled excepion (default). + ABORT, + // Ignore the exception and return to caller. + IGNORE, + // Catch any exception and return a nsresult. + NSRESULT, +}; + +// Thread that a particular JNI call is allowed on. +enum class CallingThread +{ + // Can be called from any thread (default). + ANY, + // Can be called from the Gecko thread. + GECKO, + // Can be called from the Java UI thread. + UI, +}; + +// If and where a JNI call will be dispatched. +enum class DispatchTarget +{ + // Call happens synchronously on the calling thread (default). + CURRENT, + // Call happens synchronously on the calling thread, but the call is + // wrapped in a function object and is passed thru UsesNativeCallProxy. + // Method must return void. + PROXY, + // Call is dispatched asynchronously on the Gecko thread. Method must + // return void. + GECKO, +}; + + extern JNIEnv* sGeckoThreadEnv; inline bool IsAvailable() @@ -20,13 +65,9 @@ inline bool IsAvailable() inline JNIEnv* GetGeckoThreadEnv() { -#if defined(DEBUG) || !defined(RELEASE_BUILD) - if (!NS_IsMainThread()) { - MOZ_CRASH("Not on main thread"); - } - if (!sGeckoThreadEnv) { - MOZ_CRASH("Don't have a JNIEnv"); - } +#ifdef MOZ_CHECK_JNI + MOZ_RELEASE_ASSERT(NS_IsMainThread(), "Must be on Gecko thread"); + MOZ_RELEASE_ASSERT(sGeckoThreadEnv, "Must have a JNIEnv"); #endif return sGeckoThreadEnv; } @@ -35,6 +76,21 @@ void SetGeckoThreadEnv(JNIEnv* aEnv); JNIEnv* GetEnvForThread(); +#ifdef MOZ_CHECK_JNI +#define MOZ_ASSERT_JNI_THREAD(thread) \ + do { \ + if ((thread) == mozilla::jni::CallingThread::GECKO) { \ + MOZ_RELEASE_ASSERT(::NS_IsMainThread()); \ + } else if ((thread) == mozilla::jni::CallingThread::UI) { \ + const bool isOnUiThread = ::pthread_equal(::pthread_self(), \ + ::getJavaUiThread()); \ + MOZ_RELEASE_ASSERT(isOnUiThread); \ + } \ + } while (0) +#else +#define MOZ_ASSERT_JNI_THREAD(thread) do {} while (0) +#endif + bool ThrowException(JNIEnv *aEnv, const char *aClass, const char *aMessage); @@ -62,12 +118,22 @@ bool HandleUncaughtException(JNIEnv* aEnv); } \ } while (0) + uintptr_t GetNativeHandle(JNIEnv* env, jobject instance); void SetNativeHandle(JNIEnv* env, jobject instance, uintptr_t handle); jclass GetClassGlobalRef(JNIEnv* aEnv, const char* aClassName); + +struct AbstractCall +{ + virtual ~AbstractCall() {} + virtual void operator()() = 0; +}; + +void DispatchToGeckoThread(UniquePtr&& aCall); + } // jni } // mozilla diff --git a/widget/android/jni/moz.build b/widget/android/jni/moz.build index 29a273b81557..31d7d32e6cf4 100644 --- a/widget/android/jni/moz.build +++ b/widget/android/jni/moz.build @@ -19,5 +19,6 @@ UNIFIED_SOURCES += [ FINAL_LIBRARY = 'xul' LOCAL_INCLUDES += [ + '/widget', '/widget/android', ] diff --git a/widget/android/nsAppShell.h b/widget/android/nsAppShell.h index a9cc50a73cfe..e399b0cf5ea1 100644 --- a/widget/android/nsAppShell.h +++ b/widget/android/nsAppShell.h @@ -224,18 +224,5 @@ protected: nsInterfaceHashtable mObserversHash; }; -// Class that implement native JNI methods can inherit from -// UsesGeckoThreadProxy to have the native call forwarded -// automatically to the Gecko thread. -class UsesGeckoThreadProxy : public mozilla::jni::UsesNativeCallProxy -{ -public: - template - static void OnNativeCall(Functor&& call) - { - nsAppShell::PostEvent(mozilla::Move(call)); - } -}; - #endif // nsAppShell_h__