fix(MM-62375): OOM on gif #8573

rahimrahman · 2025-02-08T18:48:05Z

Summary

Ticket Link

Fixes: https://mattermost.atlassian.net/browse/MM-62375

See changes in my own fork of APNG4Android (until I have permission to post to mattermost/APNG4Android): rahimrahman/APNG4Android@74dd31c

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.
Have run E2E tests by adding label E2E iOS tests for PR.

Device Information

This PR was tested on:

Google Pixel 6a, Android 14

Screenshots

Release Note

rahimrahman · 2025-02-08T18:54:23Z

patches/expo-image+2.0.4.patch

+  implementation project(':frameanimation')
+  implementation project(':gif')
+  implementation("com.github.penfeizhou.android.animation:glide-plugin:3.0.2") {
+    exclude group: 'com.github.penfeizhou.android.animation', module: 'frameanimation'
+    exclude group: 'com.github.penfeizhou.android.animation', module: 'gif'
+  }


The fixes are in frameanimation & gif.

Now in this PR (that will not get merged).

rahimrahman · 2025-02-08T18:55:39Z

scripts/postinstall.sh

+APNG4Android="node_modules/APNG4Android"
+if [ ! -z "$APNG4Android" ]; then
+    git clone -b "fix/MM-62375-oom-on-gif" --single-branch https://github.com/rahimrahman/APNG4Android.git "$APNG4Android";
+fi


Can't do npm i since APNG4Android doesn't have package.json.

TODO: will replace with mattermost/APNG4Android (as soon as permission to write is granted)

TODO: figure out if I can use SHA instead of branch.

No you cannot clone a specific commit, but you can clone the branch and then do a checkout

You could also use --depth=1 so that it only clones the latest one

rahimrahman · 2025-02-08T18:56:31Z

android/settings.gradle

+include ':frameanimation'
+project(':frameanimation').projectDir = new File('../node_modules/APNG4Android/frameanimation')
+include ':gif'
+project(':gif').projectDir = new File('../node_modules/APNG4Android/gif')


In order to get this as project in expo-image/android/build.gradle, need to declare frameanimation and gif here.

rahimrahman · 2025-02-08T18:57:37Z

patches/expo-image+2.0.4.patch

+    exclude group: 'com.github.penfeizhou.android.animation', module: 'frameanimation'
+    exclude group: 'com.github.penfeizhou.android.animation', module: 'gif'


Without exclusions, it will cause duplicate symbols error. Since glide-plugin will have frameanimation (required module) in other modules (apng, webp, etc).

rahimrahman · 2025-02-08T20:38:29Z

.github/actions/prepare-node-deps/action.yaml

+      run: |
+        echo "::group::clone-APNG4Android"
+        if [ ! -d "$APNG4Android" ]; then
+          git clone -b "fix/MM-62375-oom-on-gif" --single-branch https://github.com/rahimrahman/APNG4Android.git "$APNG4Android";


Is there any benefit of putting it into node_modules instead of a custom folder on the root path of the app?

TODO: Will replace this with the correct repo (mattermost/APNG4Android)

Irrelevant in my opinion, but of it works here no need to change

I like node_modules because it's easier to do the npm run clean and it'll auto-clean this library as well. But I could also have added an extra line to clean the folder wherever it might be in scripts/clean.sh as well.

I'll stick with node_modules.

enahum · 2025-02-10T14:42:27Z

package.json

@@ -54,7 +54,7 @@
    "expo-application": "6.0.1",
    "expo-crypto": "14.0.1",
    "expo-device": "7.0.1",
-    "expo-image": "2.0.3",
+    "expo-image": "2.0.4",


Any reason to bump this? Just curious

I made a mistake when reading the changelog. And looking at the changes, I thought the addition of try..catch was going to help, but it didn't. Looking deeper, I noticed that it was already implemented in 2.0.3, not a new change in 2.0.4.

Changelog for 2.0.4:

## 2.0.4 — 2025-01-10 _This version does not introduce any user-facing changes._

So it's pretty moot.

enahum · 2025-02-10T14:48:28Z

scripts/postinstall.sh

+APNG4Android="node_modules/APNG4Android"
+if [ ! -z "$APNG4Android" ]; then
+    git clone -b "fix/MM-62375-oom-on-gif" --single-branch https://github.com/rahimrahman/APNG4Android.git "$APNG4Android";
+fi


No you cannot clone a specific commit, but you can clone the branch and then do a checkout

enahum · 2025-02-10T14:49:13Z

scripts/postinstall.sh

+APNG4Android="node_modules/APNG4Android"
+if [ ! -z "$APNG4Android" ]; then
+    git clone -b "fix/MM-62375-oom-on-gif" --single-branch https://github.com/rahimrahman/APNG4Android.git "$APNG4Android";
+fi


You could also use --depth=1 so that it only clones the latest one

rahimrahman · 2025-02-10T18:24:56Z

@yasserfaraazkhan I'm flagging you for QA review, but @enzowritescode has done the test with his Android device and ✅ it in our private DM.

You're welcome to test this as well for redundancy.

fix(MM-62375): OOM on gif

12b3757

mm-cloud-bot added kind/bug Categorizes issue or PR as related to a bug. release-note labels Feb 8, 2025

rahimrahman commented Feb 8, 2025

View reviewed changes

rahimrahman requested review from enahum and larkox February 8, 2025 18:58

rahimrahman added the Build Apps for PR Build the mobile app for iOS and Android to test label Feb 8, 2025

add APNG4Android into github action script to download

ea591bf

rahimrahman added Build App for Android Build the mobile app for Android and removed Build Apps for PR Build the mobile app for iOS and Android to test labels Feb 8, 2025

rahimrahman commented Feb 8, 2025

View reviewed changes

rahimrahman added Build App for Android Build the mobile app for Android and removed Build App for Android Build the mobile app for Android labels Feb 9, 2025

rahimrahman requested a review from enzowritescode February 9, 2025 17:23

larkox approved these changes Feb 10, 2025

View reviewed changes

enahum approved these changes Feb 10, 2025

View reviewed changes

rahimrahman added the 3: Security Review Review requested from Security Team label Feb 10, 2025

rahimrahman mentioned this pull request Feb 10, 2025

fix(MM-62375) OOM on gif with build updates mattermost/APNG4Android#1

Open

change to mattermost/APNG4Android

24cf5bd

rahimrahman added Build Apps for PR Build the mobile app for iOS and Android to test and removed Build App for Android Build the mobile app for Android labels Feb 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(MM-62375): OOM on gif #8573

fix(MM-62375): OOM on gif #8573

rahimrahman commented Feb 8, 2025

rahimrahman Feb 8, 2025 •

edited

Loading

rahimrahman Feb 8, 2025 •

edited

Loading

enahum Feb 10, 2025

enahum Feb 10, 2025

rahimrahman Feb 8, 2025

rahimrahman Feb 8, 2025

rahimrahman Feb 8, 2025 •

edited

Loading

enahum Feb 10, 2025

rahimrahman Feb 10, 2025 •

edited

Loading

enahum Feb 10, 2025

rahimrahman Feb 10, 2025

enahum Feb 10, 2025

enahum Feb 10, 2025

rahimrahman commented Feb 10, 2025

		+ exclude group: 'com.github.penfeizhou.android.animation', module: 'frameanimation'
		+ exclude group: 'com.github.penfeizhou.android.animation', module: 'gif'

fix(MM-62375): OOM on gif #8573

Are you sure you want to change the base?

fix(MM-62375): OOM on gif #8573

Conversation

rahimrahman commented Feb 8, 2025

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

rahimrahman Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

rahimrahman Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimrahman Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimrahman Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimrahman commented Feb 10, 2025

rahimrahman Feb 8, 2025 •

edited

Loading

rahimrahman Feb 8, 2025 •

edited

Loading

rahimrahman Feb 8, 2025 •

edited

Loading

rahimrahman Feb 10, 2025 •

edited

Loading