Merge pull request #2659 from vector-im/feature/bma/fix_init_sync

Fix issue with delay set to 0

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
 
@@ -119,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 ?: "", false, syncTimeoutSeconds, syncDelaySeconds)
+            onRescheduleAsked(
+                    sessionId = sessionId ?: "",
+                    syncTimeoutSeconds = syncTimeoutSeconds,
+                    syncDelaySeconds = syncDelaySeconds
+            )
         }
         super.onDestroy()
     }
@@ -166,15 +171,22 @@ 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 (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 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 = sessionId ?: "",
+                        syncTimeoutSeconds = syncTimeoutSeconds,
+                        syncDelaySeconds = syncDelaySeconds,
+                        isPeriodic = periodic
+                )
             }
             // JobCancellation could be caught here when onDestroy cancels the coroutine context
             if (isRunning.get()) stopMe()
@@ -188,8 +200,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")
@@ -208,11 +220,15 @@ 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)
+    abstract fun onRescheduleAsked(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int)
 
-    abstract fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int)
+    abstract fun onNetworkError(sessionId: String, syncTimeoutSeconds: Int, syncDelaySeconds: Int, isPeriodic: Boolean)
 
     override fun onBind(intent: Intent?): IBinder? {
         return null

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

vector/src/fdroid/java/im/vector/app/fdroid/receiver/AlarmSyncBroadcastReceiver.kt

@@ -26,29 +26,28 @@ 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 {
+        VectorSyncService.newPeriodicIntent(
+                context = context,
+                sessionId = sessionId,
+                syncTimeoutSeconds = vectorPreferences.backgroundSyncTimeOut(),
+                syncDelaySeconds = vectorPreferences.backgroundSyncDelay(),
+                isNetworkBack = false
+        )
+                .let {
                     try {
                         ContextCompat.startForegroundService(context, it)
                     } catch (ex: Throwable) {

vector/src/main/java/im/vector/app/core/extensions/Session.kt

@@ -38,7 +38,11 @@ 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 {
+        VectorSyncService.newOneShotIntent(
+                context = applicationContext,
+                sessionId = sessionId
+        )
+                .let {
                     try {
                         ContextCompat.startForegroundService(applicationContext, it)
                     } catch (ex: Throwable) {

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
 
@@ -40,27 +41,26 @@ class VectorSyncService : SyncService() {
 
     companion object {
 
-        fun newOneShotIntent(context: Context, sessionId: String, timeoutSeconds: Int): Intent {
+        fun newOneShotIntent(context: Context,
+                             sessionId: String): 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, 0)
                 it.putExtra(EXTRA_PERIODIC, false)
             }
         }
 
-        fun newPeriodicIntent(
-                context: Context,
+        fun newPeriodicIntent(context: Context,
                               sessionId: String,
-                timeoutSeconds: Int,
-                delayInSeconds: Int,
-                networkBack: Boolean = false
-        ): Intent {
+                              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)
             }
         }
 
@@ -78,6 +78,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
@@ -88,20 +92,26 @@ class VectorSyncService : SyncService() {
         startForeground(NotificationUtils.NOTIFICATION_ID_FOREGROUND_SERVICE, notification)
     }
 
-    override fun onRescheduleAsked(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) {
-        rescheduleSyncService(sessionId, timeout, delay)
+    override fun onRescheduleAsked(sessionId: String,
+                                   syncTimeoutSeconds: Int,
+                                   syncDelaySeconds: Int) {
+        rescheduleSyncService(
+                sessionId = sessionId,
+                syncTimeoutSeconds = syncTimeoutSeconds,
+                syncDelaySeconds = syncDelaySeconds,
+                isPeriodic = true,
+                isNetworkBack = false
+        )
     }
 
-    override fun onNetworkError(sessionId: String, isInitialSync: Boolean, timeout: Int, delay: Int) {
-        Timber.d("## Sync: A network error occured during sync")
+    override fun onNetworkError(sessionId: String,
+                                syncTimeoutSeconds: Int,
+                                syncDelaySeconds: Int,
+                                isPeriodic: Boolean) {
+        Timber.d("## Sync: A network error occurred during sync")
         val rescheduleSyncWorkRequest: WorkRequest =
                 OneTimeWorkRequestBuilder<RestartWhenNetworkOn>()
-                        .setInputData(Data.Builder()
-                                .putString("sessionId", sessionId)
-                                .putInt("timeout", timeout)
-                                .putInt("delay", delay)
-                                .build()
-                        )
+                        .setInputData(RestartWhenNetworkOn.createInputData(sessionId, syncTimeoutSeconds, syncDelaySeconds, isPeriodic))
                         .setConstraints(Constraints.Builder()
                                 .setRequiredNetworkType(NetworkType.CONNECTED)
                                 .build()
@@ -124,31 +134,84 @@ 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)
-            applicationContext.rescheduleSyncService(sessionId, timeout, delay, true)
+            Timber.d("## Sync: RestartWhenNetworkOn.doWork()")
+            val sessionId = inputData.getString(KEY_SESSION_ID) ?: return Result.failure()
+            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 = syncTimeoutSeconds,
+                    syncDelaySeconds = syncDelaySeconds,
+                    isPeriodic = isPeriodic,
+                    isNetworkBack = true
+            )
             // Indicate whether the work finished successfully with the Result
             return Result.success()
         }
+
+        companion object {
+            fun createInputData(sessionId: String,
+                                syncTimeoutSeconds: Int,
+                                syncDelaySeconds: Int,
+                                isPeriodic: Boolean
+            ): Data {
+                return Data.Builder()
+                        .putString(KEY_SESSION_ID, sessionId)
+                        .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_SYNC_TIMEOUT_SECONDS = "timeout"
+            private const val KEY_SYNC_DELAY_SECONDS = "delay"
+            private const val KEY_IS_PERIODIC = "isPeriodic"
+        }
     }
 }
 
-private fun Context.rescheduleSyncService(sessionId: String, timeout: Int, delay: Int, networkBack: Boolean = false) {
-    val periodicIntent = VectorSyncService.newPeriodicIntent(this, sessionId, timeout, delay, networkBack)
-    val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-        PendingIntent.getForegroundService(this, 0, periodicIntent, 0)
+private fun Context.rescheduleSyncService(sessionId: String,
+                                          syncTimeoutSeconds: Int,
+                                          syncDelaySeconds: Int,
+                                          isPeriodic: Boolean,
+                                          isNetworkBack: Boolean) {
+    Timber.d("## Sync: rescheduleSyncService")
+    val intent = if (isPeriodic) {
+        VectorSyncService.newPeriodicIntent(
+                context = this,
+                sessionId = sessionId,
+                syncTimeoutSeconds = syncTimeoutSeconds,
+                syncDelaySeconds = syncDelaySeconds,
+                isNetworkBack = isNetworkBack
+        )
     } else {
-        PendingIntent.getService(this, 0, periodicIntent, 0)
+        VectorSyncService.newOneShotIntent(
+                context = this,
+                sessionId = sessionId
+        )
     }
-    val firstMillis = System.currentTimeMillis() + delay * 1000L
+
+    if (isNetworkBack || syncDelaySeconds == 0) {
+        // Do not wait, do the sync now (more reactivity if network back is due to user action)
+        startService(intent)
+    } else {
+        val pendingIntent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            PendingIntent.getForegroundService(this, 0, intent, 0)
+        } else {
+            PendingIntent.getService(this, 0, intent, 0)
+        }
+        val firstMillis = System.currentTimeMillis() + syncDelaySeconds * 1000L
         val alarmMgr = getSystemService<AlarmManager>()!!
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
             alarmMgr.setAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent)
         } else {
             alarmMgr.set(AlarmManager.RTC_WAKEUP, firstMillis, pendingIntent)
         }
+    }
 }