tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit db5b83a59660343ad980df86781bd2760489ec92
parent 5302780bc645221c713a31954fee84e3a9071aa5
Author: Marcin KoziƄski <mkozinski@mozilla.com>
Date:   Wed, 15 Oct 2025 11:30:59 +0000

Bug 1992091 - Use Play Store prompt fallback only after "Rate" button was clicked r=android-reviewers,twhite

Differential Revision: https://phabricator.services.mozilla.com/D268514

Diffstat:
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/PlayStoreReviewPromptController.kt | 7+++++--
Mmobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/CustomReviewPromptBottomSheetFragment.kt | 7++++++-
Mmobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/PlayStoreReviewPromptControllerTest.kt | 100+++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
3 files changed, 81 insertions(+), 33 deletions(-)

diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/PlayStoreReviewPromptController.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/PlayStoreReviewPromptController.kt @@ -36,7 +36,10 @@ class PlayStoreReviewPromptController( /** * Launch the in-app review flow, unless we've hit the quota. */ - suspend fun tryPromptReview(activity: Activity) { + suspend fun tryPromptReview( + activity: Activity, + onError: () -> Unit = {}, + ) { logger.info("tryPromptReview in progress...") val reviewInfoFlow = withContext(Dispatchers.IO) { manager.requestReviewFlow() } @@ -53,7 +56,7 @@ class PlayStoreReviewPromptController( (it.exception as? ReviewException)?.errorCode ?: ERROR_CODE_UNEXPECTED logger.warn("Failed to launch in-app review flow due to: $reviewErrorCode .") - tryLaunchPlayStoreReview(activity) + onError() "reviewErrorCode=$reviewErrorCode" } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/CustomReviewPromptBottomSheetFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/reviewprompt/CustomReviewPromptBottomSheetFragment.kt @@ -82,7 +82,12 @@ class CustomReviewPromptBottomSheetFragment : BottomSheetDialogFragment() { CustomReviewPromptNavigationEvent.OpenPlayStoreReviewPrompt -> { val activity = activity ?: return@collect - requireComponents.playStoreReviewPromptController.tryPromptReview(activity) + with(requireComponents.playStoreReviewPromptController) { + tryPromptReview( + activity = activity, + onError = { tryLaunchPlayStoreReview(activity) }, + ) + } } is CustomReviewPromptNavigationEvent.OpenNewTab -> { diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/PlayStoreReviewPromptControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/PlayStoreReviewPromptControllerTest.kt @@ -40,15 +40,14 @@ class PlayStoreReviewPromptControllerTest { @get:Rule val gleanTestRule = FenixGleanTestRule(testContext) - private val reviewManager = FakeReviewManager(testContext) - private val controller = PlayStoreReviewPromptController( - manager = reviewManager, - numberOfAppLaunches = { 5 }, - ) - @Test fun `GIVEN activity is resumed WHEN tryPromptReview is called THEN launches review flow`() = runTest { + val reviewManager = SuccessfulReviewManager(testContext) + val controller = PlayStoreReviewPromptController( + manager = reviewManager, + numberOfAppLaunches = { 5 }, + ) val scenario = launchActivity<ComponentActivity>() scenario.moveToState(Lifecycle.State.RESUMED) @@ -64,9 +63,14 @@ class PlayStoreReviewPromptControllerTest { @Test fun `GIVEN activity is stopped WHEN tryPromptReview is called THEN doesn't run the on complete callback`() = runTest { + val reviewManager = SuccessfulReviewManager(testContext) + val controller = PlayStoreReviewPromptController( + manager = reviewManager, + numberOfAppLaunches = { 5 }, + ) val scenario = launchActivity<ComponentActivity>() scenario.moveToState(Lifecycle.State.RESUMED) - scenario.moveToState(Lifecycle.State.CREATED) + scenario.moveToState(Lifecycle.State.CREATED) // Move back from resumed to created, in effect stopping it. scenario.onActivity { activity -> launch { @@ -78,6 +82,28 @@ class PlayStoreReviewPromptControllerTest { } @Test + fun `WHEN the reviews API fails THEN runs the error callback`() = runTest { + val controller = PlayStoreReviewPromptController( + manager = FailingReviewManager(), + numberOfAppLaunches = { 5 }, + ) + val scenario = launchActivity<ComponentActivity>() + scenario.moveToState(Lifecycle.State.RESUMED) + var onErrorRan = false + + scenario.onActivity { activity -> + launch { + controller.tryPromptReview( + activity = activity, + onError = { onErrorRan = true }, + ) + + assertTrue(onErrorRan) + } + } + } + + @Test fun reviewPromptWasDisplayed() { testRecordReviewPromptEventRecordsTheExpectedData("isNoOp=false", "true") } @@ -122,24 +148,41 @@ class PlayStoreReviewPromptControllerTest { } } -private class FakeReviewManager(context: Context) : ReviewManager { +private class SuccessfulReviewManager(context: Context) : ReviewManager { var promptHasBeenRequested = false private val wrapped = com.google.android.play.core.review.testing.FakeReviewManager(context) - override fun requestReviewFlow(): FakeGmsTask<ReviewInfo> { + override fun requestReviewFlow(): Task<ReviewInfo> { val requestReviewFlow = wrapped.requestReviewFlow() - val result = requestReviewFlow.result - return FakeGmsTask(result) + return SuccessfulTask(requestReviewFlow.result) } - override fun launchReviewFlow(activity: Activity, reviewInfo: ReviewInfo): FakeGmsTask<Void?> { + override fun launchReviewFlow(activity: Activity, reviewInfo: ReviewInfo): Task<Void?> { promptHasBeenRequested = true - return FakeGmsTask(null) + return VoidTask() } } -private class FakeGmsTask<T>(private val result: T) : Task<T>() { +private class FailingReviewManager : ReviewManager { + override fun requestReviewFlow() = FailingTask<ReviewInfo>() + override fun launchReviewFlow(activity: Activity, reviewInfo: ReviewInfo) = assertUnused() +} + +private class SuccessfulTask<T>(private val result: T) : FakeGmsTask<T>() { + override fun isSuccessful() = true + override fun getResult() = result + override fun <X : Throwable?> getResult(exceptionType: Class<X>) = result +} + +private class FailingTask<T> : FakeGmsTask<T>() { + override fun isSuccessful() = false + override fun getException() = RuntimeException("Unexpected exception.") +} + +private class VoidTask : FakeGmsTask<Void?>() + +private open class FakeGmsTask<T> : Task<T>() { override fun addOnCompleteListener(activity: Activity, listener: OnCompleteListener<T>): Task<T> { val isNotStopped = (activity as ComponentActivity).lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED) if (isNotStopped) { @@ -148,20 +191,17 @@ private class FakeGmsTask<T>(private val result: T) : Task<T>() { return this } - override fun isSuccessful() = true - - override fun getResult() = result - - override fun <X : Throwable?> getResult(exceptionType: Class<X>) = result - - override fun isComplete() = assertUnused() - override fun isCanceled() = assertUnused() - override fun getException() = assertUnused() - override fun addOnSuccessListener(listener: OnSuccessListener<in T>) = assertUnused() - override fun addOnSuccessListener(executor: Executor, listener: OnSuccessListener<in T>) = assertUnused() - override fun addOnSuccessListener(activity: Activity, listener: OnSuccessListener<in T>) = assertUnused() - override fun addOnFailureListener(listener: OnFailureListener) = assertUnused() - override fun addOnFailureListener(executor: Executor, listener: OnFailureListener) = assertUnused() - override fun addOnFailureListener(activity: Activity, listener: OnFailureListener) = assertUnused() - override fun addOnCompleteListener(listener: OnCompleteListener<T>) = assertUnused() + override fun isSuccessful(): Boolean = assertUnused() + override fun isComplete(): Boolean = assertUnused() + override fun isCanceled(): Boolean = assertUnused() + override fun getResult(): T? = assertUnused() + override fun <X : Throwable?> getResult(exceptionType: Class<X>): T? = assertUnused() + override fun getException(): Exception? = assertUnused() + override fun addOnSuccessListener(listener: OnSuccessListener<in T>): Task<T> = assertUnused() + override fun addOnSuccessListener(executor: Executor, listener: OnSuccessListener<in T>): Task<T> = assertUnused() + override fun addOnSuccessListener(activity: Activity, listener: OnSuccessListener<in T>): Task<T> = assertUnused() + override fun addOnFailureListener(listener: OnFailureListener): Task<T> = assertUnused() + override fun addOnFailureListener(executor: Executor, listener: OnFailureListener): Task<T> = assertUnused() + override fun addOnFailureListener(activity: Activity, listener: OnFailureListener): Task<T> = assertUnused() + override fun addOnCompleteListener(listener: OnCompleteListener<T>): Task<T> = assertUnused() }