From cc3a09790cae0c51a2a6bcb061e4717b8ad63544 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 11:56:20 -0500 Subject: [PATCH 1/8] Tweak Exception class --- .../java/com/google/android/ground/system/GoogleApiManager.kt | 4 ++-- .../com/google/android/ground/ui/startup/StartupFragment.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index 685d871705..f549ab7a77 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -42,7 +42,7 @@ constructor( if (status == ConnectionResult.SUCCESS) return val requestCode = INSTALL_API_REQUEST_CODE - startResolution(status, requestCode, GooglePlayServicesMissingException()) + startResolution(status, requestCode, GooglePlayServicesNotAvailableException()) getNextResult(requestCode) } @@ -61,5 +61,5 @@ constructor( } } - class GooglePlayServicesMissingException : Error("Google play services not available") + class GooglePlayServicesNotAvailableException : Error("Google Play Services not available") } diff --git a/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt b/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt index 648118f537..fc9e095ea1 100644 --- a/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt @@ -66,7 +66,7 @@ class StartupFragment : AbstractFragment() { private fun onInitFailed(t: Throwable) { Timber.e(t, "Failed to launch app") - if (t is GoogleApiManager.GooglePlayServicesMissingException) { + if (t is GoogleApiManager.GooglePlayServicesNotAvailableException) { popups.ErrorPopup().show(R.string.google_api_install_failed) } requireActivity().finish() From 174dc36e76eb2ac0dfad16be22b8e0497619473e Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 12:01:29 -0500 Subject: [PATCH 2/8] Use GMS exception --- .../com/google/android/ground/system/GoogleApiManager.kt | 5 ++--- .../com/google/android/ground/ui/startup/StartupFragment.kt | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index f549ab7a77..eb8e3a5fda 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -18,6 +18,7 @@ package com.google.android.ground.system import android.content.Context import com.google.android.gms.common.ConnectionResult import com.google.android.gms.common.GoogleApiAvailability +import com.google.android.gms.common.GooglePlayServicesNotAvailableException import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject import javax.inject.Singleton @@ -42,7 +43,7 @@ constructor( if (status == ConnectionResult.SUCCESS) return val requestCode = INSTALL_API_REQUEST_CODE - startResolution(status, requestCode, GooglePlayServicesNotAvailableException()) + startResolution(status, requestCode, GooglePlayServicesNotAvailableException(status)) getNextResult(requestCode) } @@ -60,6 +61,4 @@ constructor( error("Activity result failed: requestCode = $requestCode, result = $result") } } - - class GooglePlayServicesNotAvailableException : Error("Google Play Services not available") } diff --git a/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt b/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt index fc9e095ea1..b21ebe0954 100644 --- a/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt +++ b/ground/src/main/java/com/google/android/ground/ui/startup/StartupFragment.kt @@ -20,8 +20,8 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.lifecycle.lifecycleScope +import com.google.android.gms.common.GooglePlayServicesNotAvailableException import com.google.android.ground.R -import com.google.android.ground.system.GoogleApiManager import com.google.android.ground.ui.common.AbstractFragment import com.google.android.ground.ui.common.EphemeralPopups import dagger.hilt.android.AndroidEntryPoint @@ -66,7 +66,7 @@ class StartupFragment : AbstractFragment() { private fun onInitFailed(t: Throwable) { Timber.e(t, "Failed to launch app") - if (t is GoogleApiManager.GooglePlayServicesNotAvailableException) { + if (t is GooglePlayServicesNotAvailableException) { popups.ErrorPopup().show(R.string.google_api_install_failed) } requireActivity().finish() From abb5d8b4d8fb937c7af1b086fa1377357869990e Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 12:13:39 -0500 Subject: [PATCH 3/8] Refactor API install error handling --- .../android/ground/system/GoogleApiManager.kt | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index eb8e3a5fda..690c996b13 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -40,25 +40,23 @@ constructor( */ suspend fun installGooglePlayServices() { val status = googleApiAvailability.isGooglePlayServicesAvailable(context) - if (status == ConnectionResult.SUCCESS) return - - val requestCode = INSTALL_API_REQUEST_CODE - startResolution(status, requestCode, GooglePlayServicesNotAvailableException(status)) - getNextResult(requestCode) + if (status != ConnectionResult.SUCCESS) { + resolveError( + status, + INSTALL_API_REQUEST_CODE, + GooglePlayServicesNotAvailableException(status), + ) + } } - private fun startResolution(status: Int, requestCode: Int, throwable: Throwable) { + private suspend fun resolveError(status: Int, requestCode: Int, throwable: Throwable) { if (!googleApiAvailability.isUserResolvableError(status)) throw throwable activityStreams.withActivity { googleApiAvailability.showErrorDialogFragment(it, status, requestCode) { throw throwable } } - } - - private suspend fun getNextResult(requestCode: Int) { - val result = activityStreams.getNextActivityResult(requestCode) - if (!result.isOk()) { - error("Activity result failed: requestCode = $requestCode, result = $result") + if (!activityStreams.getNextActivityResult(requestCode).isOk()) { + throw throwable } } } From 1b94d002c2f7806c76b1698f04ed67e44b5f78c6 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 12:18:25 -0500 Subject: [PATCH 4/8] Refactor error handling --- .../android/ground/system/GoogleApiManager.kt | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index 690c996b13..78998b9eb8 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -41,22 +41,22 @@ constructor( suspend fun installGooglePlayServices() { val status = googleApiAvailability.isGooglePlayServicesAvailable(context) if (status != ConnectionResult.SUCCESS) { - resolveError( - status, - INSTALL_API_REQUEST_CODE, - GooglePlayServicesNotAvailableException(status), - ) + if (!resolveError(status, INSTALL_API_REQUEST_CODE)) + throw GooglePlayServicesNotAvailableException(status) } } - private suspend fun resolveError(status: Int, requestCode: Int, throwable: Throwable) { - if (!googleApiAvailability.isUserResolvableError(status)) throw throwable + /** + * Attempts to resolve the error indicated by the given `status` code, using the provided + * `requestCode` to uniquely identify Activity callbacks. + */ + private suspend fun resolveError(status: Int, requestCode: Int): Boolean { + if (!googleApiAvailability.isUserResolvableError(status)) return false - activityStreams.withActivity { - googleApiAvailability.showErrorDialogFragment(it, status, requestCode) { throw throwable } - } - if (!activityStreams.getNextActivityResult(requestCode).isOk()) { - throw throwable + activityStreams.withActivity { activity -> + googleApiAvailability.showErrorDialogFragment(activity, status, requestCode) } + // `isOk()` returns `false` if install failed or was cancelled. + return activityStreams.getNextActivityResult(requestCode).isOk() } } From b8b3ad22fd8cd72b605b04da705fa1dee9d67908 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 12:28:48 -0500 Subject: [PATCH 5/8] Tweak ktdoc --- .../com/google/android/ground/ui/startup/StartupViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ground/src/main/java/com/google/android/ground/ui/startup/StartupViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/startup/StartupViewModel.kt index cff41bdee6..73ec0cef28 100644 --- a/ground/src/main/java/com/google/android/ground/ui/startup/StartupViewModel.kt +++ b/ground/src/main/java/com/google/android/ground/ui/startup/StartupViewModel.kt @@ -27,7 +27,7 @@ internal constructor( private val userRepository: UserRepository, ) : AbstractViewModel() { - /** Checks & installs Google Play Services and initializes the login flow. */ + /** Initializes the login flow, installing Google Play Services if necessary. */ suspend fun initializeLogin() { googleApiManager.installGooglePlayServices() userRepository.init() From 59819aec014d5e0843410001c6e8288814891d66 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 15:57:05 -0500 Subject: [PATCH 6/8] Clean up error handing of Play services install --- .../android/ground/system/GoogleApiManager.kt | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index 78998b9eb8..a84e7e5ecc 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -16,14 +16,19 @@ package com.google.android.ground.system import android.content.Context -import com.google.android.gms.common.ConnectionResult +import com.google.android.gms.common.ConnectionResult.SUCCESS import com.google.android.gms.common.GoogleApiAvailability import com.google.android.gms.common.GooglePlayServicesNotAvailableException import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.resume +import kotlin.coroutines.suspendCoroutine +import kotlinx.coroutines.delay +import timber.log.Timber private val INSTALL_API_REQUEST_CODE = GoogleApiAvailability::class.java.hashCode() and 0xffff +private const val PLAY_SERVICES_RETRY_DELAY_MS = 2500L @Singleton class GoogleApiManager @@ -39,24 +44,35 @@ constructor( * possible or cancelled. */ suspend fun installGooglePlayServices() { - val status = googleApiAvailability.isGooglePlayServicesAvailable(context) - if (status != ConnectionResult.SUCCESS) { - if (!resolveError(status, INSTALL_API_REQUEST_CODE)) - throw GooglePlayServicesNotAvailableException(status) + val status = isGooglePlayServicesAvailable() + if (status == SUCCESS) return + if (googleApiAvailability.isUserResolvableError(status)) { + showErrorDialog(status, INSTALL_API_REQUEST_CODE) + } else { + throw GooglePlayServicesNotAvailableException(status) + } + // onActivityResult() is sometimes called with a failure prematurely or not at all. Instead, we + // poll for Play services. + while (isGooglePlayServicesAvailable() != SUCCESS) { + Timber.d("Waiting for Play services") + delay(PLAY_SERVICES_RETRY_DELAY_MS) } } + private fun isGooglePlayServicesAvailable(): Int = + googleApiAvailability.isGooglePlayServicesAvailable(context) + /** * Attempts to resolve the error indicated by the given `status` code, using the provided * `requestCode` to uniquely identify Activity callbacks. */ - private suspend fun resolveError(status: Int, requestCode: Int): Boolean { - if (!googleApiAvailability.isUserResolvableError(status)) return false - - activityStreams.withActivity { activity -> - googleApiAvailability.showErrorDialogFragment(activity, status, requestCode) + private suspend fun showErrorDialog(status: Int, requestCode: Int) = + suspendCoroutine { continuation -> + activityStreams.withActivity { activity -> + val dialog = googleApiAvailability.getErrorDialog(activity, status, requestCode) + dialog?.setCanceledOnTouchOutside(false) + dialog?.setOnDismissListener { continuation.resume(Unit) } + dialog?.show() + } } - // `isOk()` returns `false` if install failed or was cancelled. - return activityStreams.getNextActivityResult(requestCode).isOk() - } } From 49768a99368f78a69f4c858db29f75dbf8970ff8 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 16:02:21 -0500 Subject: [PATCH 7/8] Tweak ktdoc --- .../java/com/google/android/ground/system/GoogleApiManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index a84e7e5ecc..53df873fff 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -64,7 +64,7 @@ constructor( /** * Attempts to resolve the error indicated by the given `status` code, using the provided - * `requestCode` to uniquely identify Activity callbacks. + * `requestCode` to differentiate Activity callbacks from others. */ private suspend fun showErrorDialog(status: Int, requestCode: Int) = suspendCoroutine { continuation -> From b6007c94c956e358ac0133c849ae8070b376a714 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 11 Dec 2024 16:02:39 -0500 Subject: [PATCH 8/8] Tweak ktdoc --- .../java/com/google/android/ground/system/GoogleApiManager.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt index 53df873fff..b1758dab33 100644 --- a/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/GoogleApiManager.kt @@ -64,7 +64,8 @@ constructor( /** * Attempts to resolve the error indicated by the given `status` code, using the provided - * `requestCode` to differentiate Activity callbacks from others. + * `requestCode` to differentiate Activity callbacks from others. Suspends until the dialog is + * dismissed. */ private suspend fun showErrorDialog(status: Int, requestCode: Int) = suspendCoroutine { continuation ->