Kiwix-android: Make Kiwix on Android resizable

[alexanderadam]

Is your feature request related to a problem? Please describe.

I'm using Samsung DeX, which gives a desktop like experience and makes use of Android windowing support.
Kiwix works fine on that except that it isn't resizable.

Describe the solution you'd like

According to the Samsung documentation this on how to enable window resizing for Android applications:

<application
    android:resizeableActivity="true">
</application>

This documentation also has more tweaks and information on how to improve the desktop experience in case you're interested.
They also have an app testing guide (although some apps are fine with this guide but won't work on "real" DeX sometimes).

GitNex also added a few other lines (I don't know what's relevant for Kiwix but it doesn't seem to be much either way):

 app/src/main/AndroidManifest.xml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 8518fe2c..f8c2367c 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -12,6 +12,7 @@
         android:icon="@mipmap/app_logo"
         android:label="@string/app_name"
         android:networkSecurityConfig="@xml/network_security_config"
+        android:resizeableActivity="true"
         android:roundIcon="@mipmap/app_logo_round"
         android:supportsRtl="true"
         tools:targetApi="n">
@@ -60,7 +61,7 @@
             android:name=".activities.RepoDetailActivity"
             android:label="@string/title_activity_repo_detail"
             android:theme="@style/AppTheme.NoActionBar" />
-        <activity android:name=".activities.MainActivity" android:theme="@android:style/Theme.NoTitleBar">
+        <activity android:name=".activities.MainActivity" android:theme="@android:style/Theme.NoTitleBar" android:configChanges="orientation|screenSize|smallestScreenSize|density|screenLayout|uiMode|keyboard|keyboardHidden|navigation">
             <intent-filter>
                 <action android:name="android.intent.action.MAIN" />

@@ -84,6 +85,11 @@
         <activity android:name=".activities.AddNewTeamMemberActivity" />
         <activity android:name=".activities.SettingsDraftsActivity" />

+        <!-- Version < 3.0. DeX Mode and Screen Mirroring support -->
+        <meta-data android:name="com.samsung.android.keepalive.density" android:value="true"/>
+        <!-- Version >= 3.0. DeX Dual Mode support -->
+        <meta-data android:name="com.samsung.android.multidisplay.keep_process_alive" android:value="true"/>
+
     </application>

 </manifest>

Describe alternatives you've considered

Not resizing

Additional context

At the moment the resizing button is greyed out and the application is always nearly squarish:

PS: I know that it is possible to _force_ all apps to be resizable via Samsung Labs > Force Resize but it would obviously better to make the application natively resizable to create less friction.

[kelson42]

@macgills @abdulwd Would we be able do do this without having bad side effects.

[abdulwd]

We have to test if doing so breaks some layouts on some screen sizes.

[macgills]

It is all in the testing of it as the implementation is as simple as the one attribute. It could lead to some weirdness as then we would be supporting multiwindow in Android. May have to update our Activity tags with something like this

<layout android:defaultHeight="500dp"
          android:defaultWidth="600dp"
          android:gravity="top|end"
          android:minHeight="450dp"
          android:minWidth="300dp" />

@alexanderadam did you enable force resize and if you did were there any standout issues in your experience?

[alexanderadam]

@alexanderadam did you enable force resize and if you did were there any standout issues in your experience?

No, I didn't enable it yet but I could test it tomorrow if you want.

[macgills]

@alexanderadam it would be incredibly helpful if you could

[alexanderadam]

So I tested Kiwix with the Force Resize mode and every time I resize it a message appears that says:

To change the size of this app, it needs to be restarted. Make sure you've saved your data before restarting. If this app doesn't support landscape mode, it won't maximise correctly.

And this message is localized depending what language I've set within Kiwix.

This way I can _not_ resize Kiwix by touching the corners or borders (like I can with VLC, Firefox or others) _but_ I can resize it via Maximize/Minimize button. Which is obviously still a better user experience than before.

PPS: I'm not sure whether this is relevant but this is how GitNex fixed the issue and this is how NewPipe fixed it. Both are now also resizable with dragging border and corners.

[kelson42]

@abdulwd @macgills What are the next steps here?

[macgills]

Implement and test, It is in the milestone so it should get done soon, depending on priorities I could move it to the top of my list

[kelson42]

@macgills Not TOP prio, but seems relatively simple, so would be nice-to-have.

[macgills]

@alexanderadam could I trouble you with testing this apk? apologies for the delay in getting this ticket seen to

[alexanderadam]

@macgills, thank you so much for doing this! :pray:

Resizing itself is working perfectly _but_ Canary complained about three distinct leaks:

[macgills]

tch, can you share the text of those leaks here? They might not be our problem, also this branch is a bit out of date, we fixed some of the memory leaks during the new navigation feature

[macgills]

I also updated that apk so it is now in line with what is on develop+resizing

[alexanderadam]

tch, can you share the text of those leaks here?

I try to. But suddenly I cannot reproduce it anymore, although I'm doing the exact things as before!

I also updated that apk so it is now in line with what is on develop+resizing

Is it the same URL as before?

PS: Was it necessary that I uninstalled the stable kiwix version before? Because I did (just to be sure) and now my former data is gone. No biggie — I just want to know it for the next time. :wink:

[macgills]

Same URL.

It shouldn't have been necessary to uninstall the old app but sometimes it may be, there are a couple of factors

[alexanderadam]

The Heap Dump file is too bug for GitHub. Is the Heat Dump analysis text sufficient for you or do you need something else?

Click to expand

====================================
HEAP ANALYSIS RESULT
====================================
1 APPLICATION LEAKS

References underlined with "~~~" are likely causes.
Learn more at https://squ.re/leaks.

31772 bytes retained by leaking objects
Signature: f2853c31a197b19f21a5c358c443bd133fb9bb6b
┬───
│ GC Root: System class
│
├─ android.view.WindowManagerGlobal class
│    Leaking: NO (FrameLayout↓ is not leaking and a class is never leaking)
│    ↓ static WindowManagerGlobal.sDefaultWindowManager
├─ android.view.WindowManagerGlobal instance
│    Leaking: NO (FrameLayout↓ is not leaking)
│    ↓ WindowManagerGlobal.mViews
├─ java.util.ArrayList instance
│    Leaking: NO (FrameLayout↓ is not leaking)
│    ↓ ArrayList.elementData
├─ java.lang.Object[] array
│    Leaking: NO (FrameLayout↓ is not leaking)
│    ↓ Object[].[2]
├─ android.widget.FrameLayout instance
│    Leaking: NO (ImageView↓ is not leaking and View attached)
│    mContext instance of org.kiwix.kiwixmobile.KiwixApp, not wrapping activity
│    Parent android.view.ViewRootImpl not a android.view.View
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    ↓ FrameLayout.mChildren
├─ android.view.View[] array
│    Leaking: NO (ImageView↓ is not leaking)
│    ↓ View[].[1]
├─ android.widget.ImageView instance
│    Leaking: NO (Toast↓ is not leaking and View attached)
│    mContext instance of org.kiwix.kiwixmobile.KiwixApp, not wrapping activity
│    View.parent android.widget.FrameLayout attached as well
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mID = R.id.leak_canary_toast_icon
│    View.mWindowAttachCount = 1
│    ↓ ImageView.mAnimator
├─ android.view.ViewPropertyAnimator instance
│    Leaking: NO (Toast↓ is not leaking)
│    ↓ ViewPropertyAnimator.mListener
├─ leakcanary.internal.AndroidHeapDumper$showToast$1$1 instance
│    Leaking: NO (Toast↓ is not leaking)
│    Anonymous subclass of android.animation.AnimatorListenerAdapter
│    ↓ AndroidHeapDumper$showToast$1$1.$toast
├─ android.widget.Toast instance
│    Leaking: NO (This toast is showing (Toast.mTN.mWM != null && Toast.mTN.mView != null))
│    ↓ Toast.mContext
│            ~~~~~~~~
╰→ org.kiwix.kiwixmobile.settings.KiwixSettingsActivity instance
​     Leaking: YES (ObjectWatcher was watching this because org.kiwix.kiwixmobile.settings.KiwixSettingsActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
​     key = a54a706f-e5ee-495c-b83c-a1eeada2b9cc
​     watchDurationMillis = -304
​     retainedDurationMillis = -1
====================================
2 LIBRARY LEAKS

A Library Leak is a leak caused by a known bug in 3rd party code that you do not have control over.
See https://square.github.io/leakcanary/fundamentals-how-leakcanary-works/#4-categorizing-leaks

Leak pattern: instance field com.android.internal.policy.MultiWindowDecorSupport#mWindow
Description: DecorView isn't leaking but its mDecorViewSupport field holds
a MultiWindowDecorSupport which has a mWindow field which holds a leaking PhoneWindow.
DecorView.mDecorViewSupport doesn't exist in AOSP.
Filed here: https://github.com/square/leakcanary/issues/1819
73949 bytes retained by leaking objects
Displaying only 1 leak trace out of 2 with the same signature
Signature: 833cf967a3de84241180e16a7b1485d33cc5b94f
┬───
│ GC Root: System class
│
├─ android.view.WindowManagerGlobal class
│    Leaking: NO (DecorView↓ is not leaking and a class is never leaking)
│    ↓ static WindowManagerGlobal.sDefaultWindowManager
├─ android.view.WindowManagerGlobal instance
│    Leaking: NO (DecorView↓ is not leaking)
│    ↓ WindowManagerGlobal.mViews
├─ java.util.ArrayList instance
│    Leaking: NO (DecorView↓ is not leaking)
│    ↓ ArrayList.elementData
├─ java.lang.Object[] array
│    Leaking: NO (DecorView↓ is not leaking)
│    ↓ Object[].[0]
├─ com.android.internal.policy.DecorView instance
│    Leaking: NO (View attached)
│    mContext instance of com.android.internal.policy.DecorContext, wrapping activity org.kiwix.kiwixmobile.main.KiwixMainActivity with mDestroyed = false
│    Parent android.view.ViewRootImpl not a android.view.View
│    View#mParent is set
│    View#mAttachInfo is not null (view attached)
│    View.mWindowAttachCount = 1
│    ↓ DecorView.mDecorSupport
│                ~~~~~~~~~~~~~
├─ com.android.internal.policy.MultiWindowDecorSupport instance
│    Leaking: UNKNOWN
│    ↓ MultiWindowDecorSupport.mWindow
│                              ~~~~~~~
├─ com.android.internal.policy.PhoneWindow instance
│    Leaking: YES (Window#mDestroyed is true)
│    ↓ PhoneWindow.mContext
╰→ org.kiwix.kiwixmobile.main.KiwixMainActivity instance
​     Leaking: YES (ObjectWatcher was watching this because org.kiwix.kiwixmobile.main.KiwixMainActivity received Activity#onDestroy() callback and Activity#mDestroyed is true)
​     key = d68fe8ee-eb9d-433b-a730-69a93a2f16a5
​     watchDurationMillis = 5174
​     retainedDurationMillis = 173


Leak pattern: instance field android.os.Message#next
Description: A thread waiting on a blocking queue will leak the last dequeued object as a stack local reference. So when a HandlerThread becomes idle, it keeps a local reference to the last message it received. That message then gets recycled and can be used again. As long as all messages are recycled after being used, this won't be a problem, because these references are cleared when being recycled. However, dialogs create template Message instances to be copied when a message needs to be sent. These Message templates holds references to the dialog listeners, which most likely leads to holding a reference onto the activity in some way. Dialogs never recycle their template Message, assuming these Message instances will get GCed when the dialog is GCed. The combination of these two things creates a high potential for memory leaks as soon as you use dialogs. These memory leaks might be temporary, but some handler threads sleep for a long time. To fix this, you could post empty messages to the idle handler threads from time to time. This won't be easy because you cannot access all handler threads, but a library that is widely used should consider doing this for its own handler threads. This leaks has been shown to happen in both Dalvik and ART.
759953 bytes retained by leaking objects
Displaying only 1 leak trace out of 2 with the same signature
Signature: f1f8f8e79784a6d2f5de8f95e4bba4aee6e35bc
┬───
│ GC Root: System class
│
├─ android.app.ActivityThread class
│    Leaking: NO (MessageQueue↓ is not leaking and a class is never leaking)
│    ↓ static ActivityThread.sMainThreadHandler
├─ android.app.ActivityThread$H instance
│    Leaking: NO (MessageQueue↓ is not leaking)
│    ↓ ActivityThread$H.mQueue
├─ android.os.MessageQueue instance
│    Leaking: NO (MessageQueue#mQuitting is false)
│    ↓ MessageQueue.mMessages
│                   ~~~~~~~~~
├─ android.os.Message instance
│    Leaking: UNKNOWN
│    ↓ Message.next
│              ~~~~
├─ android.os.Message instance
│    Leaking: UNKNOWN
│    ↓ Message.next
│              ~~~~
├─ android.os.Message instance
│    Leaking: UNKNOWN
│    ↓ Message.callback
│              ~~~~~~~~
├─ androidx.core.content.res.ResourcesCompat$FontCallback$2 instance
│    Leaking: UNKNOWN
│    Anonymous class implementing java.lang.Runnable
│    ↓ ResourcesCompat$FontCallback$2.this$0
│                                     ~~~~~~
├─ androidx.appcompat.widget.AppCompatTextHelper$1 instance
│    Leaking: UNKNOWN
│    Anonymous subclass of androidx.core.content.res.ResourcesCompat$FontCallback
│    ↓ AppCompatTextHelper$1.this$0
│                            ~~~~~~
├─ androidx.appcompat.widget.AppCompatTextHelper instance
│    Leaking: UNKNOWN
│    ↓ AppCompatTextHelper.mView
│                          ~~~~~
├─ com.google.android.material.textview.MaterialTextView instance
│    Leaking: YES (View detached and has parent)
│    mContext instance of org.kiwix.kiwixmobile.settings.KiwixSettingsActivity with mDestroyed = false
│    View#mParent is set
│    View#mAttachInfo is null (view detached)
│    View.mID = R.id.null
│    View.mWindowAttachCount = 1
│    ↓ MaterialTextView.mParent
╰→ android.widget.LinearLayout instance
​     Leaking: YES (ObjectWatcher was watching this because org.kiwix.kiwixmobile.settings.KiwixPrefsFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
​     key = 78f76bdd-ecd7-4378-b326-efa09c55fb2e
​     watchDurationMillis = -434
​     retainedDurationMillis = -1
​     mContext instance of org.kiwix.kiwixmobile.settings.KiwixSettingsActivity with mDestroyed = false
​     View#mParent is null
​     View#mAttachInfo is null (view detached)
​     View.mWindowAttachCount = 1

====================================
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 29
Build.MANUFACTURER: samsung
LeakCanary version: 2.4
App process name: org.kiwix.kiwixmobile
Analysis duration: 14444 ms
Heap dump file path: /data/user/0/org.kiwix.kiwixmobile/files/leakcanary/2020-09-10_12-01-04_480.hprof
Heap dump timestamp: 1599732081207
====================================

[macgills]

Yeah these look like Samsung brand leaks alright. Nothing to do on our end I think. You won't see these leak reports on non debug version of kiwix

[macgills]

Okay I have set this for review, it will be in our next release. Thanks for all the help @alexanderadam

[alexanderadam]

Thanks for all the help

I definitely have to thank you, folks! :pray:
You did wonderful work and Kiwix is a great and useful app!