From 6e813c407295f140da43245e8961a0ca9b15833d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinja=20Paavosepp=C3=A4?= Date: Tue, 23 Apr 2024 16:15:43 +0300 Subject: [PATCH] Android: Track QtLoader success better QtLoader doesn't really handle failures in a way that's visible outside the class itself. There is the abstract finish() method subclasses can implement to handle clean up in the loader itself, but it's not visible outside, which leads to trying to execute paths of code we know are not going to work, since if the Qt libraries are not loaded the Qt app is not going to work. Make QtLoader throw an exception if invalid arguments are passed to it, as there is no way for the loader to do its job properly if the passed Context is not what we expect it to be, so might as well exit early. Make the loadQtLibraries() method return a boolean, letting the caller know whether the loading was successful or not. As a result, the finish() method can be removed, and the caller can handle failure as appropriate for it. Task-number: QTBUG-124114 Change-Id: I2dc1a0846eb404112f88e9da365db2ab071f4317 Reviewed-by: Assam Boudjelthia (cherry picked from commit 2b48614f68cbf98d6597819749b732556c32cb44) Reviewed-by: Qt Cherry-pick Bot --- .../qtproject/qt/android/QtActivityBase.java | 37 ++++++++-- .../qt/android/QtActivityLoader.java | 33 +-------- .../qt/android/QtEmbeddedLoader.java | 7 +- .../org/qtproject/qt/android/QtLoader.java | 67 +++++++++---------- .../qtproject/qt/android/QtServiceBase.java | 20 ++++-- .../qtproject/qt/android/QtServiceLoader.java | 19 ++---- .../src/org/qtproject/qt/android/QtView.java | 25 +++++-- 7 files changed, 105 insertions(+), 103 deletions(-) diff --git a/src/android/jar/src/org/qtproject/qt/android/QtActivityBase.java b/src/android/jar/src/org/qtproject/qt/android/QtActivityBase.java index dccaf45a05e..e2dae9eb2f0 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtActivityBase.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtActivityBase.java @@ -3,9 +3,13 @@ package org.qtproject.qt.android; +import android.annotation.SuppressLint; import android.app.Activity; +import android.app.AlertDialog; +import android.app.Dialog; import android.content.Intent; import android.content.res.Configuration; +import android.content.res.Resources; import android.net.Uri; import android.os.Build; import android.os.Bundle; @@ -18,6 +22,7 @@ import android.view.MenuItem; import android.view.MotionEvent; import android.view.View; import android.view.Window; +import java.lang.IllegalArgumentException; public class QtActivityBase extends Activity implements QtNative.AppStateDetailsListener { @@ -101,12 +106,22 @@ public class QtActivityBase extends Activity implements QtNative.AppStateDetails addReferrer(getIntent()); - QtActivityLoader loader = new QtActivityLoader(this); - loader.appendApplicationParameters(m_applicationParams); + try { + QtActivityLoader loader = new QtActivityLoader(this); + loader.appendApplicationParameters(m_applicationParams); - loader.loadQtLibraries(); - m_delegate.startNativeApplication(loader.getApplicationParameters(), - loader.getMainLibraryPath()); + if (loader.loadQtLibraries()) { + m_delegate.startNativeApplication(loader.getApplicationParameters(), + loader.getMainLibraryPath()); + } else { + showErrorDialog(); + finish(); + } + } catch (IllegalArgumentException e) { + e.printStackTrace(); + showErrorDialog(); + finish(); + } } @Override @@ -334,4 +349,16 @@ public class QtActivityBase extends Activity implements QtNative.AppStateDetails { return m_delegate; } + + private void showErrorDialog() { + Resources resources = getResources(); + String packageName = getPackageName(); + AlertDialog errorDialog = new AlertDialog.Builder(this).create(); + @SuppressLint("DiscouragedApi") int id = resources.getIdentifier( + "fatal_error_msg", "string", packageName); + errorDialog.setMessage(resources.getString(id)); + errorDialog.setButton(Dialog.BUTTON_POSITIVE, resources.getString(android.R.string.ok), + (dialog, which) -> finish()); + errorDialog.show(); + } } diff --git a/src/android/jar/src/org/qtproject/qt/android/QtActivityLoader.java b/src/android/jar/src/org/qtproject/qt/android/QtActivityLoader.java index 9313ecc3f09..46fe95543b8 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtActivityLoader.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtActivityLoader.java @@ -4,22 +4,19 @@ package org.qtproject.qt.android; -import android.annotation.SuppressLint; import android.app.Activity; -import android.app.AlertDialog; -import android.app.Dialog; import android.content.Context; import android.content.ContextWrapper; import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; -import android.content.res.Resources; import android.os.Bundle; import android.system.Os; import android.util.Base64; import android.util.DisplayMetrics; import android.util.Log; +import java.lang.IllegalArgumentException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; @@ -27,7 +24,7 @@ import java.nio.charset.StandardCharsets; class QtActivityLoader extends QtLoader { private final Activity m_activity; - QtActivityLoader(Activity activity) + QtActivityLoader(Activity activity) throws IllegalArgumentException { super(new ContextWrapper(activity)); m_activity = activity; @@ -35,32 +32,6 @@ class QtActivityLoader extends QtLoader { extractContextMetaData(m_activity); } - private void showErrorDialog() { - if (m_activity == null) { - Log.w(QtTAG, "cannot show the error dialog from a null activity object"); - return; - } - Resources resources = m_activity.getResources(); - String packageName = m_activity.getPackageName(); - AlertDialog errorDialog = new AlertDialog.Builder(m_activity).create(); - @SuppressLint("DiscouragedApi") int id = resources.getIdentifier( - "fatal_error_msg", "string", packageName); - errorDialog.setMessage(resources.getString(id)); - errorDialog.setButton(Dialog.BUTTON_POSITIVE, resources.getString(android.R.string.ok), - (dialog, which) -> finish()); - errorDialog.show(); - } - - @Override - protected void finish() { - if (m_activity == null) { - Log.w(QtTAG, "finish() called when activity object is null"); - return; - } - showErrorDialog(); - m_activity.finish(); - } - private String getDecodedUtfString(String str) { byte[] decodedExtraEnvVars = Base64.decode(str, Base64.DEFAULT); diff --git a/src/android/jar/src/org/qtproject/qt/android/QtEmbeddedLoader.java b/src/android/jar/src/org/qtproject/qt/android/QtEmbeddedLoader.java index 6defd8db85d..768f433993b 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtEmbeddedLoader.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtEmbeddedLoader.java @@ -19,6 +19,7 @@ import android.view.SurfaceView; import java.io.File; import java.io.FileOutputStream; +import java.lang.IllegalArgumentException; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -34,7 +35,7 @@ import android.content.res.Resources; class QtEmbeddedLoader extends QtLoader { private static final String TAG = "QtEmbeddedLoader"; - QtEmbeddedLoader(Context context) { + QtEmbeddedLoader(Context context) throws IllegalArgumentException { super(new ContextWrapper(context)); // TODO Service context handling QTBUG-118874 int displayDensity = context.getResources().getDisplayMetrics().densityDpi; @@ -43,8 +44,4 @@ class QtEmbeddedLoader extends QtLoader { setEnvironmentVariable("ANDROID_STYLE_PATH", stylePath); setEnvironmentVariable("QT_ANDROID_NO_EXIT_CALL", String.valueOf(true)); } - - @Override - protected void finish() { - } } diff --git a/src/android/jar/src/org/qtproject/qt/android/QtLoader.java b/src/android/jar/src/org/qtproject/qt/android/QtLoader.java index 4fce6422d16..7747e16b023 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtLoader.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtLoader.java @@ -13,12 +13,14 @@ import android.content.ContextWrapper; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.ComponentInfo; +import android.content.pm.PackageManager.NameNotFoundException; import android.content.res.Resources; import android.os.Build; import android.os.Bundle; import android.util.Log; import java.io.File; +import java.lang.IllegalArgumentException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; @@ -52,44 +54,41 @@ abstract class QtLoader { * Initializes the class loader since it doesn't rely on anything * other than the context. * Also, we can already initialize the static classes contexts here. + * @throws IllegalArgumentException if the given Context is not either an Activity or Service, + * or no ComponentInfo could be found for it **/ - public QtLoader(ContextWrapper context) { + public QtLoader(ContextWrapper context) throws IllegalArgumentException { m_resources = context.getResources(); m_packageName = context.getPackageName(); final Context baseContext = context.getBaseContext(); + if (!(baseContext instanceof Activity || baseContext instanceof Service)) { + throw new IllegalArgumentException("QtLoader: Context is not an instance of " + + "Activity or Service"); + } initClassLoader(baseContext); initStaticClasses(baseContext); - initContextInfo(baseContext); + try { + initContextInfo(baseContext); + } catch (NameNotFoundException e) { + throw new IllegalArgumentException("QtLoader: No ComponentInfo found for given " + + "Context", e); + } } - /** - * Implements the logic for finish the extended context, mostly called - * in error cases. - **/ - abstract protected void finish(); - /** * Initializes the context info instance which is used to retrieve * the app metadata from the AndroidManifest.xml or other xml resources. * Some values are dependent on the context being an Activity or Service. **/ - protected void initContextInfo(Context context) { - try { - if (context instanceof Activity) { - m_contextInfo = context.getPackageManager().getActivityInfo( - ((Activity)context).getComponentName(), PackageManager.GET_META_DATA); - } else if (context instanceof Service) { - m_contextInfo = context.getPackageManager().getServiceInfo( - new ComponentName(context, context.getClass()), - PackageManager.GET_META_DATA); - } else { - Log.w(QtTAG, "Context is not an instance of Activity or Service, could not get " + - "context info for it"); - } - } catch (Exception e) { - e.printStackTrace(); - finish(); + protected void initContextInfo(Context context) throws NameNotFoundException { + if (context instanceof Activity) { + m_contextInfo = context.getPackageManager().getActivityInfo( + ((Activity)context).getComponentName(), PackageManager.GET_META_DATA); + } else if (context instanceof Service) { + m_contextInfo = context.getPackageManager().getServiceInfo( + new ComponentName(context, context.getClass()), + PackageManager.GET_META_DATA); } } @@ -333,9 +332,6 @@ abstract class QtLoader { * * @noinspection SameParameterValue*/ private String getApplicationMetaData(String key) { - if (m_contextInfo == null) - return ""; - ApplicationInfo applicationInfo = m_contextInfo.applicationInfo; if (applicationInfo == null) return ""; @@ -417,11 +413,10 @@ abstract class QtLoader { /** * Loads all Qt native bundled libraries and main library. **/ - public void loadQtLibraries() { + public boolean loadQtLibraries() { if (!useLocalQtLibs()) { Log.w(QtTAG, "Use local Qt libs is false"); - finish(); - return; + return false; } if (m_nativeLibrariesDir == null) @@ -429,8 +424,7 @@ abstract class QtLoader { if (m_nativeLibrariesDir == null || m_nativeLibrariesDir.isEmpty()) { Log.e(QtTAG, "The native libraries directory is null or empty"); - finish(); - return; + return false; } setEnvironmentVariable("QT_PLUGIN_PATH", m_nativeLibrariesDir); @@ -449,16 +443,14 @@ abstract class QtLoader { if (!loadLibraries(nativeLibraries)) { Log.e(QtTAG, "Loading Qt native libraries failed"); - finish(); - return; + return false; } // add all bundled Qt libs to loader params ArrayList bundledLibraries = new ArrayList<>(preferredAbiLibs(getBundledLibs())); if (!loadLibraries(bundledLibraries)) { Log.e(QtTAG, "Loading Qt bundled libraries failed"); - finish(); - return; + return false; } if (m_mainLibName == null) @@ -466,8 +458,9 @@ abstract class QtLoader { // Load main lib if (!loadMainLibrary(m_mainLibName + "_" + m_preferredAbi)) { Log.e(QtTAG, "Loading main library failed"); - finish(); + return false; } + return true; } // Loading libraries using System.load() uses full lib paths diff --git a/src/android/jar/src/org/qtproject/qt/android/QtServiceBase.java b/src/android/jar/src/org/qtproject/qt/android/QtServiceBase.java index b69dea44166..395b19931b7 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtServiceBase.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtServiceBase.java @@ -8,6 +8,8 @@ import android.content.Intent; import android.os.IBinder; import android.util.Log; +import java.lang.IllegalArgumentException; + public class QtServiceBase extends Service { @Override public void onCreate() @@ -25,10 +27,20 @@ public class QtServiceBase extends Service { QtNative.setService(this); - QtServiceLoader loader = new QtServiceLoader(this); - loader.loadQtLibraries(); - QtNative.startApplication(loader.getApplicationParameters(), loader.getMainLibraryPath()); - QtNative.setApplicationState(QtNative.ApplicationState.ApplicationHidden); + try { + QtServiceLoader loader = new QtServiceLoader(this); + if (loader.loadQtLibraries()) { + QtNative.startApplication(loader.getApplicationParameters(), + loader.getMainLibraryPath()); + QtNative.setApplicationState(QtNative.ApplicationState.ApplicationHidden); + } else { + Log.w(QtNative.QtTAG, "QtServiceLoader: failed to load Qt libraries"); + stopSelf(); + } + } catch (IllegalArgumentException e) { + Log.w(QtNative.QtTAG, e.getMessage()); + stopSelf(); + } } @Override diff --git a/src/android/jar/src/org/qtproject/qt/android/QtServiceLoader.java b/src/android/jar/src/org/qtproject/qt/android/QtServiceLoader.java index 63cc3a998f2..23c7b3fa264 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtServiceLoader.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtServiceLoader.java @@ -6,23 +6,12 @@ package org.qtproject.qt.android; import android.app.Service; import android.content.ContextWrapper; -import android.util.Log; + +import java.lang.IllegalArgumentException; class QtServiceLoader extends QtLoader { - private final Service m_service; - - QtServiceLoader(Service service) { + QtServiceLoader(Service service) throws IllegalArgumentException { super(new ContextWrapper(service)); - m_service = service; - - extractContextMetaData(m_service); - } - - @Override - protected void finish() { - if (m_service != null) - m_service.stopSelf(); - else - Log.w(QtTAG, "finish() called when service object is null"); + extractContextMetaData(service); } } diff --git a/src/android/jar/src/org/qtproject/qt/android/QtView.java b/src/android/jar/src/org/qtproject/qt/android/QtView.java index cf06ce5e3d7..91756b47532 100644 --- a/src/android/jar/src/org/qtproject/qt/android/QtView.java +++ b/src/android/jar/src/org/qtproject/qt/android/QtView.java @@ -10,6 +10,7 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.content.res.Resources; import android.os.Handler; import android.os.Looper; +import android.util.Log; import android.view.View; import android.view.ViewGroup; @@ -83,7 +84,6 @@ abstract class QtView extends ViewGroup { throw new InvalidParameterException("QtView: argument 'appLibName' may not be empty "+ "or null"); } - loadQtLibraries(appLibName); } @@ -143,12 +143,25 @@ abstract class QtView extends ViewGroup { } void loadQtLibraries(String appLibName) { - QtEmbeddedLoader loader = new QtEmbeddedLoader(getContext()); + QtEmbeddedLoader loader = null; + try { + loader = new QtEmbeddedLoader(getContext()); + } catch (IllegalArgumentException e) { + Log.e(TAG, e.getMessage()); + QtEmbeddedViewInterfaceFactory.remove(getContext()); + return; + } + loader.setMainLibraryName(appLibName); - loader.loadQtLibraries(); - // Start Native Qt application - m_viewInterface.startQtApplication(loader.getApplicationParameters(), - loader.getMainLibraryPath()); + if (loader.loadQtLibraries()) { + // Start Native Qt application + m_viewInterface.startQtApplication(loader.getApplicationParameters(), + loader.getMainLibraryPath()); + } else { + // If we weren't able to load the libraries, remove the delegate from the factory + // as it's holding a reference to the Context, and we don't want it leaked + QtEmbeddedViewInterfaceFactory.remove(getContext()); + } } void setWindowReference(long windowReference) {