Use odrcore's own JNI bindings instead of the hand-written wrapper

CLAUDE.md

@@ -59,25 +59,39 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 **Native Dependencies:**
 - Uses Conan package manager for C++ dependencies
-- CMake build system for native C++ core library (`odr-core`)
-- NDK version 28.2.13676358 required
+- The app compiles no native code itself; conan builds odrcore (including its JNI
+  bindings) and the deployer drops the resulting `.so` files into `jniLibs`
+- NDK version 28.2.13676358 required (for conan's cross build, and for the
+  `libc++_shared.so` that gets shipped alongside `libodr_jni.so`)
 - C++20 standard
 - `conan` is taken from PATH; override with `-Podr.conanExecutable=...` or `ODR_CONAN`.
   Note the conan gradle plugin does not track `app/conanprofile.txt` as a task input, so
   after editing it `conanInstall-*` stays UP-TO-DATE and the native libs keep their old
   settings - run the conan install by hand to pick the change up locally.
 
 **Core Library Integration:**
-- Native C++ wrapper (`CoreWrapper.cpp`) provides JNI interface
+- The JNI interface comes from odrcore itself, both halves out of the one conan package
+  built with the recipe's `with_jni` option: java classes under `app.opendocument.core`
+  from `share/java/odr-core-java.jar`, and the matching `libodr_jni.so`. `CoreLoader` is
+  the only thing wrapping it.
+- Taking both from the same package is deliberate and should stay that way - handles
+  cross as raw longs and enums as ordinals with no version negotiation, so a separately
+  versioned java artifact could drift from the `.so`. It also keeps the build free of
+  credentials, which f-droid and other clean source builders need. `conandeployer.py`
+  puts the jar in `build/conan/<arch>/libs` and `app/build.gradle` depends on the armv8
+  copy as a file, not through a repository.
+- Anything the bindings use must exist on **android API 26**, which is far below what
+  their `--release 17` compiler accepts. That is a runtime-only failure, on device
+  (`java.lang.ref.Cleaner` and `List.of` both had to be fixed upstream for this reason)
 - Supports multiple architectures: armv8, armv7, x86, x86_64
-- Assets deployed to `assets/core` directory via custom Conan deployer
+- Assets deployed to `assets/core` and native libraries to `jniLibs/<abi>` via the custom
+  Conan deployer (`app/conandeployer.py`)
 
 ### Key Directories
 
 - `app/src/main/java/app/opendocument/droid/background/` - Document processing services
 - `app/src/main/java/app/opendocument/droid/ui/` - UI components and activities
 - `app/src/main/java/app/opendocument/droid/nonfree/` - Analytics, billing, and ads
-- `app/src/main/cpp/` - Native C++ JNI wrapper
 - `app/src/main/assets/` - HTML templates and fonts for document rendering
 
 ### Dependencies
@@ -109,10 +123,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 the `.pro` suffix) differ on purpose - do not "fix" the mismatch:
 
 - `namespace` is only the java/kotlin package plus `R`/`BuildConfig`, and is free to
-  rename. Renaming it means updating the JNI symbol names and `FindClass` strings in
-  `app/src/main/cpp/core_wrapper.{cpp,hpp}` and the `CoreWrapper` keeps in
-  `proguard-rules.txt` in lockstep - none of that is caught at compile time, and a stale
-  proguard keep only fails in minified builds.
+  rename. Nothing native is bound to it anymore - the JNI symbols live in odrcore's own
+  `app.opendocument.core` package, which the app does not rename - so the keeps in
+  `proguard-rules.txt` are about that package, not this one.
 - `applicationId` is the identity on Play and F-Droid and can never change: Play has no
   rename path, so a new one is a new listing that existing installs never update to.
   Store-facing renaming goes through the listing title in `fastlane/metadata/`.
@@ -130,10 +143,7 @@ the `.pro` suffix) differ on purpose - do not "fix" the mismatch:
 Mixed Java and Kotlin; Kotlin support is built into AGP 9, no kotlin plugin is applied.
 New code should be Kotlin. When converting an existing class, watch two things:
 
-- `CoreWrapper` and its nested `CoreOptions`/`CoreResult`/`GlobalParams` are read from C++
-  through `GetFieldID` in `app/src/main/cpp/core_wrapper.cpp` (see also Package names
-  above). Kotlin properties compile to
-  private fields with accessors, which breaks those lookups - every field needs `@JvmField`.
-  `proguard-rules.txt` keeps these classes for the same reason.
 - Java callers spell static helpers as `Foo.bar()`, so converted utility classes need
   `object` plus `@JvmStatic`, and public final fields need `@JvmField`.
+- Kotlin classes with default arguments are called from the remaining java loaders, which
+  cannot see defaults - either pass every argument or add `@JvmOverloads`.

README.md

@@ -25,6 +25,10 @@ Please help to translate on the https://crowdin.com/project/opendocument
   changing $PATH.
 - `git submodule update --init --depth 1 conan-odr-index`
 - `python conan-odr-index/scripts/conan_export_all_packages.py`
+- the java half of odrcore's JNI bindings (`odr-core-java.jar`) needs no setup: it
+  ships inside the odrcore conan package that also builds `libodr_jni.so`, and the
+  conan deployer puts it in `app/build/conan/armv8/libs`. No credentials are
+  involved anywhere in the build.
 
 ## Release signing
 

app/build.gradle

@@ -57,7 +57,8 @@ tasks.register('conanProfile', Copy) {
     tasks.named("conanInstall-" + architecture) {
         profile.set('build/conanprofile.txt')
         deployer.set('conandeployer.py')
-        deployerFolder.set(outputDirectory.get().asFile.toString() + "/assets/core")
+        // the deployer lays out both assets/core and jniLibs/<abi> underneath this
+        deployerFolder.set(outputDirectory.get().asFile.toString())
         dependsOn(tasks.named('conanProfile'))
         conanExecutable.set(conanExecutable_)
     }
@@ -78,17 +79,6 @@ android {
         // pinned here to keep it from drifting with the namespace.
         testApplicationId = "at.tomtasche.reader.test"
         testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
-
-        externalNativeBuild {
-            cmake {
-                targets = ["odr-core"]
-                arguments(
-                    "-DANDROID_STL=c++_shared",
-                    "-DCMAKE_TOOLCHAIN_FILE=build/conan/android_toolchain.cmake",
-                    "-DCMAKE_BUILD_TYPE=RelWithDebInfo",
-                )
-            }
-        }
     }
 
     flavorDimensions = ["default"]
@@ -152,13 +142,6 @@ android {
         targetCompatibility = JavaVersion.VERSION_17
     }
 
-
-    externalNativeBuild {
-        cmake {
-            version = "3.22.0+"
-            path = "CMakeLists.txt"
-        }
-    }
     lint {
         // lint errors are build failures. NewApi in particular is the check that
         // would have caught the java.nio.file usage below minSdk that shipped for
@@ -178,15 +161,6 @@ android {
         // AGP 9 makes resValue opt-in; the flavors use it for DISABLE_TRACKING
         resValues = true
     }
-    packaging {
-        jniLibs {
-            // No need to pickFirst libc++_shared.so if all files are identical.
-            // They will be identical if NDK major version matches.
-            // NDK runtime problems may occur if NDK version mismatches,
-            // so comment this out, to get a compile error instead
-            // pickFirsts += ['**/libc++_shared.so']
-        }
-    }
     // the java/kotlin package, R and BuildConfig. deliberately different from
     // applicationId above: this one is internal and free to rename, that one is not.
     // testNamespace is left to default to namespace + ".test".
@@ -197,9 +171,25 @@ android {
 
     // TODO can and should this be architecture dependent?
     sourceSets.main.assets.srcDirs += "build/conan/armv8/assets"
+
+    // libodr_jni.so (odrcore built with with_jni) plus the libc++_shared.so it links,
+    // both put there per architecture by conandeployer.py. these used to be produced by
+    // the app's own externalNativeBuild, which is gone now that the JNI layer comes
+    // from the core itself
+    sourceSets.main.jniLibs.srcDirs +=
+            ["armv8", "armv7", "x86", "x86_64"].collect { "build/conan/${it}/jniLibs" }
 }
 
 dependencies {
+    // the java side of odrcore's JNI bindings, taken from the same conan package that
+    // built the libodr_jni.so deployed into jniLibs above. the two are one ABI with no
+    // version negotiation, so sourcing both from one package is what keeps them in
+    // step. the jar is architecture independent - armv8 just happens to be the copy we
+    // read, like the assets source set above.
+    implementation files(layout.buildDirectory.file("conan/armv8/libs/odr-core-java.jar")) {
+        builtBy("conanInstall-armv8")
+    }
+
     implementation libs.play.services.ads
     implementation libs.play.review
     implementation libs.user.messaging.platform
@@ -223,12 +213,3 @@ dependencies {
     debugImplementation libs.espresso.idling.resource
     implementation libs.androidx.annotation
 }
-
-// Without removing .cxx dir on cleanup, double gradle clean is erroring out.
-// Before removing this workaround, check if "./gradlew assembleDebug; ./gradlew clean; ./gradlew clean" works
-tasks.named("clean") {
-    def dotCxxDir = layout.projectDirectory.dir(".cxx")
-    doFirst {
-        delete dotCxxDir
-    }
-}

app/conandeployer.py

@@ -1,5 +1,18 @@
+import glob
+import os
 import shutil
 
+from conan.errors import ConanException
+
+# conan architecture -> android abi directory name expected under jniLibs, and the
+# ndk toolchain triple the matching libc++_shared.so lives under
+ANDROID_ABIS = {
+    "armv8": ("arm64-v8a", "aarch64-linux-android"),
+    "armv7": ("armeabi-v7a", "arm-linux-androideabi"),
+    "x86": ("x86", "i686-linux-android"),
+    "x86_64": ("x86_64", "x86_64-linux-android"),
+}
+
 
 def deploy(graph, output_folder: str, **kwargs):
     conanfile = graph.root.conanfile
@@ -17,20 +30,68 @@ def deploy(graph, output_folder: str, **kwargs):
 
     copytree_kwargs = {"symlinks": symlinks, "dirs_exist_ok": True}
 
+    # assets/ and jniLibs/ are separate android source sets, so the deployer writes the
+    # whole build/conan/<arch> tree rather than a single directory
+    assets_folder = f"{output_folder}/assets/core"
+
     if "odrcore" in deps:
         dep = deps["odrcore"]
-        conanfile.output.info(f"Deploying odrcore to {output_folder}")
+        conanfile.output.info(f"Deploying odrcore assets to {assets_folder}")
         shutil.copytree(
             f"{dep.package_folder}/share",
-            f"{output_folder}/odrcore",
+            f"{assets_folder}/odrcore",
+            # share/java holds odr-core-java.jar, which is deployed as a library
+            # below - shipping it as an asset too would only bloat the apk
+            ignore=shutil.ignore_patterns("java"),
             **copytree_kwargs,
         )
 
+        # the java half of the JNI bindings. taking it from the very package that
+        # built libodr_jni.so is what keeps the two halves in lockstep: they are one
+        # ABI with no version negotiation, and a maven artifact resolved by version
+        # could drift from the .so. it also keeps the build credential-free, which
+        # f-droid and other clean source builders need
+        libs_folder = f"{output_folder}/libs"
+        os.makedirs(libs_folder, exist_ok=True)
+        conanfile.output.info(f"Deploying odr-core-java.jar to {libs_folder}")
+        shutil.copy2(
+            f"{dep.package_folder}/share/java/odr-core-java.jar",
+            f"{libs_folder}/odr-core-java.jar",
+        )
+
     if "libmagic" in deps:
         dep = deps["libmagic"]
-        conanfile.output.info(f"Deploying libmagic to {output_folder}")
+        conanfile.output.info(f"Deploying libmagic assets to {assets_folder}")
         shutil.copytree(
             f"{dep.package_folder}/res",
-            f"{output_folder}/libmagic",
+            f"{assets_folder}/libmagic",
             **copytree_kwargs,
         )
+
+    if arch not in ANDROID_ABIS:
+        raise ConanException(f"No android abi known for arch {arch}")
+    abi, triple = ANDROID_ABIS[arch]
+    jni_libs_folder = f"{output_folder}/jniLibs/{abi}"
+    os.makedirs(jni_libs_folder, exist_ok=True)
+
+    if "odrcore" in deps:
+        dep = deps["odrcore"]
+        # built by the recipe's with_jni option, alongside the odr-core-java.jar
+        # deployed above - both halves come out of this one package
+        source = f"{dep.package_folder}/lib/libodr_jni.so"
+        conanfile.output.info(f"Deploying libodr_jni.so to {jni_libs_folder}")
+        shutil.copy2(source, f"{jni_libs_folder}/libodr_jni.so")
+
+    # libodr_jni.so links the shared c++ runtime (compiler.libcxx=c++_shared in
+    # conanprofile.txt). the app no longer builds any native code of its own, so
+    # nothing else would pull libc++_shared.so into the apk and the library would
+    # fail to load at runtime with "library libc++_shared.so not found"
+    ndk_path = conanfile.conf.get("tools.android:ndk_path")
+    if not ndk_path:
+        raise ConanException("tools.android:ndk_path is not set, cannot find libc++_shared.so")
+    pattern = f"{ndk_path}/toolchains/llvm/prebuilt/*/sysroot/usr/lib/{triple}/libc++_shared.so"
+    matches = glob.glob(pattern)
+    if not matches:
+        raise ConanException(f"No libc++_shared.so found at {pattern}")
+    conanfile.output.info(f"Deploying libc++_shared.so to {jni_libs_folder}")
+    shutil.copy2(matches[0], f"{jni_libs_folder}/libc++_shared.so")

app/conanfile.txt

@@ -1,5 +1,5 @@
 [requires]
-odrcore/5.7.0
+odrcore/5.7.1
 
 [generators]
 CMakeToolchain
@@ -12,4 +12,5 @@ odrcore/*:with_wvWare=False
 odrcore/*:with_libmagic=True
 odrcore/*:with_http_server=True
 odrcore/*:with_cli=False
+odrcore/*:with_jni=True
 odrcore/*:bundle_assets=False

app/proguard-rules.txt

@@ -22,12 +22,12 @@
 
 -keepnames class at.stefl.** { *; }
 
-# the c++ side looks these up by name through FindClass/GetFieldID in
-# src/main/cpp/core_wrapper.cpp, so r8 must not rename or strip them. keep these in sync
-# with the java package - a stale rule here only breaks minified builds, where the fields
-# get stripped and GetFieldID then returns null at runtime.
--keep class app.opendocument.droid.background.CoreWrapper { *; }
--keep class app.opendocument.droid.background.CoreWrapper$* { *; }
+# libodr_jni.so resolves these by name: FindClass for HtmlConfig, HtmlPage, HtmlResource
+# and the Html result holders, GetFieldID for every HtmlConfig field, and the native
+# methods are bound by signature. it also constructs the OdrException subclasses when
+# translating C++ exceptions. r8 must not rename or strip any of it - the failure only
+# shows up in minified builds, at runtime, as a null field id or a NoSuchMethodError.
+-keep class app.opendocument.core.** { *; }
 
 -keepattributes *Annotation*
 -keepattributes SourceFile,LineNumberTable