From 83072451202f78e6d90e967adaa127dc4118b250 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 11 Jan 2021 18:31:57 +0100 Subject: [PATCH 01/14] Fix issue with delay set to 0 --- .../sdk/internal/session/sync/job/SyncService.kt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index 9d854229df..16c4095654 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -166,15 +166,17 @@ abstract class SyncService : Service() { } if (throwable is Failure.NetworkConnection) { // Timeout is not critical, so retry as soon as possible. - val retryDelay = if (isInitialSync || throwable.cause is SocketTimeoutException) { - 0 - } else { - syncDelaySeconds + if (isInitialSync || throwable.cause is SocketTimeoutException) { + // For big accounts, computing init sync response can take time, but Synapse will cache the + // result for the next request. So keep retrying in loop + Timber.w("Timeout during initial sync, retry in loop") + doSync() + return } // Network might be off, no need to reschedule endless alarms :/ preventReschedule = true // Instead start a work to restart background sync when network is on - onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, retryDelay) + onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, syncDelaySeconds) } // JobCancellation could be caught here when onDestroy cancels the coroutine context if (isRunning.get()) stopMe() From ec0a04e89305004a1f321849b45c46224f7b4fe6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 10:45:18 +0100 Subject: [PATCH 02/14] timeout -> restart without delay --- .../android/sdk/internal/session/sync/job/SyncService.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index 16c4095654..a535a322f4 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -166,10 +166,10 @@ abstract class SyncService : Service() { } if (throwable is Failure.NetworkConnection) { // Timeout is not critical, so retry as soon as possible. - if (isInitialSync || throwable.cause is SocketTimeoutException) { - // For big accounts, computing init sync response can take time, but Synapse will cache the + if (throwable.cause is SocketTimeoutException) { + // For big accounts, computing sync response can take time, but Synapse will cache the // result for the next request. So keep retrying in loop - Timber.w("Timeout during initial sync, retry in loop") + Timber.w("Timeout during sync, retry in loop") doSync() return } From b69d8ad71a400c8abe863b55445a94b06137082e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 11:13:38 +0100 Subject: [PATCH 03/14] Code cleanup, fix crash if `appContext !is HasVectorInjector` --- .../receiver/AlarmSyncBroadcastReceiver.kt | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt b/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt index 674e7dfef5..1b5c71d952 100644 --- a/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt +++ b/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt @@ -26,28 +26,20 @@ import androidx.core.content.ContextCompat import androidx.core.content.getSystemService import im.vector.app.core.di.HasVectorInjector import im.vector.app.core.services.VectorSyncService -import im.vector.app.features.settings.VectorPreferences import org.matrix.android.sdk.internal.session.sync.job.SyncService import timber.log.Timber class AlarmSyncBroadcastReceiver : BroadcastReceiver() { - lateinit var vectorPreferences: VectorPreferences - override fun onReceive(context: Context, intent: Intent) { - val appContext = context.applicationContext - if (appContext is HasVectorInjector) { - val activeSession = appContext.injector().activeSessionHolder().getSafeActiveSession() - if (activeSession == null) { - Timber.v("No active session don't launch sync service.") - return - } - vectorPreferences = appContext.injector().vectorPreferences() - } + Timber.d("## Sync: AlarmSyncBroadcastReceiver received intent") + val vectorPreferences = (context.applicationContext as? HasVectorInjector) + ?.injector() + ?.takeIf { it.activeSessionHolder().getSafeActiveSession() != null } + ?.vectorPreferences() + ?: return Unit.also { Timber.v("No active session, so don't launch sync service.") } val sessionId = intent.getStringExtra(SyncService.EXTRA_SESSION_ID) ?: return - // This method is called when the BroadcastReceiver is receiving an Intent broadcast. - Timber.d("RestartBroadcastReceiver received intent") VectorSyncService.newPeriodicIntent(context, sessionId, vectorPreferences.backgroundSyncTimeOut(), vectorPreferences.backgroundSyncDelay()).let { try { ContextCompat.startForegroundService(context, it) From 561b89830a842bda22cd0078d64e7355283cdce2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 11:21:22 +0100 Subject: [PATCH 04/14] Avoid default value for param --- .../im/vector/app/core/services/VectorSyncService.kt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 0950bdf121..36f7497b7d 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -89,11 +89,11 @@ class VectorSyncService : SyncService() { } override fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { - rescheduleSyncService(sessionId, timeout, delay) + rescheduleSyncService(sessionId, timeout, delay, false) } override fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { - Timber.d("## Sync: A network error occured during sync") + Timber.d("## Sync: A network error occurred during sync") val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() .setInputData(Data.Builder() @@ -137,8 +137,11 @@ class VectorSyncService : SyncService() { } } -private fun Context.rescheduleSyncService(sessionId: String, timeout: Int, delay: Int, networkBack: Boolean = false) { - val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, networkBack) +private fun Context.rescheduleSyncService(sessionId: String, + timeout: Int, + delay: Int, + isNetworkBack: Boolean) { + val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, isNetworkBack) val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { PendingIntent.getForegroundService(this, 0, periodicIntent, 0) } else { From d46ae83dc49fcfbee07bc12790a50ecd53c32a05 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 11:41:20 +0100 Subject: [PATCH 05/14] Define constant for keys --- .../app/core/services/VectorSyncService.kt | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 36f7497b7d..ce1e8d136d 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -96,12 +96,7 @@ class VectorSyncService : SyncService() { Timber.d("## Sync: A network error occurred during sync") val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() - .setInputData(Data.Builder() - .putString("sessionId", sessionId) - .putInt("timeout", timeout) - .putInt("delay", delay) - .build() - ) + .setInputData(RestartWhenNetworkOn.createInputData(sessionId, timeout, delay)) .setConstraints(Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) .build() @@ -124,16 +119,32 @@ class VectorSyncService : SyncService() { notificationManager.cancel(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE) } + // I do not move or rename this class, since I'm not sure about the side effect regarding the WorkManager class RestartWhenNetworkOn(appContext: Context, workerParams: WorkerParameters) : Worker(appContext, workerParams) { override fun doWork(): Result { - val sessionId = inputData.getString("sessionId") ?: return Result.failure() - val timeout = inputData.getInt("timeout", 6) - val delay = inputData.getInt("delay", 60) + Timber.d("## Sync: RestartWhenNetworkOn.doWork()") + val sessionId = inputData.getString(KEY_SESSION_ID) ?: return Result.failure() + val timeout = inputData.getInt(KEY_TIMEOUT, 6) + val delay = inputData.getInt(KEY_DELAY, 60) applicationContext.rescheduleSyncService(sessionId, timeout, delay, true) // Indicate whether the work finished successfully with the Result return Result.success() } + + companion object { + fun createInputData(sessionId: String, timeout: Int, delay: Int): Data { + return Data.Builder() + .putString(KEY_SESSION_ID, sessionId) + .putInt(KEY_TIMEOUT, timeout) + .putInt(KEY_DELAY, delay) + .build() + } + + private const val KEY_SESSION_ID = "sessionId" + private const val KEY_TIMEOUT = "timeout" + private const val KEY_DELAY = "delay" + } } } From aac3f379a782a45aa3294e88b562367adbb95739 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 11:42:39 +0100 Subject: [PATCH 06/14] Add a log --- .../matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt index c6647f7572..b58cab99b5 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt @@ -20,6 +20,7 @@ import android.content.Context import androidx.work.ListenableWorker import androidx.work.WorkerFactory import androidx.work.WorkerParameters +import timber.log.Timber import javax.inject.Inject import javax.inject.Provider @@ -32,6 +33,8 @@ class MatrixWorkerFactory @Inject constructor( workerClassName: String, workerParameters: WorkerParameters ): ListenableWorker? { + Timber.d("MatrixWorkerFactory.createWorker for $workerClassName") + val foundEntry = workerFactories.entries.find { Class.forName(workerClassName).isAssignableFrom(it.key) } val factoryProvider = foundEntry?.value From 609ceb7fa421b8d2189e5df9e83524a648c5d63a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 12:06:52 +0100 Subject: [PATCH 07/14] Avoid Magic numbers --- .../sdk/internal/session/sync/job/SyncService.kt | 13 +++++++++---- .../vector/app/core/services/VectorSyncService.kt | 10 ++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index a535a322f4..c79b25fb93 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -50,8 +50,9 @@ abstract class SyncService : Service() { private var sessionId: String? = null private var mIsSelfDestroyed: Boolean = false - private var syncTimeoutSeconds: Int = 6 - private var syncDelaySeconds: Int = 60 + private var syncTimeoutSeconds: Int = getDefaultSyncTimeoutSeconds() + private var syncDelaySeconds: Int = getDefaultSyncDelaySeconds() + private var periodic: Boolean = false private var preventReschedule: Boolean = false @@ -190,8 +191,8 @@ abstract class SyncService : Service() { } val matrix = Matrix.getInstance(applicationContext) val safeSessionId = intent.getStringExtra(EXTRA_SESSION_ID) ?: return false - syncTimeoutSeconds = intent.getIntExtra(EXTRA_TIMEOUT_SECONDS, 6) - syncDelaySeconds = intent.getIntExtra(EXTRA_DELAY_SECONDS, 60) + syncTimeoutSeconds = intent.getIntExtra(EXTRA_TIMEOUT_SECONDS, getDefaultSyncTimeoutSeconds()) + syncDelaySeconds = intent.getIntExtra(EXTRA_DELAY_SECONDS, getDefaultSyncDelaySeconds()) try { val sessionComponent = matrix.sessionManager.getSessionComponent(safeSessionId) ?: throw IllegalStateException("## Sync: You should have a session to make it work") @@ -210,6 +211,10 @@ abstract class SyncService : Service() { } } + abstract fun getDefaultSyncTimeoutSeconds(): Int + + abstract fun getDefaultSyncDelaySeconds(): Int + abstract fun onStart(isInitialSync: Boolean) abstract fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index ce1e8d136d..dbf51b3c99 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -33,6 +33,7 @@ import androidx.work.WorkerParameters import im.vector.app.R import im.vector.app.core.extensions.vectorComponent import im.vector.app.features.notifications.NotificationUtils +import im.vector.app.features.settings.BackgroundSyncMode import org.matrix.android.sdk.internal.session.sync.job.SyncService import timber.log.Timber @@ -78,6 +79,10 @@ class VectorSyncService : SyncService() { notificationUtils = vectorComponent().notificationUtils() } + override fun getDefaultSyncDelaySeconds() = BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS + + override fun getDefaultSyncTimeoutSeconds() = BackgroundSyncMode.DEFAULT_SYNC_TIMEOUT_SECONDS + override fun onStart(isInitialSync: Boolean) { val notificationSubtitleRes = if (isInitialSync) { R.string.notification_initial_sync @@ -125,8 +130,8 @@ class VectorSyncService : SyncService() { override fun doWork(): Result { Timber.d("## Sync: RestartWhenNetworkOn.doWork()") val sessionId = inputData.getString(KEY_SESSION_ID) ?: return Result.failure() - val timeout = inputData.getInt(KEY_TIMEOUT, 6) - val delay = inputData.getInt(KEY_DELAY, 60) + val timeout = inputData.getInt(KEY_TIMEOUT, BackgroundSyncMode.DEFAULT_SYNC_TIMEOUT_SECONDS) + val delay = inputData.getInt(KEY_DELAY, BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS) applicationContext.rescheduleSyncService(sessionId, timeout, delay, true) // Indicate whether the work finished successfully with the Result return Result.success() @@ -152,6 +157,7 @@ private fun Context.rescheduleSyncService(sessionId: String, timeout: Int, delay: Int, isNetworkBack: Boolean) { + Timber.d("## Sync: rescheduleSyncService") val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, isNetworkBack) val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { PendingIntent.getForegroundService(this, 0, periodicIntent, 0) From b20bbc12957b5e70217a1be3a6d568f16945d5f2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 13:56:06 +0100 Subject: [PATCH 08/14] When network is back, do an immediate sync --- .../app/core/services/VectorSyncService.kt | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index dbf51b3c99..2f21d53177 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -159,16 +159,22 @@ private fun Context.rescheduleSyncService(sessionId: String, isNetworkBack: Boolean) { Timber.d("## Sync: rescheduleSyncService") val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, isNetworkBack) - val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - PendingIntent.getForegroundService(this, 0, periodicIntent, 0) + + if (isNetworkBack || delay == 0) { + // Do not wait, do the sync now (more reactivity if network back is due to user action) + startService(periodicIntent) } else { - PendingIntent.getService(this, 0, periodicIntent, 0) - } - val firstMillis = System.currentTimeMillis() + delay * 1000L - val alarmMgr = getSystemService()!! - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) - } else { - alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) + val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + PendingIntent.getForegroundService(this, 0, periodicIntent, 0) + } else { + PendingIntent.getService(this, 0, periodicIntent, 0) + } + val firstMillis = System.currentTimeMillis() + delay * 1000L + val alarmMgr = getSystemService()!! + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) + } else { + alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) + } } } From e771b21ea3c0578dfd3cfc06e597d7b98953800a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 15:21:16 +0100 Subject: [PATCH 09/14] Rename parameters for clarity --- .../receiver/AlarmSyncBroadcastReceiver.kt | 25 +++++++----- .../im/vector/app/core/extensions/Session.kt | 21 ++++++---- .../app/core/services/VectorSyncService.kt | 40 +++++++++++-------- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt b/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt index 1b5c71d952..b94e99208b 100644 --- a/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt +++ b/vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt @@ -40,15 +40,22 @@ class AlarmSyncBroadcastReceiver : BroadcastReceiver() { ?: return Unit.also { Timber.v("No active session, so don't launch sync service.") } val sessionId = intent.getStringExtra(SyncService.EXTRA_SESSION_ID) ?: return - VectorSyncService.newPeriodicIntent(context, sessionId, vectorPreferences.backgroundSyncTimeOut(), vectorPreferences.backgroundSyncDelay()).let { - try { - ContextCompat.startForegroundService(context, it) - } catch (ex: Throwable) { - Timber.i("## Sync: Failed to start service, Alarm scheduled to restart service") - scheduleAlarm(context, sessionId, vectorPreferences.backgroundSyncDelay()) - Timber.e(ex) - } - } + VectorSyncService.newPeriodicIntent( + context = context, + sessionId = sessionId, + syncTimeoutSeconds = vectorPreferences.backgroundSyncTimeOut(), + syncDelaySeconds = vectorPreferences.backgroundSyncDelay(), + isNetworkBack = false + ) + .let { + try { + ContextCompat.startForegroundService(context, it) + } catch (ex: Throwable) { + Timber.i("## Sync: Failed to start service, Alarm scheduled to restart service") + scheduleAlarm(context, sessionId, vectorPreferences.backgroundSyncDelay()) + Timber.e(ex) + } + } } companion object { diff --git a/vector/src/main/java/im/vector/app/core/extensions/Session.kt b/vector/src/main/java/im/vector/app/core/extensions/Session.kt index cb87947612..b8cdd83324 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/Session.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/Session.kt @@ -38,14 +38,19 @@ fun Session.startSyncing(context: Context) { val applicationContext = context.applicationContext if (!hasAlreadySynced()) { // initial sync is done as a service so it can continue below app lifecycle - VectorSyncService.newOneShotIntent(applicationContext, sessionId, 0).also { - try { - ContextCompat.startForegroundService(applicationContext, it) - } catch (ex: Throwable) { - // TODO - Timber.e(ex) - } - } + VectorSyncService.newOneShotIntent( + context = applicationContext, + sessionId = sessionId, + syncTimeoutSeconds = 0 + ) + .let { + try { + ContextCompat.startForegroundService(applicationContext, it) + } catch (ex: Throwable) { + // TODO + Timber.e(ex) + } + } } else { val isAtLeastStarted = ProcessLifecycleOwner.get().lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED) Timber.v("--> is at least started? $isAtLeastStarted") diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 2f21d53177..c9b72708f8 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -41,27 +41,27 @@ class VectorSyncService : SyncService() { companion object { - fun newOneShotIntent(context: Context, sessionId: String, timeoutSeconds: Int): Intent { + fun newOneShotIntent(context: Context, + sessionId: String, + syncTimeoutSeconds: Int): Intent { return Intent(context, VectorSyncService::class.java).also { it.putExtra(EXTRA_SESSION_ID, sessionId) - it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds) + it.putExtra(EXTRA_TIMEOUT_SECONDS, syncTimeoutSeconds) it.putExtra(EXTRA_PERIODIC, false) } } - fun newPeriodicIntent( - context: Context, - sessionId: String, - timeoutSeconds: Int, - delayInSeconds: Int, - networkBack: Boolean = false - ): Intent { + fun newPeriodicIntent(context: Context, + sessionId: String, + syncTimeoutSeconds: Int, + syncDelaySeconds: Int, + isNetworkBack: Boolean): Intent { return Intent(context, VectorSyncService::class.java).also { it.putExtra(EXTRA_SESSION_ID, sessionId) - it.putExtra(EXTRA_TIMEOUT_SECONDS, timeoutSeconds) + it.putExtra(EXTRA_TIMEOUT_SECONDS, syncTimeoutSeconds) it.putExtra(EXTRA_PERIODIC, true) - it.putExtra(EXTRA_DELAY_SECONDS, delayInSeconds) - it.putExtra(EXTRA_NETWORK_BACK_RESTART, networkBack) + it.putExtra(EXTRA_DELAY_SECONDS, syncDelaySeconds) + it.putExtra(EXTRA_NETWORK_BACK_RESTART, isNetworkBack) } } @@ -154,13 +154,19 @@ class VectorSyncService : SyncService() { } private fun Context.rescheduleSyncService(sessionId: String, - timeout: Int, - delay: Int, + syncTimeoutSeconds: Int, + syncDelaySeconds: Int, isNetworkBack: Boolean) { Timber.d("## Sync: rescheduleSyncService") - val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, isNetworkBack) + val periodicIntent = VectorSyncService.newPeriodicIntent( + context = this, + sessionId = sessionId, + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds, + isNetworkBack = isNetworkBack + ) - if (isNetworkBack || delay == 0) { + if (isNetworkBack || syncDelaySeconds == 0) { // Do not wait, do the sync now (more reactivity if network back is due to user action) startService(periodicIntent) } else { @@ -169,7 +175,7 @@ private fun Context.rescheduleSyncService(sessionId: String, } else { PendingIntent.getService(this, 0, periodicIntent, 0) } - val firstMillis = System.currentTimeMillis() + delay * 1000L + val firstMillis = System.currentTimeMillis() + syncDelaySeconds * 1000L val alarmMgr = getSystemService()!! if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent) From 2b60affd9a6ab7a27b34b1d51eb0323df32b4a6b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 15:23:14 +0100 Subject: [PATCH 10/14] Remove useless param --- .../src/main/java/im/vector/app/core/extensions/Session.kt | 3 +-- .../java/im/vector/app/core/services/VectorSyncService.kt | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/extensions/Session.kt b/vector/src/main/java/im/vector/app/core/extensions/Session.kt index b8cdd83324..10dade70ea 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/Session.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/Session.kt @@ -40,8 +40,7 @@ fun Session.startSyncing(context: Context) { // initial sync is done as a service so it can continue below app lifecycle VectorSyncService.newOneShotIntent( context = applicationContext, - sessionId = sessionId, - syncTimeoutSeconds = 0 + sessionId = sessionId ) .let { try { diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index c9b72708f8..65b81be384 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -42,11 +42,10 @@ class VectorSyncService : SyncService() { companion object { fun newOneShotIntent(context: Context, - sessionId: String, - syncTimeoutSeconds: Int): Intent { + sessionId: String): Intent { return Intent(context, VectorSyncService::class.java).also { it.putExtra(EXTRA_SESSION_ID, sessionId) - it.putExtra(EXTRA_TIMEOUT_SECONDS, syncTimeoutSeconds) + it.putExtra(EXTRA_TIMEOUT_SECONDS, 0) it.putExtra(EXTRA_PERIODIC, false) } } From b2df107f1777dd6d5643742699733b92fab8689f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 16:11:22 +0100 Subject: [PATCH 11/14] Remove unused params --- .../android/sdk/internal/session/sync/job/SyncService.kt | 8 ++++---- .../java/im/vector/app/core/services/VectorSyncService.kt | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index c79b25fb93..01ab47a3b8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -120,7 +120,7 @@ abstract class SyncService : Service() { serviceScope.coroutineContext.cancelChildren() if (!preventReschedule && periodic && sessionId != null && backgroundDetectionObserver.isInBackground) { Timber.d("## Sync: Reschedule service in $syncDelaySeconds sec") - onRescheduleAsked(sessionId ?: "", false, syncTimeoutSeconds, syncDelaySeconds) + onRescheduleAsked(sessionId ?: "", syncTimeoutSeconds, syncDelaySeconds) } super.onDestroy() } @@ -177,7 +177,7 @@ abstract class SyncService : Service() { // Network might be off, no need to reschedule endless alarms :/ preventReschedule = true // Instead start a work to restart background sync when network is on - onNetworkError(sessionId ?: "", isInitialSync, syncTimeoutSeconds, syncDelaySeconds) + onNetworkError(sessionId ?: "", syncTimeoutSeconds, syncDelaySeconds) } // JobCancellation could be caught here when onDestroy cancels the coroutine context if (isRunning.get()) stopMe() @@ -217,9 +217,9 @@ abstract class SyncService : Service() { abstract fun onStart(isInitialSync: Boolean) - abstract fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) + abstract fun onRescheduleAsked(sessionId: String, timeout: Int, delay: Int) - abstract fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) + abstract fun onNetworkError(sessionId: String, timeout: Int, delay: Int) override fun onBind(intent: Intent?): IBinder? { return null diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 65b81be384..5b732a5e3c 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -92,11 +92,11 @@ class VectorSyncService : SyncService() { startForeground(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE, notification) } - override fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { + override fun onRescheduleAsked(sessionId: String, timeout: Int, delay: Int) { rescheduleSyncService(sessionId, timeout, delay, false) } - override fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) { + override fun onNetworkError(sessionId: String, timeout: Int, delay: Int) { Timber.d("## Sync: A network error occurred during sync") val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() From 50ba13135039327d262081c6f8999fc67dc028ee Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 16:18:29 +0100 Subject: [PATCH 12/14] More renaming --- .../sdk/internal/session/sync/job/SyncService.kt | 16 ++++++++++++---- .../app/core/services/VectorSyncService.kt | 12 ++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index 01ab47a3b8..f3c2235dc9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -120,7 +120,11 @@ abstract class SyncService : Service() { serviceScope.coroutineContext.cancelChildren() if (!preventReschedule && periodic && sessionId != null && backgroundDetectionObserver.isInBackground) { Timber.d("## Sync: Reschedule service in $syncDelaySeconds sec") - onRescheduleAsked(sessionId ?: "", syncTimeoutSeconds, syncDelaySeconds) + onRescheduleAsked( + sessionId = sessionId ?: "", + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds + ) } super.onDestroy() } @@ -177,7 +181,11 @@ abstract class SyncService : Service() { // Network might be off, no need to reschedule endless alarms :/ preventReschedule = true // Instead start a work to restart background sync when network is on - onNetworkError(sessionId ?: "", syncTimeoutSeconds, syncDelaySeconds) + onNetworkError( + sessionId = sessionId ?: "", + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds + ) } // JobCancellation could be caught here when onDestroy cancels the coroutine context if (isRunning.get()) stopMe() @@ -217,9 +225,9 @@ abstract class SyncService : Service() { abstract fun onStart(isInitialSync: Boolean) - abstract fun onRescheduleAsked(sessionId: String, timeout: Int, delay: Int) + abstract fun onRescheduleAsked(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int) - abstract fun onNetworkError(sessionId: String, timeout: Int, delay: Int) + abstract fun onNetworkError(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int) override fun onBind(intent: Intent?): IBinder? { return null diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 5b732a5e3c..3f096376ff 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -92,15 +92,19 @@ class VectorSyncService : SyncService() { startForeground(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE, notification) } - override fun onRescheduleAsked(sessionId: String, timeout: Int, delay: Int) { - rescheduleSyncService(sessionId, timeout, delay, false) + override fun onRescheduleAsked(sessionId: String, + syncTimeoutSeconds: Int, + syncDelaySeconds: Int) { + rescheduleSyncService(sessionId, syncTimeoutSeconds, syncDelaySeconds, false) } - override fun onNetworkError(sessionId: String, timeout: Int, delay: Int) { + override fun onNetworkError(sessionId: String, + syncTimeoutSeconds: Int, + syncDelaySeconds: Int) { Timber.d("## Sync: A network error occurred during sync") val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() - .setInputData(RestartWhenNetworkOn.createInputData(sessionId, timeout, delay)) + .setInputData(RestartWhenNetworkOn.createInputData(sessionId, syncTimeoutSeconds, syncDelaySeconds)) .setConstraints(Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) .build() From 2ea45185d41ff3b61db39a52163241c23b11415e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 16:26:52 +0100 Subject: [PATCH 13/14] Ensure the service is restarted with the correct intent --- .../internal/session/sync/job/SyncService.kt | 5 +- .../app/core/services/VectorSyncService.kt | 58 ++++++++++++++----- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt index f3c2235dc9..cce169c246 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/job/SyncService.kt @@ -184,7 +184,8 @@ abstract class SyncService : Service() { onNetworkError( sessionId = sessionId ?: "", syncTimeoutSeconds = syncTimeoutSeconds, - syncDelaySeconds = syncDelaySeconds + syncDelaySeconds = syncDelaySeconds, + isPeriodic = periodic ) } // JobCancellation could be caught here when onDestroy cancels the coroutine context @@ -227,7 +228,7 @@ abstract class SyncService : Service() { abstract fun onRescheduleAsked(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int) - abstract fun onNetworkError(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int) + abstract fun onNetworkError(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int, isPeriodic: Boolean) override fun onBind(intent: Intent?): IBinder? { return null diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 3f096376ff..4b5c470ad2 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -95,16 +95,23 @@ class VectorSyncService : SyncService() { override fun onRescheduleAsked(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int) { - rescheduleSyncService(sessionId, syncTimeoutSeconds, syncDelaySeconds, false) + rescheduleSyncService( + sessionId = sessionId, + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds, + isPeriodic = true, + isNetworkBack = false + ) } override fun onNetworkError(sessionId: String, syncTimeoutSeconds: Int, - syncDelaySeconds: Int) { + syncDelaySeconds: Int, + isPeriodic: Boolean) { Timber.d("## Sync: A network error occurred during sync") val rescheduleSyncWorkRequest: WorkRequest = OneTimeWorkRequestBuilder() - .setInputData(RestartWhenNetworkOn.createInputData(sessionId, syncTimeoutSeconds, syncDelaySeconds)) + .setInputData(RestartWhenNetworkOn.createInputData(sessionId, syncTimeoutSeconds, syncDelaySeconds, isPeriodic)) .setConstraints(Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) .build() @@ -135,23 +142,36 @@ class VectorSyncService : SyncService() { val sessionId = inputData.getString(KEY_SESSION_ID) ?: return Result.failure() val timeout = inputData.getInt(KEY_TIMEOUT, BackgroundSyncMode.DEFAULT_SYNC_TIMEOUT_SECONDS) val delay = inputData.getInt(KEY_DELAY, BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS) - applicationContext.rescheduleSyncService(sessionId, timeout, delay, true) + val isPeriodic = inputData.getBoolean(KEY_IS_PERIODIC, false) + applicationContext.rescheduleSyncService( + sessionId = sessionId, + syncTimeoutSeconds = timeout, + syncDelaySeconds = delay, + isPeriodic = isPeriodic, + isNetworkBack = true + ) // Indicate whether the work finished successfully with the Result return Result.success() } companion object { - fun createInputData(sessionId: String, timeout: Int, delay: Int): Data { + fun createInputData(sessionId: String, + timeout: Int, + delay: Int, + isPeriodic: Boolean + ): Data { return Data.Builder() .putString(KEY_SESSION_ID, sessionId) .putInt(KEY_TIMEOUT, timeout) .putInt(KEY_DELAY, delay) + .putBoolean(KEY_IS_PERIODIC, isPeriodic) .build() } private const val KEY_SESSION_ID = "sessionId" private const val KEY_TIMEOUT = "timeout" private const val KEY_DELAY = "delay" + private const val KEY_IS_PERIODIC = "isPeriodic" } } } @@ -159,24 +179,32 @@ class VectorSyncService : SyncService() { private fun Context.rescheduleSyncService(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int, + isPeriodic: Boolean, isNetworkBack: Boolean) { Timber.d("## Sync: rescheduleSyncService") - val periodicIntent = VectorSyncService.newPeriodicIntent( - context = this, - sessionId = sessionId, - syncTimeoutSeconds = syncTimeoutSeconds, - syncDelaySeconds = syncDelaySeconds, - isNetworkBack = isNetworkBack - ) + val intent = if (isPeriodic) { + VectorSyncService.newPeriodicIntent( + context = this, + sessionId = sessionId, + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds, + isNetworkBack = isNetworkBack + ) + } else { + VectorSyncService.newOneShotIntent( + context = this, + sessionId = sessionId + ) + } if (isNetworkBack || syncDelaySeconds == 0) { // Do not wait, do the sync now (more reactivity if network back is due to user action) - startService(periodicIntent) + startService(intent) } else { val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - PendingIntent.getForegroundService(this, 0, periodicIntent, 0) + PendingIntent.getForegroundService(this, 0, intent, 0) } else { - PendingIntent.getService(this, 0, periodicIntent, 0) + PendingIntent.getService(this, 0, intent, 0) } val firstMillis = System.currentTimeMillis() + syncDelaySeconds * 1000L val alarmMgr = getSystemService()!! From d4c8f56c6e91a7697d992ef4a813116dad817b78 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 12 Jan 2021 16:29:05 +0100 Subject: [PATCH 14/14] More renaming --- .../app/core/services/VectorSyncService.kt | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt index 4b5c470ad2..2a00e94976 100644 --- a/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt +++ b/vector/src/main/java/im/vector/app/core/services/VectorSyncService.kt @@ -140,13 +140,13 @@ class VectorSyncService : SyncService() { override fun doWork(): Result { Timber.d("## Sync: RestartWhenNetworkOn.doWork()") val sessionId = inputData.getString(KEY_SESSION_ID) ?: return Result.failure() - val timeout = inputData.getInt(KEY_TIMEOUT, BackgroundSyncMode.DEFAULT_SYNC_TIMEOUT_SECONDS) - val delay = inputData.getInt(KEY_DELAY, BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS) + val syncTimeoutSeconds = inputData.getInt(KEY_SYNC_TIMEOUT_SECONDS, BackgroundSyncMode.DEFAULT_SYNC_TIMEOUT_SECONDS) + val syncDelaySeconds = inputData.getInt(KEY_SYNC_DELAY_SECONDS, BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS) val isPeriodic = inputData.getBoolean(KEY_IS_PERIODIC, false) applicationContext.rescheduleSyncService( sessionId = sessionId, - syncTimeoutSeconds = timeout, - syncDelaySeconds = delay, + syncTimeoutSeconds = syncTimeoutSeconds, + syncDelaySeconds = syncDelaySeconds, isPeriodic = isPeriodic, isNetworkBack = true ) @@ -156,21 +156,21 @@ class VectorSyncService : SyncService() { companion object { fun createInputData(sessionId: String, - timeout: Int, - delay: Int, + syncTimeoutSeconds: Int, + syncDelaySeconds: Int, isPeriodic: Boolean ): Data { return Data.Builder() .putString(KEY_SESSION_ID, sessionId) - .putInt(KEY_TIMEOUT, timeout) - .putInt(KEY_DELAY, delay) + .putInt(KEY_SYNC_TIMEOUT_SECONDS, syncTimeoutSeconds) + .putInt(KEY_SYNC_DELAY_SECONDS, syncDelaySeconds) .putBoolean(KEY_IS_PERIODIC, isPeriodic) .build() } private const val KEY_SESSION_ID = "sessionId" - private const val KEY_TIMEOUT = "timeout" - private const val KEY_DELAY = "delay" + private const val KEY_SYNC_TIMEOUT_SECONDS = "timeout" + private const val KEY_SYNC_DELAY_SECONDS = "delay" private const val KEY_IS_PERIODIC = "isPeriodic" } }