From daf15a4aafdcd6978916bef2dae1fb2f2f5e62da Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 27 Oct 2023 11:25:02 +0200 Subject: [PATCH] Android: clean up error handling and native methods registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emit log output and return false immediately if we fail to get both the Activity and the Service objects. Standardize the registerNative methods to return bool and take a QJniEnvironment, and adjust the macros accordingly. Simplify the startup routine to use QJniEnvironment. Change-Id: I11be35426520dc803f5a07bbb495e908592f254e Reviewed-by: Tinja Paavoseppä --- .../android/androidjniaccessibility.cpp | 22 +--- .../android/androidjniaccessibility.h | 3 +- .../platforms/android/androidjniinput.cpp | 7 +- .../platforms/android/androidjniinput.h | 4 +- .../platforms/android/androidjnimain.cpp | 100 +++++++++--------- .../platforms/android/androidjnimenu.cpp | 4 +- .../platforms/android/androidjnimenu.h | 3 +- .../android/qandroidplatformdialoghelpers.cpp | 12 +-- .../android/qandroidplatformdialoghelpers.h | 5 +- 9 files changed, 72 insertions(+), 88 deletions(-) diff --git a/src/plugins/platforms/android/androidjniaccessibility.cpp b/src/plugins/platforms/android/androidjniaccessibility.cpp index c0e4bfacb6..38e8d6dfd0 100644 --- a/src/plugins/platforms/android/androidjniaccessibility.cpp +++ b/src/plugins/platforms/android/androidjniaccessibility.cpp @@ -21,7 +21,6 @@ #include static const char m_qtTag[] = "Qt A11Y"; -static const char m_classErrorMsg[] = "Can't find class \"%s\""; QT_BEGIN_NAMESPACE @@ -353,16 +352,6 @@ namespace QtAndroidAccessibility return result && oldPosition != screenRect_helper(firstChildId, false); } - -#define FIND_AND_CHECK_CLASS(CLASS_NAME) \ -clazz = env->FindClass(CLASS_NAME); \ -if (!clazz) { \ - __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_classErrorMsg, CLASS_NAME); \ - return JNI_FALSE; \ -} - - //__android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_methodErrorMsg, METHOD_NAME, METHOD_SIGNATURE); - static QString textFromValue(QAccessibleInterface *iface) { QString valueStr; @@ -559,7 +548,7 @@ if (!clazz) { \ return true; } - static JNINativeMethod methods[] = { + static const JNINativeMethod methods[] = { {"setActive","(Z)V",(void*)setActive}, {"childIdListForAccessibleObject", "(I)[I", (jintArray)childIdListForAccessibleObject}, {"parentId", "(I)I", (void*)parentId}, @@ -579,13 +568,10 @@ if (!clazz) { \ return false; \ } - bool registerNatives(JNIEnv *env) + bool registerNatives(QJniEnvironment &env) { - jclass clazz; - FIND_AND_CHECK_CLASS("org/qtproject/qt/android/accessibility/QtNativeAccessibility"); - jclass appClass = static_cast(env->NewGlobalRef(clazz)); - - if (env->RegisterNatives(appClass, methods, sizeof(methods) / sizeof(methods[0])) < 0) { + if (!env.registerNativeMethods("org/qtproject/qt/android/accessibility/QtNativeAccessibility", + methods, sizeof(methods) / sizeof(methods[0]))) { __android_log_print(ANDROID_LOG_FATAL,"Qt A11y", "RegisterNatives failed"); return false; } diff --git a/src/plugins/platforms/android/androidjniaccessibility.h b/src/plugins/platforms/android/androidjniaccessibility.h index 9bbbe80fe9..d967dde3ff 100644 --- a/src/plugins/platforms/android/androidjniaccessibility.h +++ b/src/plugins/platforms/android/androidjniaccessibility.h @@ -9,12 +9,13 @@ QT_BEGIN_NAMESPACE class QObject; +class QJniEnvironment; namespace QtAndroidAccessibility { void initialize(); bool isActive(); - bool registerNatives(JNIEnv *env); + bool registerNatives(QJniEnvironment &env); void notifyLocationChange(uint accessibilityObjectId); void notifyObjectHide(uint accessibilityObjectId); void notifyObjectFocus(uint accessibilityObjectId); diff --git a/src/plugins/platforms/android/androidjniinput.cpp b/src/plugins/platforms/android/androidjniinput.cpp index afaf5f1d2a..2c4c95e787 100644 --- a/src/plugins/platforms/android/androidjniinput.cpp +++ b/src/plugins/platforms/android/androidjniinput.cpp @@ -921,11 +921,10 @@ namespace QtAndroidInput {"dispatchKeyEvent", "(Landroid/view/KeyEvent;)Z", reinterpret_cast(dispatchKeyEvent)}, }; - bool registerNatives() + bool registerNatives(QJniEnvironment &env) { - QJniEnvironment qenv; - if (!qenv.registerNativeMethods(QtJniTypes::Traits::className(), - methods, sizeof(methods) / sizeof(methods[0]))) { + if (!env.registerNativeMethods(QtJniTypes::Traits::className(), + methods, sizeof(methods) / sizeof(methods[0]))) { __android_log_print(ANDROID_LOG_FATAL,"Qt", "RegisterNatives failed"); return false; } diff --git a/src/plugins/platforms/android/androidjniinput.h b/src/plugins/platforms/android/androidjniinput.h index 982065eff0..28a2665bf6 100644 --- a/src/plugins/platforms/android/androidjniinput.h +++ b/src/plugins/platforms/android/androidjniinput.h @@ -13,6 +13,8 @@ QT_BEGIN_NAMESPACE Q_DECLARE_LOGGING_CATEGORY(lcQpaInputMethods); +class QJniEnvironment; + namespace QtAndroidInput { // Software keyboard support @@ -49,7 +51,7 @@ namespace QtAndroidInput void registerKeyEventListener(KeyEventListener *listener); void unregisterKeyEventListener(KeyEventListener *listener); - bool registerNatives(); + bool registerNatives(QJniEnvironment &env); } QT_END_NAMESPACE diff --git a/src/plugins/platforms/android/androidjnimain.cpp b/src/plugins/platforms/android/androidjnimain.cpp index ebed64d48d..bed3777d45 100644 --- a/src/plugins/platforms/android/androidjnimain.cpp +++ b/src/plugins/platforms/android/androidjnimain.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -807,46 +808,47 @@ static JNINativeMethod methods[] = { clazz = env->FindClass(CLASS_NAME); \ if (!clazz) { \ __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_classErrorMsg, CLASS_NAME); \ - return JNI_FALSE; \ + return false; \ } #define GET_AND_CHECK_METHOD(VAR, CLASS, METHOD_NAME, METHOD_SIGNATURE) \ VAR = env->GetMethodID(CLASS, METHOD_NAME, METHOD_SIGNATURE); \ if (!VAR) { \ __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_methodErrorMsg, METHOD_NAME, METHOD_SIGNATURE); \ - return JNI_FALSE; \ + return false; \ } #define GET_AND_CHECK_STATIC_METHOD(VAR, CLASS, METHOD_NAME, METHOD_SIGNATURE) \ VAR = env->GetStaticMethodID(CLASS, METHOD_NAME, METHOD_SIGNATURE); \ if (!VAR) { \ __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_methodErrorMsg, METHOD_NAME, METHOD_SIGNATURE); \ - return JNI_FALSE; \ + return false; \ } #define GET_AND_CHECK_FIELD(VAR, CLASS, FIELD_NAME, FIELD_SIGNATURE) \ VAR = env->GetFieldID(CLASS, FIELD_NAME, FIELD_SIGNATURE); \ if (!VAR) { \ __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_methodErrorMsg, FIELD_NAME, FIELD_SIGNATURE); \ - return JNI_FALSE; \ + return false; \ } #define GET_AND_CHECK_STATIC_FIELD(VAR, CLASS, FIELD_NAME, FIELD_SIGNATURE) \ VAR = env->GetStaticFieldID(CLASS, FIELD_NAME, FIELD_SIGNATURE); \ if (!VAR) { \ __android_log_print(ANDROID_LOG_FATAL, m_qtTag, m_methodErrorMsg, FIELD_NAME, FIELD_SIGNATURE); \ - return JNI_FALSE; \ + return false; \ } -static int registerNatives(JNIEnv *env) +static bool registerNatives(QJniEnvironment &env) { jclass clazz; FIND_AND_CHECK_CLASS("org/qtproject/qt/android/QtNative"); m_applicationClass = static_cast(env->NewGlobalRef(clazz)); - if (env->RegisterNatives(m_applicationClass, methods, sizeof(methods) / sizeof(methods[0])) < 0) { + if (!env.registerNativeMethods(m_applicationClass, + methods, sizeof(methods) / sizeof(methods[0]))) { __android_log_print(ANDROID_LOG_FATAL,"Qt", "RegisterNatives failed"); - return JNI_FALSE; + return false; } GET_AND_CHECK_STATIC_METHOD(m_createSurfaceMethodID, m_applicationClass, "createSurface", "(IZIIIII)V"); @@ -860,42 +862,46 @@ static int registerNatives(JNIEnv *env) GET_AND_CHECK_STATIC_METHOD(methodID, m_applicationClass, "service", "()Landroid/app/Service;"); contextObject = env->CallStaticObjectMethod(m_applicationClass, methodID); } + + if (!contextObject) { + __android_log_print(ANDROID_LOG_FATAL,"Qt", "Failed to get Activity or Service object"); + return false; + } + const auto releaseContextObject = qScopeGuard([&env, contextObject]{ + env->DeleteLocalRef(contextObject); + }); + GET_AND_CHECK_STATIC_METHOD(methodID, m_applicationClass, "classLoader", "()Ljava/lang/ClassLoader;"); m_classLoaderObject = env->NewGlobalRef(env->CallStaticObjectMethod(m_applicationClass, methodID)); clazz = env->GetObjectClass(m_classLoaderObject); GET_AND_CHECK_METHOD(m_loadClassMethodID, clazz, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); - if (contextObject) { - FIND_AND_CHECK_CLASS("android/content/ContextWrapper"); - GET_AND_CHECK_METHOD(methodID, clazz, "getAssets", "()Landroid/content/res/AssetManager;"); - m_assets = env->NewGlobalRef(env->CallObjectMethod(contextObject, methodID)); - m_assetManager = AAssetManager_fromJava(env, m_assets); + FIND_AND_CHECK_CLASS("android/content/ContextWrapper"); + GET_AND_CHECK_METHOD(methodID, clazz, "getAssets", "()Landroid/content/res/AssetManager;"); + m_assets = env->NewGlobalRef(env->CallObjectMethod(contextObject, methodID)); + m_assetManager = AAssetManager_fromJava(env.jniEnv(), m_assets); - GET_AND_CHECK_METHOD(methodID, clazz, "getResources", "()Landroid/content/res/Resources;"); - m_resourcesObj = env->NewGlobalRef(env->CallObjectMethod(contextObject, methodID)); + GET_AND_CHECK_METHOD(methodID, clazz, "getResources", "()Landroid/content/res/Resources;"); + m_resourcesObj = env->NewGlobalRef(env->CallObjectMethod(contextObject, methodID)); - FIND_AND_CHECK_CLASS("android/graphics/Bitmap"); - m_bitmapClass = static_cast(env->NewGlobalRef(clazz)); - GET_AND_CHECK_STATIC_METHOD(m_createBitmapMethodID, m_bitmapClass - , "createBitmap", "(IILandroid/graphics/Bitmap$Config;)Landroid/graphics/Bitmap;"); - FIND_AND_CHECK_CLASS("android/graphics/Bitmap$Config"); - jfieldID fieldId; - GET_AND_CHECK_STATIC_FIELD(fieldId, clazz, "ARGB_8888", "Landroid/graphics/Bitmap$Config;"); - m_ARGB_8888_BitmapConfigValue = env->NewGlobalRef(env->GetStaticObjectField(clazz, fieldId)); - GET_AND_CHECK_STATIC_FIELD(fieldId, clazz, "RGB_565", "Landroid/graphics/Bitmap$Config;"); - m_RGB_565_BitmapConfigValue = env->NewGlobalRef(env->GetStaticObjectField(clazz, fieldId)); + FIND_AND_CHECK_CLASS("android/graphics/Bitmap"); + m_bitmapClass = static_cast(env->NewGlobalRef(clazz)); + GET_AND_CHECK_STATIC_METHOD(m_createBitmapMethodID, m_bitmapClass, + "createBitmap", "(IILandroid/graphics/Bitmap$Config;)Landroid/graphics/Bitmap;"); + FIND_AND_CHECK_CLASS("android/graphics/Bitmap$Config"); + jfieldID fieldId; + GET_AND_CHECK_STATIC_FIELD(fieldId, clazz, "ARGB_8888", "Landroid/graphics/Bitmap$Config;"); + m_ARGB_8888_BitmapConfigValue = env->NewGlobalRef(env->GetStaticObjectField(clazz, fieldId)); + GET_AND_CHECK_STATIC_FIELD(fieldId, clazz, "RGB_565", "Landroid/graphics/Bitmap$Config;"); + m_RGB_565_BitmapConfigValue = env->NewGlobalRef(env->GetStaticObjectField(clazz, fieldId)); - FIND_AND_CHECK_CLASS("android/graphics/drawable/BitmapDrawable"); - m_bitmapDrawableClass = static_cast(env->NewGlobalRef(clazz)); - GET_AND_CHECK_METHOD(m_bitmapDrawableConstructorMethodID, - m_bitmapDrawableClass, - "", - "(Landroid/content/res/Resources;Landroid/graphics/Bitmap;)V"); + FIND_AND_CHECK_CLASS("android/graphics/drawable/BitmapDrawable"); + m_bitmapDrawableClass = static_cast(env->NewGlobalRef(clazz)); + GET_AND_CHECK_METHOD(m_bitmapDrawableConstructorMethodID, + m_bitmapDrawableClass, + "", "(Landroid/content/res/Resources;Landroid/graphics/Bitmap;)V"); - env->DeleteLocalRef(contextObject); - } - - return JNI_TRUE; + return true; } QT_END_NAMESPACE @@ -908,23 +914,16 @@ Q_DECL_EXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void */*reserved*/) initialized = true; QT_USE_NAMESPACE - typedef union { - JNIEnv *nativeEnvironment; - void *venv; - } UnionJNIEnvToVoid; - - UnionJNIEnvToVoid uenv; - uenv.venv = nullptr; - m_javaVM = nullptr; - - if (vm->GetEnv(&uenv.venv, JNI_VERSION_1_6) != JNI_OK) { - __android_log_print(ANDROID_LOG_FATAL, "Qt", "GetEnv failed"); + m_javaVM = vm; + QJniEnvironment env; + if (!env.isValid()) { + m_javaVM = nullptr; + __android_log_print(ANDROID_LOG_FATAL, "Qt", "Failed to initialize the JNI Environment"); return -1; } - JNIEnv *env = uenv.nativeEnvironment; if (!registerNatives(env) - || !QtAndroidInput::registerNatives() + || !QtAndroidInput::registerNatives(env) || !QtAndroidMenu::registerNatives(env) || !QtAndroidAccessibility::registerNatives(env) || !QtAndroidDialogHelpers::registerNatives(env) @@ -934,11 +933,8 @@ Q_DECL_EXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void */*reserved*/) } QWindowSystemInterfacePrivate::TabletEvent::setPlatformSynthesizesMouse(false); - m_javaVM = vm; // attach qt main thread data to this thread - QObject threadSetter; - if (threadSetter.thread()) - threadSetter.thread()->setObjectName("QtMainLoopThread"); + QThread::currentThread()->setObjectName("QtMainLoopThread"); __android_log_print(ANDROID_LOG_INFO, "Qt", "qt started"); return JNI_VERSION_1_6; } diff --git a/src/plugins/platforms/android/androidjnimenu.cpp b/src/plugins/platforms/android/androidjnimenu.cpp index 7aaa5d921c..a8af3feeb5 100644 --- a/src/plugins/platforms/android/androidjnimenu.cpp +++ b/src/plugins/platforms/android/androidjnimenu.cpp @@ -378,11 +378,11 @@ namespace QtAndroidMenu return false; \ } - bool registerNatives(JNIEnv *env) + bool registerNatives(QJniEnvironment &env) { jclass appClass = applicationClass(); - if (env->RegisterNatives(appClass, methods, sizeof(methods) / sizeof(methods[0])) < 0) { + if (!env.registerNativeMethods(appClass, methods, sizeof(methods) / sizeof(methods[0]))) { __android_log_print(ANDROID_LOG_FATAL,"Qt", "RegisterNatives failed"); return false; } diff --git a/src/plugins/platforms/android/androidjnimenu.h b/src/plugins/platforms/android/androidjnimenu.h index 1645ce750b..308f34867c 100644 --- a/src/plugins/platforms/android/androidjnimenu.h +++ b/src/plugins/platforms/android/androidjnimenu.h @@ -15,6 +15,7 @@ class QAndroidPlatformMenuItem; class QWindow; class QRect; class QPoint; +class QJniEnvironment; namespace QtAndroidMenu { @@ -31,7 +32,7 @@ namespace QtAndroidMenu void removeMenuBar(QAndroidPlatformMenuBar *menuBar); // Menu support - bool registerNatives(JNIEnv *env); + bool registerNatives(QJniEnvironment &env); } QT_END_NAMESPACE diff --git a/src/plugins/platforms/android/qandroidplatformdialoghelpers.cpp b/src/plugins/platforms/android/qandroidplatformdialoghelpers.cpp index ec2199b727..2b9a1194d2 100644 --- a/src/plugins/platforms/android/qandroidplatformdialoghelpers.cpp +++ b/src/plugins/platforms/android/qandroidplatformdialoghelpers.cpp @@ -150,7 +150,7 @@ static void dialogResult(JNIEnv * /*env*/, jobject /*thiz*/, jlong handler, int QMetaObject::invokeMethod(object, "dialogResult", Qt::QueuedConnection, Q_ARG(int, buttonID)); } -static JNINativeMethod methods[] = { +static const JNINativeMethod methods[] = { {"dialogResult", "(JI)V", (void *)dialogResult} }; @@ -162,21 +162,19 @@ static JNINativeMethod methods[] = { return false; \ } -bool registerNatives(JNIEnv *env) +bool registerNatives(QJniEnvironment &env) { const char QtMessageHandlerHelperClassName[] = "org/qtproject/qt/android/QtMessageDialogHelper"; - QJniEnvironment qenv; - jclass clazz = qenv.findClass(QtMessageHandlerHelperClassName); + jclass clazz = env.findClass(QtMessageHandlerHelperClassName); if (!clazz) { __android_log_print(ANDROID_LOG_FATAL, QtAndroid::qtTagText(), QtAndroid::classErrorMsgFmt() , QtMessageHandlerHelperClassName); return false; } g_messageDialogHelperClass = static_cast(env->NewGlobalRef(clazz)); - FIND_AND_CHECK_CLASS("org/qtproject/qt/android/QtNativeDialogHelper"); - jclass appClass = static_cast(env->NewGlobalRef(clazz)); - if (env->RegisterNatives(appClass, methods, sizeof(methods) / sizeof(methods[0])) < 0) { + if (!env.registerNativeMethods("org/qtproject/qt/android/QtNativeDialogHelper", + methods, sizeof(methods) / sizeof(methods[0]))) { __android_log_print(ANDROID_LOG_FATAL, "Qt", "RegisterNatives failed"); return false; } diff --git a/src/plugins/platforms/android/qandroidplatformdialoghelpers.h b/src/plugins/platforms/android/qandroidplatformdialoghelpers.h index f06d2a9990..86f1cf77e7 100644 --- a/src/plugins/platforms/android/qandroidplatformdialoghelpers.h +++ b/src/plugins/platforms/android/qandroidplatformdialoghelpers.h @@ -8,12 +8,13 @@ #include #include -#include #include #include QT_BEGIN_NAMESPACE +class QJniEnvironment; + namespace QtAndroidDialogHelpers { class QAndroidPlatformMessageDialogHelper: public QPlatformMessageDialogHelper @@ -41,7 +42,7 @@ private: }; -bool registerNatives(JNIEnv *env); +bool registerNatives(QJniEnvironment &env); }