Always embed jni libs and store uncompressed

Previously, unlike unbundled apps, if a platform app depends on a native
library, it is not embedded in the app, but installed to <partition>/lib
and the app is instead provided with a symlink to the lib. This actualy
is a legacy from the original Android where native libraries couldn't be
executed while embedded in the apk. To be executed, the native libs had
to be extracted to a mutable storage, but that had a risk of breaking
the verified boot, so libs couldn't be embedded.

Since API level 23, execute-in-place of native libs has become
possible. But platform apps had to opt-in (use_embedded_native_libs:
true) for the feature for a reason that is unclear today. Perhaps, it
was to save disk space in case when multiple apps share the same native
library, but such cases are found to be very rare, or non-existing.

With this CL, two changes are introduced:

1. jni libs are always embededd for all apps (bundled or unbundled)

2. use_embedded_native_libs is deprecated unless the module type is
   android_test or android_test_helper_app. It is now a
   no-op property. It's left just to not break existing bp files.

This will make unbundled apps bigger, but given that unbundled apps
built using android platform build system are tests, the size increase
should be acceptible.

Bug: 330276359
Test: m
Change-Id: I7df993ea85bce1c0a7222000c403a974818c3362

6 files changed, 36 insertions, 195 deletions

diff --git a/java/androidmk.go b/java/androidmk.go
index a52d43965..ed9d5de32 100644
--- a/java/androidmk.go
+++ b/java/androidmk.go
@@ -17,7 +17,6 @@ package java
 import (
 	"fmt"
 	"io"
-	"strings"
 
 	"android/soong/android"
 
@@ -419,23 +418,6 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries {
 				if app.embeddedJniLibs {
 					jniSymbols := app.JNISymbolsInstalls(app.installPathForJNISymbols.String())
 					entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", jniSymbols.String())
-				} else {
-					for _, jniLib := range app.jniLibs {
-						entries.AddStrings("LOCAL_SOONG_JNI_LIBS_"+jniLib.target.Arch.ArchType.String(), jniLib.name)
-						var partitionTag string
-
-						// Mimic the creation of partition_tag in build/make,
-						// which defaults to an empty string when the partition is system.
-						// Otherwise, capitalize with a leading _
-						if jniLib.partition == "system" {
-							partitionTag = ""
-						} else {
-							split := strings.Split(jniLib.partition, "/")
-							partitionTag = "_" + strings.ToUpper(split[len(split)-1])
-						}
-						entries.AddStrings("LOCAL_SOONG_JNI_LIBS_PARTITION_"+jniLib.target.Arch.ArchType.String(),
-							jniLib.name+":"+partitionTag)
-					}
 				}
 
 				if len(app.jniCoverageOutputs) > 0 {
diff --git a/java/androidmk_test.go b/java/androidmk_test.go
index 1232cd1ee..197da4f38 100644
--- a/java/androidmk_test.go
+++ b/java/androidmk_test.go
@@ -19,9 +19,6 @@ import (
 	"testing"
 
 	"android/soong/android"
-	"android/soong/cc"
-
-	"github.com/google/blueprint/proptools"
 )
 
 func TestRequired(t *testing.T) {
@@ -255,149 +252,3 @@ func TestGetOverriddenPackages(t *testing.T) {
 		android.AssertDeepEquals(t, "overrides property", expected.overrides, actual)
 	}
 }
-
-func TestJniPartition(t *testing.T) {
-	bp := `
-		cc_library {
-			name: "libjni_system",
-			system_shared_libs: [],
-			sdk_version: "current",
-			stl: "none",
-		}
-
-		cc_library {
-			name: "libjni_system_ext",
-			system_shared_libs: [],
-			sdk_version: "current",
-			stl: "none",
-			system_ext_specific: true,
-		}
-
-		cc_library {
-			name: "libjni_odm",
-			system_shared_libs: [],
-			sdk_version: "current",
-			stl: "none",
-			device_specific: true,
-		}
-
-		cc_library {
-			name: "libjni_product",
-			system_shared_libs: [],
-			sdk_version: "current",
-			stl: "none",
-			product_specific: true,
-		}
-
-		cc_library {
-			name: "libjni_vendor",
-			system_shared_libs: [],
-			sdk_version: "current",
-			stl: "none",
-			soc_specific: true,
-		}
-
-		android_app {
-			name: "test_app_system_jni_system",
-			privileged: true,
-			platform_apis: true,
-			certificate: "platform",
-			jni_libs: ["libjni_system"],
-		}
-
-		android_app {
-			name: "test_app_system_jni_system_ext",
-			privileged: true,
-			platform_apis: true,
-			certificate: "platform",
-			jni_libs: ["libjni_system_ext"],
-		}
-
-		android_app {
-			name: "test_app_system_ext_jni_system",
-			privileged: true,
-			platform_apis: true,
-			certificate: "platform",
-			jni_libs: ["libjni_system"],
-			system_ext_specific: true
-		}
-
-		android_app {
-			name: "test_app_system_ext_jni_system_ext",
-			sdk_version: "core_platform",
-			jni_libs: ["libjni_system_ext"],
-			system_ext_specific: true
-		}
-
-		android_app {
-			name: "test_app_product_jni_product",
-			sdk_version: "core_platform",
-			jni_libs: ["libjni_product"],
-			product_specific: true
-		}
-
-		android_app {
-			name: "test_app_vendor_jni_odm",
-			sdk_version: "core_platform",
-			jni_libs: ["libjni_odm"],
-			soc_specific: true
-		}
-
-		android_app {
-			name: "test_app_odm_jni_vendor",
-			sdk_version: "core_platform",
-			jni_libs: ["libjni_vendor"],
-			device_specific: true
-		}
-		android_app {
-			name: "test_app_system_jni_multiple",
-			privileged: true,
-			platform_apis: true,
-			certificate: "platform",
-			jni_libs: ["libjni_system", "libjni_system_ext"],
-		}
-		android_app {
-			name: "test_app_vendor_jni_multiple",
-			sdk_version: "core_platform",
-			jni_libs: ["libjni_odm", "libjni_vendor"],
-			soc_specific: true
-		}
-		`
-	arch := "arm64"
-	ctx := android.GroupFixturePreparers(
-		PrepareForTestWithJavaDefaultModules,
-		cc.PrepareForTestWithCcDefaultModules,
-		android.PrepareForTestWithAndroidMk,
-		android.FixtureModifyConfig(func(config android.Config) {
-			config.TestProductVariables.DeviceArch = proptools.StringPtr(arch)
-		}),
-	).
-		RunTestWithBp(t, bp)
-	testCases := []struct {
-		name           string
-		partitionNames []string
-		partitionTags  []string
-	}{
-		{"test_app_system_jni_system", []string{"libjni_system"}, []string{""}},
-		{"test_app_system_jni_system_ext", []string{"libjni_system_ext"}, []string{"_SYSTEM_EXT"}},
-		{"test_app_system_ext_jni_system", []string{"libjni_system"}, []string{""}},
-		{"test_app_system_ext_jni_system_ext", []string{"libjni_system_ext"}, []string{"_SYSTEM_EXT"}},
-		{"test_app_product_jni_product", []string{"libjni_product"}, []string{"_PRODUCT"}},
-		{"test_app_vendor_jni_odm", []string{"libjni_odm"}, []string{"_ODM"}},
-		{"test_app_odm_jni_vendor", []string{"libjni_vendor"}, []string{"_VENDOR"}},
-		{"test_app_system_jni_multiple", []string{"libjni_system", "libjni_system_ext"}, []string{"", "_SYSTEM_EXT"}},
-		{"test_app_vendor_jni_multiple", []string{"libjni_odm", "libjni_vendor"}, []string{"_ODM", "_VENDOR"}},
-	}
-
-	for _, test := range testCases {
-		t.Run(test.name, func(t *testing.T) {
-			mod := ctx.ModuleForTests(test.name, "android_common").Module()
-			entry := android.AndroidMkEntriesForTest(t, ctx.TestContext, mod)[0]
-			for i := range test.partitionNames {
-				actual := entry.EntryMap["LOCAL_SOONG_JNI_LIBS_PARTITION_"+arch][i]
-				expected := test.partitionNames[i] + ":" + test.partitionTags[i]
-				android.AssertStringEquals(t, "Expected and actual differ", expected, actual)
-			}
-		})
-	}
-}
diff --git a/java/app.go b/java/app.go
index 50d1a2f43..0170ea186 100644
--- a/java/app.go
+++ b/java/app.go
@@ -90,20 +90,17 @@ type appProperties struct {
 	Stl *string `android:"arch_variant"`
 
 	// Store native libraries uncompressed in the APK and set the android:extractNativeLibs="false" manifest
-	// flag so that they are used from inside the APK at runtime.  Defaults to true for android_test modules unless
-	// sdk_version or min_sdk_version is set to a version that doesn't support it (<23), defaults to true for
-	// android_app modules that are embedded to APEXes, defaults to false for other module types where the native
-	// libraries are generally preinstalled outside the APK.
+	// flag so that they are used from inside the APK at runtime. This property is respected only for
+	// APKs built using android_test or android_test_helper_app. For other APKs, this property is ignored
+	// and native libraries are always embedded compressed.
 	Use_embedded_native_libs *bool
 
 	// Store dex files uncompressed in the APK and set the android:useEmbeddedDex="true" manifest attribute so that
 	// they are used from inside the APK at runtime.
 	Use_embedded_dex *bool
 
-	// Forces native libraries to always be packaged into the APK,
-	// Use_embedded_native_libs still selects whether they are stored uncompressed and aligned or compressed.
-	// True for android_test* modules.
-	AlwaysPackageNativeLibs bool `blueprint:"mutated"`
+	// Allows compressing of embedded native libs. Only for android_test and android_test_helper_app.
+	AllowCompressingNativeLibs bool `blueprint:"mutated"`
 
 	// If set, find and merge all NOTICE files that this module and its dependencies have and store
 	// it in the APK as an asset.
@@ -403,14 +400,20 @@ func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVer
 // Returns true if the native libraries should be stored in the APK uncompressed and the
 // extractNativeLibs application flag should be set to false in the manifest.
 func (a *AndroidApp) useEmbeddedNativeLibs(ctx android.ModuleContext) bool {
+	var useEmbedded bool
+	if a.appProperties.AllowCompressingNativeLibs {
+		useEmbedded = BoolDefault(a.appProperties.Use_embedded_native_libs, true)
+	} else {
+		useEmbedded = true // always uncompress for non-test apps
+	}
+
 	minSdkVersion, err := a.MinSdkVersion(ctx).EffectiveVersion(ctx)
 	if err != nil {
 		ctx.PropertyErrorf("min_sdk_version", "invalid value %q: %s", a.MinSdkVersion(ctx), err)
 	}
+	supported := minSdkVersion.FinalOrFutureInt() >= 23
 
-	apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider)
-	return (minSdkVersion.FinalOrFutureInt() >= 23 && Bool(a.appProperties.Use_embedded_native_libs)) ||
-		!apexInfo.IsForPlatform()
+	return useEmbedded && supported
 }
 
 // Returns whether this module should have the dex file stored uncompressed in the APK.
@@ -433,9 +436,8 @@ func (a *AndroidApp) shouldUncompressDex(ctx android.ModuleContext) bool {
 }
 
 func (a *AndroidApp) shouldEmbedJnis(ctx android.BaseModuleContext) bool {
-	apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider)
-	return ctx.Config().UnbundledBuild() || Bool(a.appProperties.Use_embedded_native_libs) ||
-		!apexInfo.IsForPlatform() || a.appProperties.AlwaysPackageNativeLibs
+	// Always!
+	return true
 }
 
 func generateAaptRenamePackageFlags(packageName string, renameResourcesPackage bool) []string {
@@ -1400,8 +1402,7 @@ func AndroidTestFactory() android.Module {
 	module.Module.properties.Instrument = true
 	module.Module.properties.Supports_static_instrumentation = true
 	module.Module.properties.Installable = proptools.BoolPtr(true)
-	module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true)
-	module.appProperties.AlwaysPackageNativeLibs = true
+	module.appProperties.AllowCompressingNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
 
@@ -1456,8 +1457,7 @@ func AndroidTestHelperAppFactory() android.Module {
 	module.Module.dexProperties.Optimize.EnabledByDefault = true
 
 	module.Module.properties.Installable = proptools.BoolPtr(true)
-	module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true)
-	module.appProperties.AlwaysPackageNativeLibs = true
+	module.appProperties.AllowCompressingNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
 
diff --git a/java/app_test.go b/java/app_test.go
index eab40e7da..938b88aba 100644
--- a/java/app_test.go
+++ b/java/app_test.go
@@ -2013,8 +2013,8 @@ func TestJNIPackaging(t *testing.T) {
 		packaged   bool
 		compressed bool
 	}{
-		{"app", false, false},
-		{"app_noembed", false, false},
+		{"app", true, false},
+		{"app_noembed", true, false},
 		{"app_embed", true, false},
 		{"test", true, false},
 		{"test_noembed", true, true},
@@ -3319,8 +3319,7 @@ func TestUsesLibraries(t *testing.T) {
 	// These also include explicit `uses_libs`/`optional_uses_libs` entries, as they may be
 	// propagated from dependencies.
 	actualManifestFixerArgs := app.Output("manifest_fixer/AndroidManifest.xml").Args["args"]
-	expectManifestFixerArgs := `--extract-native-libs=true ` +
-		`--uses-library foo ` +
+	expectManifestFixerArgs := `--uses-library foo ` +
 		`--uses-library com.non.sdk.lib ` +
 		`--uses-library qux ` +
 		`--uses-library quuz ` +
@@ -4110,7 +4109,7 @@ func TestAppIncludesJniPackages(t *testing.T) {
 		},
 		{
 			name:       "aary-no-use-embedded",
-			hasPackage: false,
+			hasPackage: true,
 		},
 	}
 
diff --git a/scripts/manifest_fixer.py b/scripts/manifest_fixer.py
index 58079aa5d..35d2a1c81 100755
--- a/scripts/manifest_fixer.py
+++ b/scripts/manifest_fixer.py
@@ -62,8 +62,8 @@ def parse_args():
                             'in the manifest.'))
   parser.add_argument('--extract-native-libs', dest='extract_native_libs',
                       default=None, type=lambda x: (str(x).lower() == 'true'),
-                      help=('specify if the app wants to use embedded native libraries. Must not conflict '
-                            'if already declared in the manifest.'))
+                      help=('specify if the app wants to use embedded native libraries. Must not '
+                            'be true if manifest says false.'))
   parser.add_argument('--has-no-code', dest='has_no_code', action='store_true',
                       help=('adds hasCode="false" attribute to application. Ignored if application elem '
                             'already has a hasCode attribute.'))
@@ -299,7 +299,16 @@ def add_extract_native_libs(doc, extract_native_libs):
     attr = doc.createAttributeNS(android_ns, 'android:extractNativeLibs')
     attr.value = value
     application.setAttributeNode(attr)
-  elif attr.value != value:
+  elif attr.value == "false" and value == "true":
+    # Note that we don't disallow the case of extractNativeLibs="true" in manifest and
+    # --extract-native-libs="false". This is fine because --extract-native-libs="false" means that
+    # the build system didn't compress the JNI libs, which is a fine choice for built-in apps. At
+    # runtime the JNI libs will be extracted to outside of the APK, but everything will still work
+    # okay.
+    #
+    # The opposite (extractNativeLibs="false" && --extract-native-libs="true") should however be
+    # disallowed because otherwise that would make an ill-formed APK; JNI libs are stored compressed
+    # but they won't be extracted. There's no way to execute the JNI libs.
     raise RuntimeError('existing attribute extractNativeLibs="%s" conflicts with --extract-native-libs="%s"' %
                        (attr.value, value))
 
diff --git a/scripts/manifest_fixer_test.py b/scripts/manifest_fixer_test.py
index 0a62b10a4..9fce6b9b8 100755
--- a/scripts/manifest_fixer_test.py
+++ b/scripts/manifest_fixer_test.py
@@ -479,8 +479,8 @@ class AddExtractNativeLibsTest(unittest.TestCase):
     self.assert_xml_equal(output, expected)
 
   def test_conflict(self):
-    manifest_input = self.manifest_tmpl % self.extract_native_libs('true')
-    self.assertRaises(RuntimeError, self.run_test, manifest_input, False)
+    manifest_input = self.manifest_tmpl % self.extract_native_libs('false')
+    self.assertRaises(RuntimeError, self.run_test, manifest_input, True)
 
 
 class AddNoCodeApplicationTest(unittest.TestCase):