Normalize Java SDK (JDK) location logic across the tool (#124233)

Normalize Java SDK (JDK) location logic across the tool 

packages/flutter_tools/lib/src/android/android_sdk.dart

@@ -462,7 +462,17 @@ class AndroidSdk {
     return versionString.split('_').first;
   }
 
+  /// Finds the java binary that is used for all operations across the tool.
+  ///
   /// First try Java bundled with Android Studio, then sniff JAVA_HOME, then fallback to PATH.
+  ///
+  // TODO(andrewkolos): To prevent confusion when debugging Android-related
+  // issues (see https://github.com/flutter/flutter/issues/122609 for an example),
+  // this logic should be consistently followed by any Java-dependent operation
+  // across the  the tool (building Android apps, interacting with the Android SDK, etc.).
+  // Currently, this consistency is fragile since the logic used for building
+  // Android apps exists independently of this method.
+  // See https://github.com/flutter/flutter/issues/124252.
   static String? findJavaBinary({
     required AndroidStudio? androidStudio,
     required FileSystem fileSystem,
@@ -482,30 +492,6 @@ class AndroidSdk {
       return fileSystem.path.join(javaHomeEnv, 'bin', 'java');
     }
 
-    // MacOS specific logic to avoid popping up a dialog window.
-    // See: http://stackoverflow.com/questions/14292698/how-do-i-check-if-the-java-jdk-is-installed-on-mac.
-    if (platform.isMacOS) {
-      try {
-        // -v Filter versions (as if JAVA_VERSION had been set in the environment).
-        // It is unlikley that filtering to java version 1.8 is the right
-        // decision here. That said, trying this on a mac shows the same jdk
-        // path no matter what input is passed.
-        final String javaHomeOutput = globals.processUtils
-            .runSync(
-              <String>['/usr/libexec/java_home', '-v', '1.8'],
-              throwOnError: true,
-              hideStdout: true,
-            )
-            .stdout
-            .trim();
-        if (javaHomeOutput.isNotEmpty) {
-          final String javaHome = javaHomeOutput.split('\n').last.trim();
-          globals.printTrace('Using mac JAVA_HOME.');
-          return fileSystem.path.join(javaHome, 'bin', 'java');
-        }
-      } on Exception {/* ignore */}
-    }
-
     // Fallback to PATH based lookup.
     final String? pathJava = operatingSystemUtils.which(_javaExecutable)?.path;
     if (pathJava != null) {

packages/flutter_tools/lib/src/android/android_studio.dart

@@ -27,6 +27,16 @@ import 'android_studio_validator.dart';
 final RegExp _dotHomeStudioVersionMatcher =
     RegExp(r'^\.?(AndroidStudio[^\d]*)([\d.]+)');
 
+// TODO(andrewkolos): this global variable is used in several places to provide
+// a java binary to multiple Java-dependent tools, including the Android SDK
+// and Gradle. If this is null, these tools will implicitly fall back to current
+// JAVA_HOME env variable and then to any java found on PATH.
+//
+// This logic is consistent with that used by flutter doctor to find a valid JDK,
+// but this consistency is fragile--the implementations of this logic
+// exist independently of each other.
+//
+// See https://github.com/flutter/flutter/issues/124252.
 String? get javaPath => globals.androidStudio?.javaPath;
 
 class AndroidStudio implements Comparable<AndroidStudio> {

packages/flutter_tools/test/general.shard/android/android_sdk_test.dart

@@ -8,9 +8,11 @@ import 'package:flutter_tools/src/android/android_studio.dart';
 import 'package:flutter_tools/src/base/config.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/base/process.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;
+import 'package:test/fake.dart';
 
 import '../../integration.shard/test_utils.dart';
 import '../../src/common.dart';
@@ -346,6 +348,87 @@ void main() {
       Platform: () => FakePlatform(operatingSystem: 'windows'),
       Config: () => config,
     });
+
+    group('findJavaBinary', () {
+      testUsingContext('returns the path of the JDK bundled with Android Studio, if it exists', () {
+        final String androidStudioBundledJdkHome = globals.androidStudio!.javaPath!;
+        final String expectedJavaBinaryPath = globals.fs.path.join(androidStudioBundledJdkHome, 'bin', 'java');
+
+        final String? foundJavaBinaryPath = AndroidSdk.findJavaBinary(
+          androidStudio: globals.androidStudio,
+          fileSystem: globals.fs,
+          operatingSystemUtils: globals.os,
+          platform: globals.platform,
+        );
+
+        expect(foundJavaBinaryPath, expectedJavaBinaryPath);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => MemoryFileSystem.test(),
+        ProcessManager: () => FakeProcessManager.any(),
+        OperatingSystemUtils: () => FakeOperatingSystemUtilsWithJava(),
+        Platform: () => FakePlatform(),
+        Config: () => Config,
+        AndroidStudio: () => FakeAndroidStudioWithJdk(),
+      });
+
+      testUsingContext('returns the current value of JAVA_HOME if it is set and the JDK bundled with Android Studio could not be found', () {
+        final String expectedJavaBinaryPath = globals.fs.path.join('java-home-path', 'bin', 'java');
+
+        final String? foundJavaBinaryPath = AndroidSdk.findJavaBinary(
+          androidStudio: globals.androidStudio,
+          fileSystem: globals.fs,
+          operatingSystemUtils: globals.os,
+          platform: globals.platform,
+        );
+
+        expect(foundJavaBinaryPath, expectedJavaBinaryPath);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => MemoryFileSystem.test(),
+        ProcessManager: () => FakeProcessManager.any(),
+        OperatingSystemUtils: () => FakeOperatingSystemUtilsWithJava(),
+        Platform: () => FakePlatform(environment: <String, String>{
+          'JAVA_HOME': 'java-home-path',
+        }),
+        Config: () => Config,
+        AndroidStudio: () => FakeAndroidStudioWithoutJdk(),
+      });
+
+      testUsingContext('returns the java binary found on PATH if no other can be found', () {
+        final String? foundJavaBinaryPath = AndroidSdk.findJavaBinary(
+          androidStudio: globals.androidStudio,
+          fileSystem: globals.fs,
+          operatingSystemUtils: globals.os,
+          platform: globals.platform,
+        );
+
+        expect(foundJavaBinaryPath, globals.os.which('java')!.path);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => MemoryFileSystem.test(),
+        ProcessManager: () => FakeProcessManager.any(),
+        OperatingSystemUtils: () => FakeOperatingSystemUtilsWithJava(),
+        Platform: () => FakePlatform(),
+        Config: () => Config,
+        AndroidStudio: () => FakeAndroidStudioWithoutJdk(),
+      });
+
+      testUsingContext('returns null if no java binary could be found', () {
+        final String? foundJavaBinaryPath = AndroidSdk.findJavaBinary(
+          androidStudio: globals.androidStudio,
+          fileSystem: globals.fs,
+          operatingSystemUtils: globals.os,
+          platform: globals.platform,
+        );
+
+        expect(foundJavaBinaryPath, null);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => MemoryFileSystem.test(),
+        ProcessManager: () => FakeProcessManager.any(),
+        OperatingSystemUtils: () => FakeOperatingSystemUtilsWithoutJava(),
+        Platform: () => FakePlatform(),
+        Config: () => Config,
+        AndroidStudio: () => FakeAndroidStudioWithoutJdk(),
+      });
+    });
   });
 
   group('java version', () {
@@ -553,3 +636,30 @@ ro.build.version.incremental=1624448
 ro.build.version.sdk=24
 ro.build.version.codename=REL
 ''';
+
+class FakeAndroidStudioWithJdk extends Fake implements AndroidStudio {
+  @override
+  String? get javaPath => '/fake/android_studio/java/path/';
+}
+
+class FakeAndroidStudioWithoutJdk extends Fake implements AndroidStudio {
+  @override
+  String? get javaPath => null;
+}
+
+class FakeOperatingSystemUtilsWithJava extends Fake implements OperatingSystemUtils {
+  @override
+  File? which(String execName) {
+    if (execName == 'java') {
+      return globals.fs.file('/fake/which/java/path');
+    }
+    return null;
+  }
+}
+
+class FakeOperatingSystemUtilsWithoutJava extends Fake implements OperatingSystemUtils {
+  @override
+  File? which(String execName) {
+    return null;
+  }
+}