Improve signalwebp JNI.

dependencies.gradle

@@ -183,7 +183,9 @@ dependencyResolutionManagement {
             library('androidx-test-core-ktx', 'androidx.test', 'core-ktx').versionRef('androidx-test')
             library('androidx-test-ext-junit', 'androidx.test.ext', 'junit').versionRef('androidx-test-ext-junit')
             library('androidx-test-ext-junit-ktx', 'androidx.test.ext', 'junit-ktx').versionRef('androidx-test-ext-junit')
+            library('androidx-test-monitor', 'androidx.test:monitor:1.6.1')
             library('androidx-test-orchestrator', 'androidx.test:orchestrator:1.4.1')
+            library('androidx-test-runner', 'androidx.test', 'runner').versionRef('androidx-test')
             library('espresso-core', 'androidx.test.espresso:espresso-core:3.4.0')
             library('mockito-core', 'org.mockito:mockito-inline:4.6.1')
             library('mockito-kotlin', 'org.mockito.kotlin:mockito-kotlin:4.0.0')

glide-webp/lib/build.gradle.kts

@@ -18,6 +18,8 @@ android {
         arguments("-DLIBWEBP_PATH=$rootDir/libwebp")
       }
     }
+
+    testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
   }
 
   externalNativeBuild {
@@ -33,4 +35,11 @@ dependencies {
 
   implementation(libs.glide.glide)
   kapt(libs.glide.compiler)
+
+  androidTestImplementation(testLibs.androidx.test.core)
+  androidTestImplementation(testLibs.androidx.test.core.ktx)
+  androidTestImplementation(testLibs.androidx.test.ext.junit)
+  androidTestImplementation(testLibs.androidx.test.ext.junit.ktx)
+  androidTestImplementation(testLibs.androidx.test.monitor)
+  androidTestImplementation(testLibs.androidx.test.runner)
 }

glide-webp/lib/src/androidTest/java/org/signal/glide/webp/WebpDecoderTest.kt

@@ -0,0 +1,102 @@
+package org.signal.glide.webp
+
+import android.os.Build
+import android.os.Debug
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.signal.core.util.StreamUtil
+
+@RunWith(AndroidJUnit4::class)
+class WebpDecoderTest {
+  private fun readAsset(name: String): ByteArray {
+    val source = InstrumentationRegistry.getInstrumentation().context.assets.open(name)
+    return StreamUtil.readFully(source)
+  }
+
+  @Test
+  fun landscapeUnscaled() {
+    val input = readAsset("landscape_550x368.webp")
+
+    val outputUnconstrained = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000)!!
+    assertEquals(550, outputUnconstrained.width)
+    assertEquals(368, outputUnconstrained.height)
+
+    val outputSquareConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 550, 550)!!
+    assertEquals(550, outputSquareConstraints.width)
+    assertEquals(368, outputSquareConstraints.height)
+
+    val outputExactConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 550, 368)!!
+    assertEquals(550, outputExactConstraints.width)
+    assertEquals(368, outputExactConstraints.height)
+  }
+
+  @Test
+  fun landscapeScaledBasedOnWidth() {
+    val input = readAsset("landscape_550x368.webp")
+
+    val outputUnconstrainedHeight = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 1000)!!
+    assertEquals(275, outputUnconstrainedHeight.width)
+    assertEquals(184, outputUnconstrainedHeight.height)
+
+    val outputSquareConstraints = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 275)!!
+    assertEquals(275, outputSquareConstraints.width)
+    assertEquals(184, outputSquareConstraints.height)
+
+    val outputSmallHeight = WebpDecoder().nativeDecodeBitmapScaled(input, 275, 200)!!
+    assertEquals(275, outputSmallHeight.width)
+    assertEquals(184, outputSmallHeight.height)
+  }
+
+  @Test
+  fun landscapeScaledBasedOnHeight() {
+    val input = readAsset("landscape_550x368.webp")
+
+    val outputUnconstrainedWidth = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 184)!!
+    assertEquals(275, outputUnconstrainedWidth.width)
+    assertEquals(184, outputUnconstrainedWidth.height)
+
+    val outputSmallWidth = WebpDecoder().nativeDecodeBitmapScaled(input, 300, 184)!!
+    assertEquals(275, outputSmallWidth.width)
+    assertEquals(184, outputSmallWidth.height)
+  }
+
+  private fun getNativeHeapSize(): Long {
+    return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+      Debug.getNativeHeapSize()
+    } else {
+      0
+    }
+  }
+
+  @Test
+  fun canScaleMassiveImagesWithoutAllocatingLargeBuffers() {
+    val nativeHeapSizeAtStart = getNativeHeapSize()
+
+    val input = readAsset("solid_white_10Kx10K.webp")
+
+    val output = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000)!!
+    assertEquals(1000, output.width)
+    assertEquals(1000, output.height)
+
+    var nativeHeapSizeAtEnd = getNativeHeapSize()
+    assertTrue("heap size: " + nativeHeapSizeAtStart + " -> " + nativeHeapSizeAtEnd, nativeHeapSizeAtStart + 10_000 * 10_000 > nativeHeapSizeAtEnd)
+  }
+
+  @Test
+  fun canHandleMassiveBrokenImagesWithoutAllocatingLargeBuffers() {
+    val nativeHeapSizeAtStart = getNativeHeapSize()
+
+    val input = readAsset("broken_10Kx10K.webp")
+
+    val output = WebpDecoder().nativeDecodeBitmapScaled(input, 1000, 1000)
+    assertNull(output)
+
+    var nativeHeapSizeAtEnd = getNativeHeapSize()
+    assertTrue("heap size: " + nativeHeapSizeAtStart + " -> " + nativeHeapSizeAtEnd, nativeHeapSizeAtStart + 10_000 * 10_000 > nativeHeapSizeAtEnd)
+  }
+}

glide-webp/lib/src/main/cpp/signalwebp.cpp

@@ -1,6 +1,8 @@
+#include <android/log.h>
 #include <jni.h>
 #include <webp/demux.h>
-#include <android/log.h>
+
+#include <algorithm>
 #include <string>
 
 #define TAG "WebpResourceDecoder"
@@ -16,7 +18,6 @@ jobject createBitmap(JNIEnv *env, int width, int height, const uint8_t *pixels)
 
     jobject argb8888Config = env->GetStaticObjectField(jbitmapConfigClass, jbitmapConfigARGB8888Field);
     jobject jbitmap        = env->CallStaticObjectMethod(jbitmapClass, jbitmapCreateBitmapMethod, intArray, 0, width, width, height, argb8888Config);
-    env->DeleteLocalRef(argb8888Config);
 
     return jbitmap;
 }
@@ -29,14 +30,14 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req
     WebPBitstreamFeatures features;
     if (WebPGetFeatures(buffer, bufferLength, &features) != VP8_STATUS_OK) {
         __android_log_write(ANDROID_LOG_WARN, TAG, "GetFeatures");
-        env->ReleaseByteArrayElements(data, javaBytes, 0);
+        env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT);
         return nullptr;
     }
 
     WebPDecoderConfig config;
     if (!WebPInitDecoderConfig(&config)) {
         __android_log_write(ANDROID_LOG_WARN, TAG, "Init decoder config");
-        env->ReleaseByteArrayElements(data, javaBytes, 0);
+        env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT);
         return nullptr;
     }
 
@@ -44,17 +45,13 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req
     config.output.colorspace           = MODE_BGRA;
 
     if (requestedWidth > 0 && requestedHeight > 0 && features.width > 0 && features.height > 0 && (requestedWidth < features.width || requestedHeight < features.height)) {
-        float hRatio = 1.0;
-        float vRatio = 1.0;
-        if (features.width >= features.height) {
-            vRatio = static_cast<float>(features.height) / static_cast<float>(features.width);
-        } else {
-            hRatio = static_cast<float>(features.width) / static_cast<float>(features.height);
-        }
+        double widthScale    = static_cast<double>(requestedWidth)  / features.width;
+        double heightScale   = static_cast<double>(requestedHeight) / features.height;
+        double requiredScale = std::min(widthScale, heightScale);
 
         config.options.use_scaling   = 1;
-        config.options.scaled_width  = static_cast<int>(static_cast<float>(requestedWidth) * hRatio);
-        config.options.scaled_height = static_cast<int>(static_cast<float>(requestedHeight) * vRatio);
+        config.options.scaled_width  = static_cast<int>(requiredScale * features.width);
+        config.options.scaled_height = static_cast<int>(requiredScale * features.height);
     }
 
     uint8_t *pixels = nullptr;
@@ -63,8 +60,7 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req
 
     VP8StatusCode result = WebPDecode(buffer, bufferLength, &config);
     if (result != VP8_STATUS_OK) {
-        __android_log_write(ANDROID_LOG_WARN, TAG, ("Scaled WebPDecode failed (" + std::to_string(result) + ") Trying without scaled").c_str());
-        pixels = WebPDecodeBGRA(buffer, bufferLength, &width, &height);
+        __android_log_write(ANDROID_LOG_WARN, TAG, ("Scaled WebPDecode failed (" + std::to_string(result) + ")").c_str());
     } else {
         pixels = config.output.u.RGBA.rgba;
         width  = config.output.width;
@@ -77,7 +73,7 @@ jobject nativeDecodeBitmapScaled(JNIEnv *env, jobject, jbyteArray data, jint req
     }
 
     WebPFree(pixels);
-    env->ReleaseByteArrayElements(data, javaBytes, 0);
+    env->ReleaseByteArrayElements(data, javaBytes, JNI_ABORT);
 
     return jbitmap;
 }