Restore checkSystemServerOrder

Move checkSystemServerOrder into the java package so it can filter out
bootclasspathDependencyTag dependencies in order to prevent traversing
into bootclasspath dependencies and confusing them with system server
classpath dependencies.

Bug: 397461231
Test: TestCheckSystemServerOrderWithArtApex
Change-Id: I64630eb2edfcffe796cc5eb47c08ed167a6ff0dd

3 files changed, 149 insertions, 27 deletions

diff --git a/apex/systemserver_classpath_fragment_test.go b/apex/systemserver_classpath_fragment_test.go
index cf7ea8af9..61f79d695 100644
--- a/apex/systemserver_classpath_fragment_test.go
+++ b/apex/systemserver_classpath_fragment_test.go
@@ -471,3 +471,120 @@ func assertProfileGuidedPrebuilt(t *testing.T, ctx *android.TestContext, apexNam
 		t.Fatalf("Expected profile-guided to be %v, got %v", expected, actual)
 	}
 }
+
+func TestCheckSystemServerOrderWithArtApex(t *testing.T) {
+	preparers := android.GroupFixturePreparers(
+		java.PrepareForTestWithDexpreopt,
+		java.PrepareForTestWithJavaSdkLibraryFiles,
+		PrepareForTestWithApexBuildComponents,
+		prepareForTestWithArtApex,
+		java.FixtureConfigureBootJars("com.android.art:framework-art"),
+		dexpreopt.FixtureSetApexSystemServerJars("com.android.apex1:service-apex1", "com.android.art:service-art"),
+		java.FixtureWithLastReleaseApis("baz"),
+	)
+
+	// Creates a com.android.art apex with a bootclasspath fragment and a systemserverclasspath fragment, and a
+	// com.android.apex1 prebuilt whose bootclasspath fragment depends on the com.android.art bootclasspath fragment.
+	// Verifies that the checkSystemServerOrder doesn't get confused by the bootclasspath dependencies and report
+	// that service-apex1 depends on service-art.
+	result := preparers.RunTestWithBp(t, `
+		apex {
+			name: "com.android.art",
+			key: "com.android.art.key",
+			bootclasspath_fragments: ["art-bootclasspath-fragment"],
+			systemserverclasspath_fragments: ["art-systemserverclasspath-fragment"],
+			updatable: false,
+		}
+
+		apex_key {
+			name: "com.android.art.key",
+			public_key: "com.android.art.avbpubkey",
+			private_key: "com.android.art.pem",
+		}
+
+		bootclasspath_fragment {
+			name: "art-bootclasspath-fragment",
+			image_name: "art",
+			contents: ["framework-art"],
+			apex_available: [
+				"com.android.art",
+			],
+			hidden_api: {
+				split_packages: ["*"],
+			},
+		}
+
+		java_library {
+			name: "framework-art",
+			apex_available: ["com.android.art"],
+			srcs: ["a.java"],
+			compile_dex: true,
+		}
+
+		systemserverclasspath_fragment {
+			name: "art-systemserverclasspath-fragment",
+			apex_available: ["com.android.art"],
+			contents: ["service-art"],
+		}
+
+		java_library {
+			name: "service-art",
+			srcs: ["a.java"],
+			apex_available: ["com.android.art"],
+			compile_dex: true,
+		}
+
+		prebuilt_apex {
+			name: "com.android.apex1",
+			arch: {
+				arm64: {
+					src: "myapex-arm64.apex",
+				},
+				arm: {
+					src: "myapex-arm.apex",
+				},
+			},
+			exported_bootclasspath_fragments: ["com.android.apex1-bootclasspath-fragment"],
+			exported_systemserverclasspath_fragments: ["com.android.apex1-systemserverclasspath-fragment"],
+		}
+
+		prebuilt_bootclasspath_fragment {
+			name: "com.android.apex1-bootclasspath-fragment",
+			visibility: ["//visibility:public"],
+			apex_available: ["com.android.apex1"],
+			contents: ["framework-apex1"],
+			fragments: [
+				{
+					apex: "com.android.art",
+					module: "art-bootclasspath-fragment",
+				},
+			],
+			hidden_api: {
+				annotation_flags: "hiddenapi/annotation-flags.csv",
+				metadata: "hiddenapi/metadata.csv",
+				index: "hiddenapi/index.csv",
+				stub_flags: "hiddenapi/stub-flags.csv",
+				all_flags: "hiddenapi/all-flags.csv",
+			},
+		}
+
+		java_import {
+			name: "framework-apex1",
+			apex_available: ["com.android.apex1"],
+		}
+
+		prebuilt_systemserverclasspath_fragment {
+			name: "com.android.apex1-systemserverclasspath-fragment",
+			apex_available: ["com.android.apex1"],
+			contents: ["service-apex1"],
+		}
+
+		java_import {
+			name: "service-apex1",
+			installable: true,
+			apex_available: ["com.android.apex1"],
+			sdk_version: "current",
+		}`)
+
+	_ = result
+}
diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go
index 1452b412e..699a6757d 100644
--- a/dexpreopt/dexpreopt.go
+++ b/dexpreopt/dexpreopt.go
@@ -283,10 +283,7 @@ func dexpreoptCommand(ctx android.BuilderContext, globalSoong *GlobalSoongConfig
 		clcHostString := "PCL[" + strings.Join(clcHost.Strings(), ":") + "]"
 		clcTargetString := "PCL[" + strings.Join(clcTarget, ":") + "]"
 
-		if systemServerClasspathJars.ContainsJar(module.Name) {
-			// TODO(b/397461231): renable this check
-			//checkSystemServerOrder(ctx, jarIndex)
-		} else {
+		if !systemServerClasspathJars.ContainsJar(module.Name) {
 			// Standalone jars are loaded by separate class loaders with SYSTEMSERVERCLASSPATH as the
 			// parent.
 			clcHostString = "PCL[];" + clcHostString
@@ -578,29 +575,6 @@ func SystemServerDexJarHostPath(ctx android.PathContext, jar string) android.Out
 	}
 }
 
-// Check the order of jars on the system server classpath and give a warning/error if a jar precedes
-// one of its dependencies. This is not an error, but a missed optimization, as dexpreopt won't
-// have the dependency jar in the class loader context, and it won't be able to resolve any
-// references to its classes and methods.
-func checkSystemServerOrder(ctx android.PathContext, jarIndex int) {
-	mctx, isModule := ctx.(android.ModuleContext)
-	if isModule {
-		config := GetGlobalConfig(ctx)
-		jars := config.AllSystemServerClasspathJars(ctx)
-		mctx.WalkDeps(func(dep android.Module, parent android.Module) bool {
-			depIndex := jars.IndexOfJar(dep.Name())
-			if jarIndex < depIndex && !config.BrokenSuboptimalOrderOfSystemServerJars {
-				jar := jars.Jar(jarIndex)
-				dep := jars.Jar(depIndex)
-				mctx.ModuleErrorf("non-optimal order of jars on the system server classpath:"+
-					" '%s' precedes its dependency '%s', so dexpreopt is unable to resolve any"+
-					" references from '%s' to '%s'.\n", jar, dep, jar, dep)
-			}
-			return true
-		})
-	}
-}
-
 // Returns path to a file containing the reult of verify_uses_libraries check (empty if the check
 // has succeeded, or an error message if it failed).
 func UsesLibrariesStatusFile(ctx android.ModuleContext) android.WritablePath {
diff --git a/java/dexpreopt.go b/java/dexpreopt.go
index 5755dec23..b21cfc968 100644
--- a/java/dexpreopt.go
+++ b/java/dexpreopt.go
@@ -565,6 +565,10 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, libName string, dexJa
 	if !isApexSystemServerJar {
 		d.builtInstalled = dexpreoptRule.Installs().String()
 	}
+
+	if isSystemServerJar {
+		checkSystemServerOrder(ctx, libName)
+	}
 }
 
 func getModuleInstallPathInfo(ctx android.ModuleContext, fullInstallPath string) (android.InstallPath, string, string) {
@@ -631,3 +635,30 @@ func (d *dexpreopter) GetRewrittenProfile() android.Path {
 func (d *dexpreopter) SetRewrittenProfile(p android.Path) {
 	d.rewrittenProfile = p
 }
+
+// Check the order of jars on the system server classpath and give a warning/error if a jar precedes
+// one of its dependencies. This is not an error, but a missed optimization, as dexpreopt won't
+// have the dependency jar in the class loader context, and it won't be able to resolve any
+// references to its classes and methods.
+func checkSystemServerOrder(ctx android.ModuleContext, libName string) {
+	config := dexpreopt.GetGlobalConfig(ctx)
+	jars := config.AllSystemServerClasspathJars(ctx)
+	jarIndex := config.AllSystemServerJars(ctx).IndexOfJar(libName)
+	ctx.WalkDeps(func(dep android.Module, parent android.Module) bool {
+		tag := ctx.OtherModuleDependencyTag(dep)
+		// Ideally this should only be walking relevant dependencies, but to maintain existing behavior
+		// for now just exclude any known irrelevant dependencies that would lead to incorrect errors.
+		if _, ok := tag.(bootclasspathDependencyTag); ok {
+			return false
+		}
+		depIndex := jars.IndexOfJar(dep.Name())
+		if jarIndex < depIndex && !config.BrokenSuboptimalOrderOfSystemServerJars {
+			jar := jars.Jar(jarIndex)
+			dep := jars.Jar(depIndex)
+			ctx.ModuleErrorf("non-optimal order of jars on the system server classpath:"+
+				" '%s' precedes its dependency '%s', so dexpreopt is unable to resolve any"+
+				" references from '%s' to '%s'.\n", jar, dep, jar, dep)
+		}
+		return true
+	})
+}