mirror of
https://github.com/fork-maintainers/iceraven-browser
synced 2024-11-11 13:11:01 +00:00
[fenix] For https://github.com/mozilla-mobile/fenix/issues/13507 - Performance fixes for the ReviewPromptController
This commit is contained in:
parent
eb45012b4b
commit
4e2b53b340
@ -43,6 +43,7 @@ import mozilla.components.support.webextensions.WebExtensionSupport
|
||||
import org.mozilla.fenix.StrictModeManager.enableStrictMode
|
||||
import org.mozilla.fenix.components.Components
|
||||
import org.mozilla.fenix.components.metrics.MetricServiceType
|
||||
import org.mozilla.fenix.ext.components
|
||||
import org.mozilla.fenix.ext.resetPoliciesAfter
|
||||
import org.mozilla.fenix.ext.settings
|
||||
import org.mozilla.fenix.perf.StorageStatsMetrics
|
||||
@ -217,6 +218,12 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
|
||||
}
|
||||
}
|
||||
|
||||
fun queueReviewPrompt() {
|
||||
GlobalScope.launch(Dispatchers.IO) {
|
||||
components.reviewPromptController.trackApplicationLaunch()
|
||||
}
|
||||
}
|
||||
|
||||
initQueue()
|
||||
|
||||
// We init these items in the visual completeness queue to avoid them initing in the critical
|
||||
@ -224,6 +231,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
|
||||
queueInitExperiments()
|
||||
queueInitStorageAndServices()
|
||||
queueMetrics()
|
||||
queueReviewPrompt()
|
||||
}
|
||||
|
||||
private fun startMetricsIfEnabled() {
|
||||
|
@ -232,8 +232,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
|
||||
|
||||
setAppAllStartTelemetry(intent.toSafeIntent())
|
||||
|
||||
components.services.reviewPromptController.trackApplicationLaunch()
|
||||
|
||||
StartupTimeline.onActivityCreateEndHome(this) // DO NOT MOVE ANYTHING BELOW HERE.
|
||||
}
|
||||
|
||||
|
@ -19,6 +19,7 @@ import mozilla.components.support.migration.state.MigrationStore
|
||||
import org.mozilla.fenix.BuildConfig
|
||||
import org.mozilla.fenix.HomeActivity
|
||||
import org.mozilla.fenix.components.metrics.AppAllSourceStartTelemetry
|
||||
import org.mozilla.fenix.ext.settings
|
||||
import org.mozilla.fenix.utils.ClipboardHandler
|
||||
import org.mozilla.fenix.utils.Mockable
|
||||
import org.mozilla.fenix.utils.Settings
|
||||
@ -114,4 +115,11 @@ class Components(private val context: Context) {
|
||||
val wifiConnectionMonitor by lazy { WifiConnectionMonitor(context.getSystemService()!!) }
|
||||
|
||||
val settings by lazy { Settings(context) }
|
||||
|
||||
val reviewPromptController by lazy {
|
||||
ReviewPromptController(
|
||||
context,
|
||||
FenixReviewSettings(settings)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
@ -10,6 +10,8 @@ import androidx.annotation.VisibleForTesting
|
||||
import com.google.android.play.core.ktx.launchReview
|
||||
import com.google.android.play.core.ktx.requestReview
|
||||
import com.google.android.play.core.review.ReviewManagerFactory
|
||||
import kotlinx.coroutines.Dispatchers.Main
|
||||
import kotlinx.coroutines.withContext
|
||||
import org.mozilla.fenix.utils.Settings
|
||||
|
||||
/**
|
||||
@ -47,9 +49,15 @@ class ReviewPromptController(
|
||||
private val tryPromptReview: suspend (Activity) -> Unit = {
|
||||
val manager = ReviewManagerFactory.create(context)
|
||||
val reviewInfo = manager.requestReview()
|
||||
|
||||
withContext(Main) {
|
||||
manager.launchReview(it, reviewInfo)
|
||||
}
|
||||
}
|
||||
) {
|
||||
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
|
||||
@Volatile var reviewPromptIsReady = false
|
||||
|
||||
suspend fun promptReview(activity: Activity) {
|
||||
if (shouldShowPrompt()) {
|
||||
tryPromptReview(activity)
|
||||
@ -59,10 +67,19 @@ class ReviewPromptController(
|
||||
|
||||
fun trackApplicationLaunch() {
|
||||
reviewSettings.numberOfAppLaunches = reviewSettings.numberOfAppLaunches + 1
|
||||
// We only want to show the the prompt after we've finished "launching" the application.
|
||||
reviewPromptIsReady = true
|
||||
}
|
||||
|
||||
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
|
||||
fun shouldShowPrompt(): Boolean {
|
||||
if (!reviewPromptIsReady) {
|
||||
return false
|
||||
} else {
|
||||
// We only want to try to show it once to avoid unnecessary disk reads
|
||||
reviewPromptIsReady = false
|
||||
}
|
||||
|
||||
if (!reviewSettings.isDefaultBrowser) { return false }
|
||||
|
||||
val hasOpenedFiveTimes = reviewSettings.numberOfAppLaunches >= NUMBER_OF_LAUNCHES_REQUIRED
|
||||
|
@ -14,7 +14,6 @@ import mozilla.components.feature.app.links.AppLinksInterceptor
|
||||
import mozilla.components.service.fxa.manager.FxaAccountManager
|
||||
import org.mozilla.fenix.R
|
||||
import org.mozilla.fenix.ext.getPreferenceKey
|
||||
import org.mozilla.fenix.ext.settings
|
||||
import org.mozilla.fenix.settings.SupportUtils
|
||||
import org.mozilla.fenix.utils.Mockable
|
||||
|
||||
@ -45,11 +44,4 @@ class Services(
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
val reviewPromptController by lazy {
|
||||
ReviewPromptController(
|
||||
context,
|
||||
FenixReviewSettings(context.settings())
|
||||
)
|
||||
}
|
||||
}
|
||||
|
@ -173,8 +173,6 @@ class HomeFragment : Fragment() {
|
||||
if (!onboarding.userHasBeenOnboarded()) {
|
||||
requireComponents.analytics.metrics.track(Event.OpenedAppFirstRun)
|
||||
}
|
||||
|
||||
requireComponents.services.reviewPromptController.promptReview(requireActivity())
|
||||
}
|
||||
}
|
||||
|
||||
@ -571,6 +569,10 @@ class HomeFragment : Fragment() {
|
||||
|
||||
// We only want this observer live just before we navigate away to the collection creation screen
|
||||
requireComponents.core.tabCollectionStorage.unregister(collectionStorageObserver)
|
||||
|
||||
lifecycleScope.launch(IO) {
|
||||
requireComponents.reviewPromptController.promptReview(requireActivity())
|
||||
}
|
||||
}
|
||||
|
||||
private fun dispatchModeChanges(mode: Mode) {
|
||||
|
@ -43,7 +43,9 @@ class ReviewPromptControllerTest {
|
||||
{ promptWasCalled = true }
|
||||
)
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
controller.promptReview(HomeActivity())
|
||||
|
||||
assertEquals(settings.lastReviewPromptTimeInMillis, 0L)
|
||||
assertFalse(promptWasCalled)
|
||||
}
|
||||
@ -64,11 +66,57 @@ class ReviewPromptControllerTest {
|
||||
{ promptWasCalled = true }
|
||||
)
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
controller.promptReview(HomeActivity())
|
||||
assertEquals(100L, settings.lastReviewPromptTimeInMillis)
|
||||
assertTrue(promptWasCalled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun promptReviewWillNotBeCalledIfNotReady() = runBlockingTest {
|
||||
var promptWasCalled = false
|
||||
val settings = TestReviewSettings(
|
||||
numberOfAppLaunches = 5,
|
||||
isDefault = true,
|
||||
lastReviewPromptTimeInMillis = 0L
|
||||
)
|
||||
|
||||
val controller = ReviewPromptController(
|
||||
testContext,
|
||||
settings,
|
||||
{ 100L },
|
||||
{ promptWasCalled = true }
|
||||
)
|
||||
|
||||
controller.promptReview(HomeActivity())
|
||||
assertFalse(promptWasCalled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun promptReviewWillUnreadyPromptAfterCalled() = runBlockingTest {
|
||||
var promptWasCalled = false
|
||||
val settings = TestReviewSettings(
|
||||
numberOfAppLaunches = 5,
|
||||
isDefault = true,
|
||||
lastReviewPromptTimeInMillis = 0L
|
||||
)
|
||||
|
||||
val controller = ReviewPromptController(
|
||||
testContext,
|
||||
settings,
|
||||
{ 100L },
|
||||
{ promptWasCalled = true }
|
||||
)
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
|
||||
assertTrue(controller.reviewPromptIsReady)
|
||||
controller.promptReview(HomeActivity())
|
||||
|
||||
assertFalse(controller.reviewPromptIsReady)
|
||||
assertTrue(promptWasCalled)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun trackApplicationLaunch() {
|
||||
val settings = TestReviewSettings(
|
||||
@ -80,12 +128,16 @@ class ReviewPromptControllerTest {
|
||||
val controller = ReviewPromptController(
|
||||
testContext,
|
||||
settings,
|
||||
{ 100L }
|
||||
{ 0L }
|
||||
)
|
||||
|
||||
assertFalse(controller.reviewPromptIsReady)
|
||||
assertEquals(4, settings.numberOfAppLaunches)
|
||||
|
||||
controller.trackApplicationLaunch()
|
||||
|
||||
assertEquals(5, settings.numberOfAppLaunches)
|
||||
assertTrue(controller.reviewPromptIsReady)
|
||||
}
|
||||
|
||||
@Test
|
||||
@ -99,28 +151,31 @@ class ReviewPromptControllerTest {
|
||||
val controller = ReviewPromptController(
|
||||
testContext,
|
||||
settings,
|
||||
{ 1598416882805L }
|
||||
{ TEST_TIME_NOW }
|
||||
)
|
||||
|
||||
// Test first success criteria
|
||||
controller.reviewPromptIsReady = true
|
||||
assertTrue(controller.shouldShowPrompt())
|
||||
|
||||
// Test with last prompt approx 4 months earlier
|
||||
settings.apply {
|
||||
numberOfAppLaunches = 5
|
||||
isDefault = true
|
||||
lastReviewPromptTimeInMillis = 1588048882804L
|
||||
lastReviewPromptTimeInMillis = MORE_THAN_4_MONTHS_FROM_TEST_TIME_NOW
|
||||
}
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
assertTrue(controller.shouldShowPrompt())
|
||||
|
||||
// Test without being the default browser
|
||||
settings.apply {
|
||||
numberOfAppLaunches = 5
|
||||
isDefault = false
|
||||
lastReviewPromptTimeInMillis = 1595824882805L
|
||||
lastReviewPromptTimeInMillis = 0L
|
||||
}
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
assertFalse(controller.shouldShowPrompt())
|
||||
|
||||
// Test with number of app launches < 5
|
||||
@ -130,15 +185,23 @@ class ReviewPromptControllerTest {
|
||||
lastReviewPromptTimeInMillis = 0L
|
||||
}
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
assertFalse(controller.shouldShowPrompt())
|
||||
|
||||
// Test with last prompt less than 4 months ago
|
||||
settings.apply {
|
||||
numberOfAppLaunches = 5
|
||||
isDefault = true
|
||||
lastReviewPromptTimeInMillis = 1595824882905L
|
||||
lastReviewPromptTimeInMillis = LESS_THAN_4_MONTHS_FROM_TEST_TIME_NOW
|
||||
}
|
||||
|
||||
controller.reviewPromptIsReady = true
|
||||
assertFalse(controller.shouldShowPrompt())
|
||||
}
|
||||
|
||||
companion object {
|
||||
private const val TEST_TIME_NOW = 1598416882805L
|
||||
private const val MORE_THAN_4_MONTHS_FROM_TEST_TIME_NOW = 1588048882804L
|
||||
private const val LESS_THAN_4_MONTHS_FROM_TEST_TIME_NOW = 1595824882905L
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user