commit 6b7b5631645f96916f651e3fa5d1151a7696a5cd
parent 176bd8cfc0f8240c7daca1114a95f42272d2856e
Author: Lando <lando@lando.test>
Date: Mon, 6 Oct 2025 10:14:28 +0000
Merge mozilla-central to autoland
Diffstat:
2 files changed, 39 insertions(+), 105 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
@@ -9,7 +9,6 @@ import android.content.ActivityNotFoundException
import android.content.Intent
import androidx.annotation.VisibleForTesting
import androidx.core.net.toUri
-import com.google.android.gms.tasks.Task
import com.google.android.play.core.review.ReviewException
import com.google.android.play.core.review.ReviewInfo
import com.google.android.play.core.review.ReviewManager
@@ -20,9 +19,6 @@ import mozilla.components.support.base.log.logger.Logger
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.ReviewPrompt
import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.components.ReviewPromptDisplayState.Displayed
-import org.mozilla.fenix.components.ReviewPromptDisplayState.NotDisplayed
-import org.mozilla.fenix.components.ReviewPromptDisplayState.Unknown
import org.mozilla.fenix.settings.SupportUtils
import java.text.SimpleDateFormat
import java.util.Date
@@ -43,30 +39,23 @@ class PlayStoreReviewPromptController(
*/
suspend fun tryPromptReview(activity: Activity) {
logger.info("tryPromptReview in progress...")
- val request = withContext(Dispatchers.IO) { manager.requestReviewFlow() }
+ val reviewInfoFlow = withContext(Dispatchers.IO) { manager.requestReviewFlow() }
- request.addOnCompleteListener(activity) { task ->
- val promptWasDisplayed: Boolean
-
- if (task.isSuccessful) {
- logger.info("Launching in-app review flow.")
- manager.launchReviewFlow(activity, task.result)
- promptWasDisplayed = task.result.promptDisplayState == Displayed
- if (!promptWasDisplayed) {
- logger.warn("Looks like in-app review flow wasn't displayed, even though there was no error.")
- }
+ reviewInfoFlow.addOnCompleteListener {
+ if (it.isSuccessful) {
+ logger.info("Review flow launched.")
+ // Launch the in-app flow.
+ manager.launchReviewFlow(activity, it.result)
} else {
- promptWasDisplayed = false
-
- logger.warn("Failed to launch in-app review flow due to: ${task.reviewErrorCode}.")
- }
+ // Launch the Play store flow.
+ @ReviewErrorCode val reviewErrorCode = (it.exception as ReviewException).errorCode
+ logger.warn("Failed to launch in-app review flow due to: $reviewErrorCode .")
- if (!promptWasDisplayed) {
tryLaunchPlayStoreReview(activity)
}
recordReviewPromptEvent(
- promptDisplayState = task.result.promptDisplayState,
+ reviewInfoAsString = it.result.toString(),
numberOfAppLaunches = numberOfAppLaunches(),
now = Date(),
)
@@ -94,72 +83,43 @@ class PlayStoreReviewPromptController(
newTab = true,
from = BrowserDirection.FromSettings,
)
- logger.warn("Failed to launch play store review flow due to: $e.")
+ logger.warn("Failed to launch play store review flow due to: $e .")
}
logger.info("tryLaunchPlayStoreReview completed.")
}
}
-@ReviewErrorCode
-private val Task<ReviewInfo>.reviewErrorCode: Int
- get() = (exception as ReviewException).errorCode
-
-private val ReviewInfo.promptDisplayState: ReviewPromptDisplayState
- get() {
- // The internals of ReviewInfo cannot be accessed directly or cast nicely, so let's simply use
- // the object as a string.
- return ReviewPromptDisplayState.from(reviewInfoAsString = toString())
- }
-
-/**
- * Result of an attempt to determine if Play Store In-App Review Prompt was displayed.
- */
-@VisibleForTesting
-enum class ReviewPromptDisplayState {
- NotDisplayed, Displayed, Unknown;
-
- /**
- * @see [ReviewPromptDisplayState]
- */
- companion object {
- /**
- * The docs for [ReviewManager.launchReviewFlow] state 'In some circumstances the review
- * flow will not be shown to the user, e.g. they have already seen it recently, so do not assume that
- * calling this method will always display the review dialog.'
- * However, investigation has shown that a [ReviewInfo] instance with the flag:
- * - 'isNoOp=true' indicates that the prompt has NOT been displayed.
- * - 'isNoOp=false' indicates that a prompt has been displayed.
- * [ReviewManager.launchReviewFlow] will modify the ReviewInfo instance which can be used to determine
- * which of these flags is present.
- */
- fun from(reviewInfoAsString: String): ReviewPromptDisplayState {
- return when {
- reviewInfoAsString.contains("isNoOp=true") -> NotDisplayed
- reviewInfoAsString.contains("isNoOp=false") -> Displayed
- // ReviewInfo is susceptible to changes outside of our control hence the catch-all 'else' statement.
- else -> Unknown
- }
- }
- }
-}
-
/**
* Records a [ReviewPrompt] with the required data.
+ *
+ * **Note:** The docs for [ReviewManager.launchReviewFlow] state 'In some circumstances the review
+ * flow will not be shown to the user, e.g. they have already seen it recently, so do not assume that
+ * calling this method will always display the review dialog.'
+ * However, investigation has shown that a [ReviewInfo] instance with the flag:
+ * - 'isNoOp=true' indicates that the prompt has NOT been displayed.
+ * - 'isNoOp=false' indicates that a prompt has been displayed.
+ * [ReviewManager.launchReviewFlow] will modify the ReviewInfo instance which can be used to determine
+ * which of these flags is present.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
fun recordReviewPromptEvent(
- promptDisplayState: ReviewPromptDisplayState,
+ reviewInfoAsString: String,
numberOfAppLaunches: Int,
now: Date,
) {
val formattedLocalDatetime =
SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss", Locale.getDefault()).format(now)
- val promptWasDisplayed = when (promptDisplayState) {
- NotDisplayed -> "false"
- Displayed -> "true"
- Unknown -> "error"
+ // The internals of ReviewInfo cannot be accessed directly or cast nicely, so lets simply use
+ // the object as a string.
+ // ReviewInfo is susceptible to changes outside of our control hence the catch-all 'else' statement.
+ val promptWasDisplayed = if (reviewInfoAsString.contains("isNoOp=true")) {
+ "false"
+ } else if (reviewInfoAsString.contains("isNoOp=false")) {
+ "true"
+ } else {
+ "error"
}
ReviewPrompt.promptAttempt.record(
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
@@ -27,9 +27,6 @@ import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.ReviewPrompt
-import org.mozilla.fenix.components.ReviewPromptDisplayState.Displayed
-import org.mozilla.fenix.components.ReviewPromptDisplayState.NotDisplayed
-import org.mozilla.fenix.components.ReviewPromptDisplayState.Unknown
import org.mozilla.fenix.helpers.FenixGleanTestRule
import org.robolectric.RobolectricTestRunner
import java.text.SimpleDateFormat
@@ -81,58 +78,35 @@ class PlayStoreReviewPromptControllerTest {
}
@Test
- fun `WHEN review info contains 'isNoOp=false' THEN prompt was displayed`() {
- val displayState = ReviewPromptDisplayState.from(createReviewInfoString("isNoOp=false"))
-
- assertEquals(Displayed, displayState)
- }
-
- @Test
- fun `WHEN review info contains 'isNoOp=true' THEN prompt wasn't displayed`() {
- val displayState = ReviewPromptDisplayState.from(createReviewInfoString("isNoOp=true"))
-
- assertEquals(NotDisplayed, displayState)
- }
-
- @Test
- fun `WHEN review info doesn't contain 'isNoOp' THEN prompt display state is unknown`() {
- val displayState = ReviewPromptDisplayState.from(createReviewInfoString())
-
- assertEquals(Unknown, displayState)
- }
-
- private fun createReviewInfoString(optionalArg: String? = ""): String {
- return "ReviewInfo{pendingIntent=PendingIntent{5b613b1: android.os.BinderProxy@46c8096}, $optionalArg}"
- }
-
- @Test
fun reviewPromptWasDisplayed() {
- testRecordReviewPromptEventRecordsTheExpectedData(Displayed, "true")
+ testRecordReviewPromptEventRecordsTheExpectedData("isNoOp=false", "true")
}
@Test
fun reviewPromptWasNotDisplayed() {
- testRecordReviewPromptEventRecordsTheExpectedData(NotDisplayed, "false")
+ testRecordReviewPromptEventRecordsTheExpectedData("isNoOp=true", "false")
}
@Test
fun reviewPromptDisplayStateUnknown() {
- testRecordReviewPromptEventRecordsTheExpectedData(Unknown, "error")
+ testRecordReviewPromptEventRecordsTheExpectedData(expected = "error")
}
private fun testRecordReviewPromptEventRecordsTheExpectedData(
- promptDisplayState: ReviewPromptDisplayState,
- promptWasDisplayed: String,
+ reviewInfoArg: String = "",
+ expected: String,
) {
val numberOfAppLaunches = 1
+ val reviewInfoAsString =
+ "ReviewInfo{pendingIntent=PendingIntent{5b613b1: android.os.BinderProxy@46c8096}, $reviewInfoArg}"
val datetime = Date(TEST_TIME_NOW)
val formattedNowLocalDatetime = SIMPLE_DATE_FORMAT.format(datetime)
assertNull(ReviewPrompt.promptAttempt.testGetValue())
- recordReviewPromptEvent(promptDisplayState, numberOfAppLaunches, datetime)
+ recordReviewPromptEvent(reviewInfoAsString, numberOfAppLaunches, datetime)
val reviewPromptData = ReviewPrompt.promptAttempt.testGetValue()!!.last().extra!!
- assertEquals(promptWasDisplayed, reviewPromptData["prompt_was_displayed"])
+ assertEquals(expected, reviewPromptData["prompt_was_displayed"])
assertEquals(numberOfAppLaunches, reviewPromptData["number_of_app_launches"]!!.toInt())
assertEquals(formattedNowLocalDatetime, reviewPromptData["local_datetime"])
}
