diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/PromptBatterySaverDialogFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/components/PromptBatterySaverDialogFragment.kt index b17590a679..df0ce51542 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/PromptBatterySaverDialogFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/PromptBatterySaverDialogFragment.kt @@ -10,6 +10,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.annotation.RequiresApi +import androidx.core.content.ContextCompat import androidx.core.os.bundleOf import androidx.fragment.app.FragmentManager import org.signal.core.util.concurrent.LifecycleDisposable @@ -17,6 +18,7 @@ import org.signal.core.util.logging.Log import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.databinding.PromptBatterySaverBottomSheetBinding import org.thoughtcrime.securesms.keyvalue.SignalStore +import org.thoughtcrime.securesms.notifications.DelayedNotificationConfig import org.thoughtcrime.securesms.util.BottomSheetUtil import org.thoughtcrime.securesms.util.LocalMetrics import org.thoughtcrime.securesms.util.PowerManagerCompat @@ -26,12 +28,15 @@ class PromptBatterySaverDialogFragment : FixedRoundedCornerBottomSheetDialogFrag companion object { private val TAG = Log.tag(PromptBatterySaverDialogFragment::class.java) + private const val ARG_LEARN_MORE_LINK = "arg.learn.more.link" @JvmStatic fun show(fragmentManager: FragmentManager) { if (fragmentManager.findFragmentByTag(BottomSheetUtil.STANDARD_BOTTOM_SHEET_FRAGMENT_TAG) == null) { PromptBatterySaverDialogFragment().apply { - arguments = bundleOf() + arguments = bundleOf( + ARG_LEARN_MORE_LINK to DelayedNotificationConfig.currentConfig.link + ) }.show(fragmentManager, BottomSheetUtil.STANDARD_BOTTOM_SHEET_FRAGMENT_TAG) SignalStore.uiHints.lastBatterySaverPrompt = System.currentTimeMillis() } @@ -52,6 +57,11 @@ class PromptBatterySaverDialogFragment : FixedRoundedCornerBottomSheetDialogFrag override fun onViewCreated(view: View, savedInstanceState: Bundle?) { disposables.bindTo(viewLifecycleOwner) + val learnMoreLink = arguments?.getString(ARG_LEARN_MORE_LINK) ?: getString(R.string.PromptBatterySaverBottomSheet__learn_more_url) + binding.message.setLearnMoreVisible(true) + binding.message.setLinkColor(ContextCompat.getColor(requireContext(), R.color.signal_colorPrimary)) + binding.message.setLink(learnMoreLink) + binding.continueButton.setOnClickListener { PowerManagerCompat.requestIgnoreBatteryOptimizations(requireContext()) Log.i(TAG, "Requested to ignore battery optimizations, clearing local metrics.") diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/notifications/NotificationsSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/notifications/NotificationsSettingsViewModel.kt index be4564f558..d384f1a40c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/notifications/NotificationsSettingsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/notifications/NotificationsSettingsViewModel.kt @@ -120,7 +120,7 @@ class NotificationsSettingsViewModel(private val sharedPreferences: SharedPrefer messagePrivacy = SignalStore.settings.messageNotificationsPrivacy.toString(), priority = TextSecurePreferences.getNotificationPriority(AppDependencies.application), troubleshootNotifications = if (calculateSlowNotifications) { - SlowNotificationHeuristics.isPotentiallyCausedByBatteryOptimizations() && SlowNotificationHeuristics.isHavingDelayedNotifications() + SlowNotificationHeuristics.isPotentiallyCausedByBatteryOptimizations() && (SlowNotificationHeuristics.isHavingDelayedNotifications() || SlowNotificationHeuristics.showPreemptively()) } else if (currentState != null) { currentState.messageNotificationsState.troubleshootNotifications } else { diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfig.kt new file mode 100644 index 0000000000..93101f466f --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfig.kt @@ -0,0 +1,62 @@ +package org.thoughtcrime.securesms.notifications + +import android.os.Build +import androidx.annotation.VisibleForTesting +import com.fasterxml.jackson.annotation.JsonProperty +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.util.JsonUtils +import org.thoughtcrime.securesms.util.RemoteConfig +import java.io.IOException + +/** + * Remote configs for a device to show a support screen in an effort to prevent delayed notifications + */ +object DelayedNotificationConfig { + + private val TAG = Log.tag(DelayedNotificationConfig::class.java) + private const val GENERAL_SUPPORT_URL = "https://support.signal.org/hc/articles/360007318711#android_notifications_troubleshooting" + + val currentConfig: Config by lazy { computeConfig() } + + /** + * Maps a device model to specific modifications set in order to support better notification + * @param model either exact device model name or model name that ends with a wildcard + * @param showPreemptively shows support sheet immediately if true or after a vitals failure if not, still dependent on localePercent + * @param link represents the Signal support url that corresponds to this device model + * @param localePercent represents the percent of people who will get this change per country + */ + data class Config( + @JsonProperty val model: String = "", + @JsonProperty val showPreemptively: Boolean = false, + @JsonProperty val link: String = GENERAL_SUPPORT_URL, + @JsonProperty val localePercent: String = RemoteConfig.promptBatterySaver + ) + + @VisibleForTesting + fun computeConfig(): Config { + val default = Config() + val serialized = RemoteConfig.promptDelayedNotificationConfig + if (serialized.isNullOrBlank()) { + return default + } + + val list: List = try { + JsonUtils.fromJsonArray(serialized, Config::class.java) + } catch (e: IOException) { + Log.w(TAG, "Failed to parse json!", e) + emptyList() + } + + val config: Config? = list + .filter { it.model.isNotEmpty() } + .find { + if (it.model.last() == '*') { + Build.MODEL.startsWith(it.model.substring(0, it.model.length - 1)) + } else { + it.model.contains(Build.MODEL) + } + } + + return config ?: default + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/SlowNotificationHeuristics.kt b/app/src/main/java/org/thoughtcrime/securesms/notifications/SlowNotificationHeuristics.kt index 3852790ce9..08ed78096e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/SlowNotificationHeuristics.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/SlowNotificationHeuristics.kt @@ -143,6 +143,10 @@ object SlowNotificationHeuristics { return true } + fun showPreemptively(): Boolean { + return DelayedNotificationConfig.currentConfig.showPreemptively + } + private fun hasRepeatedFailedServiceStarts(metrics: List, minimumEventAgeMs: Long, minimumEventCount: Int, failurePercentage: Float): Boolean { if (!haveEnoughData(SignalLocalMetrics.FcmServiceStartSuccess.NAME, minimumEventAgeMs) && !haveEnoughData(SignalLocalMetrics.FcmServiceStartFailure.NAME, minimumEventAgeMs)) { Log.d(TAG, "insufficient data for service starts") diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/VitalsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/notifications/VitalsViewModel.kt index f68dcad7da..40787459df 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/VitalsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/VitalsViewModel.kt @@ -46,7 +46,7 @@ class VitalsViewModel(private val context: Application) : AndroidViewModel(conte private fun checkHeuristics(): Single { return Single.fromCallable { var state = State.NONE - if (SlowNotificationHeuristics.isHavingDelayedNotifications()) { + if (SlowNotificationHeuristics.showPreemptively() || SlowNotificationHeuristics.isHavingDelayedNotifications()) { if (SlowNotificationHeuristics.isPotentiallyCausedByBatteryOptimizations() && SlowNotificationHeuristics.shouldPromptBatterySaver()) { state = State.PROMPT_BATTERY_SAVER_DIALOG } else if (SlowNotificationHeuristics.shouldPromptUserForLogs()) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/LocaleRemoteConfig.java b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleRemoteConfig.java index ed6cacd9cf..0c824edfb0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/LocaleRemoteConfig.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleRemoteConfig.java @@ -8,6 +8,7 @@ import com.google.i18n.phonenumbers.PhoneNumberUtil; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.mms.PushMediaConstraints; +import org.thoughtcrime.securesms.notifications.DelayedNotificationConfig; import org.thoughtcrime.securesms.recipients.Recipient; import java.util.Arrays; @@ -74,7 +75,7 @@ public final class LocaleRemoteConfig { } public static boolean isBatterySaverPromptEnabled() { - return RemoteConfig.internalUser() || isEnabledPartsPerMillion(RemoteConfig.PROMPT_BATTERY_SAVER, RemoteConfig.promptBatterySaver()); + return RemoteConfig.internalUser() || isEnabledPartsPerMillion(RemoteConfig.PROMPT_BATTERY_SAVER, DelayedNotificationConfig.INSTANCE.getCurrentConfig().getLocalePercent()); } /** diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/RemoteConfig.kt b/app/src/main/java/org/thoughtcrime/securesms/util/RemoteConfig.kt index a367be7d35..5e6ba4fa5e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/RemoteConfig.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/util/RemoteConfig.kt @@ -16,6 +16,9 @@ import org.thoughtcrime.securesms.jobs.RemoteConfigRefreshJob import org.thoughtcrime.securesms.jobs.Svr3MirrorJob import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.messageprocessingalarm.RoutineMessageFetchReceiver +import org.thoughtcrime.securesms.util.RemoteConfig.Config +import org.thoughtcrime.securesms.util.RemoteConfig.remoteBoolean +import org.thoughtcrime.securesms.util.RemoteConfig.remoteValue import java.io.IOException import java.util.TreeMap import kotlin.math.max @@ -873,6 +876,14 @@ object RemoteConfig { hotSwappable = true ) + private const val PROMPT_DELAYED_NOTIFICATION_CONFIG: String = "android.delayedNotificationConfig" + + val promptDelayedNotificationConfig: String by remoteString( + key = PROMPT_DELAYED_NOTIFICATION_CONFIG, + defaultValue = "", + hotSwappable = true + ) + const val CRASH_PROMPT_CONFIG: String = "android.crashPromptConfig" /** Config object for what crashes to prompt about. */ diff --git a/app/src/main/res/layout/prompt_battery_saver_bottom_sheet.xml b/app/src/main/res/layout/prompt_battery_saver_bottom_sheet.xml index 17fd1d195d..ba52f42d05 100644 --- a/app/src/main/res/layout/prompt_battery_saver_bottom_sheet.xml +++ b/app/src/main/res/layout/prompt_battery_saver_bottom_sheet.xml @@ -32,6 +32,7 @@ android:layout_gravity="center_horizontal" android:gravity="center" android:layout_marginTop="16dp" + android:layout_marginHorizontal="24dp" android:text="@string/PromptBatterySaverBottomSheet__title" /> @@ -42,7 +43,7 @@ android:layout_height="wrap_content" android:layout_gravity="center_horizontal" android:gravity="center" - android:layout_marginTop="12dp" + android:layout_marginTop="8dp" android:layout_marginHorizontal="24dp" android:text="@string/PromptBatterySaverBottomSheet__message" /> @@ -51,9 +52,9 @@ android:layout_width="match_parent" android:layout_height="wrap_content" android:orientation="horizontal" - android:minWidth="320dp" + android:minWidth="340dp" android:gravity="center_horizontal" - android:layout_marginHorizontal="34dp"> + android:layout_marginHorizontal="24dp"> + android:layout_marginEnd="12dp" + android:text="@string/PromptBatterySaverBottomSheet__no_thanks"/> diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a2ec6a2001..87f877702b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1098,7 +1098,8 @@ Continue - Dismiss + No thanks + https://support.signal.org/hc/articles/360007318711#android_notifications_troubleshooting Requests & invites diff --git a/app/src/test/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfigTest.kt b/app/src/test/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfigTest.kt new file mode 100644 index 0000000000..22ab2b2a9b --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/notifications/DelayedNotificationConfigTest.kt @@ -0,0 +1,100 @@ +package org.thoughtcrime.securesms.notifications + +import android.app.Application +import android.os.Build +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkObject +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.util.ReflectionHelpers +import org.thoughtcrime.securesms.assertIs +import org.thoughtcrime.securesms.util.RemoteConfig + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class DelayedNotificationConfigTest { + + @Before + fun setup() { + mockkObject(RemoteConfig) + } + + @After + fun tearDown() { + unmockkObject(RemoteConfig) + } + + @Test + fun `empty config`() { + every { RemoteConfig.promptDelayedNotificationConfig } returns "" + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config() + } + + @Test + fun `invalid config`() { + every { RemoteConfig.promptDelayedNotificationConfig } returns "bad" + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config() + } + + @Test + fun `simple device match`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "test") + every { RemoteConfig.promptDelayedNotificationConfig } returns """[ { "model": "test", "link": "test.com", "showPreemptively": true, "localePercent": "*:500000" } ]""" + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config(model = "test", link = "test.com", showPreemptively = true, localePercent = "*:500000") + } + + @Test + fun `complex device match`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "test-1") + every { RemoteConfig.promptDelayedNotificationConfig } returns + """ + [ + { "model": "test", "showPreemptively": false, "localePercent": "*:10000" }, + { "model": "test-1", "showPreemptively": true, "localePercent": "*:20000" }, + { "model": "test-11", "showPreemptively": false, "localePercent": "*:30000" }, + { "model": "test-11*", "showPreemptively": false, "localePercent": "*:40000" } + ] + """.trimMargin() + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config(model = "test-1", showPreemptively = true, localePercent = "*:20000") + } + + @Test + fun `simple wildcard device match`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "test1") + every { RemoteConfig.promptDelayedNotificationConfig } returns """[ { "model": "test*", "link": "test.com", "showPreemptively": true, "localePercent": "*:500000" } ]""" + DelayedNotificationConfig.currentConfig assertIs DelayedNotificationConfig.Config(model = "test*", link = "test.com", showPreemptively = true, localePercent = "*:500000") + } + + @Test + fun `complex wildcard device match`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "test-1") + every { RemoteConfig.promptDelayedNotificationConfig } returns + """ + [ + { "model": "*", "showPreemptively": false, "localePercent": "*:10000" }, + { "model": "test1", "showPreemptively": false, "localePercent": "*:20000" }, + { "model": "test-", "showPreemptively": false, "localePercent": "*:30000" } + ] + """.trimMargin() + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config(model = "*", showPreemptively = false, localePercent = "*:10000") + } + + @Test + fun `no device match`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "bad") + every { RemoteConfig.promptDelayedNotificationConfig } returns """[ { "model": "test", "link": "test.com", "showPreemptively": true, "localePercent": "*:500000" } ]""" + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config() + } + + @Test + fun `default fields is zero percent`() { + ReflectionHelpers.setStaticField(Build::class.java, "MODEL", "test") + every { RemoteConfig.promptDelayedNotificationConfig } returns """[ { "model": "test" } ]""" + DelayedNotificationConfig.computeConfig() assertIs DelayedNotificationConfig.Config(model = "test", showPreemptively = false, localePercent = "*") + } +}