From 25e9a179d2b7539a9e4b862bdeff13d85f3164df Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 17 Sep 2019 14:26:30 +0200 Subject: [PATCH] SyncThread: Fix issue when network is back and the app was in background: do not restart the thread --- .../network/NetworkConnectivityChecker.kt | 9 ++--- .../internal/session/sync/job/SyncService.kt | 4 +- .../internal/session/sync/job/SyncThread.kt | 40 +++++++++++-------- .../util/BackgroundDetectionObserver.kt | 2 +- .../features/sync/widget/SyncStateView.kt | 2 +- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/NetworkConnectivityChecker.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/NetworkConnectivityChecker.kt index f89b737eff..ed1702ec07 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/NetworkConnectivityChecker.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/network/NetworkConnectivityChecker.kt @@ -36,7 +36,8 @@ internal class NetworkConnectivityChecker @Inject constructor(context: Context) private val listeners = Collections.synchronizedSet(LinkedHashSet()) // True when internet is available - private var hasInternetAccess = false + var hasInternetAccess = false + private set init { merlin.bind() @@ -63,7 +64,7 @@ internal class NetworkConnectivityChecker @Inject constructor(context: Context) } suspend fun waitUntilConnected() { - if (isConnected()) { + if (hasInternetAccess) { return } else { suspendCoroutine { continuation -> @@ -85,10 +86,6 @@ internal class NetworkConnectivityChecker @Inject constructor(context: Context) listeners.remove(listener) } - fun isConnected(): Boolean { - return hasInternetAccess - } - interface Listener { fun onConnect() { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncService.kt index 148e25b3a7..b5d83607b2 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncService.kt @@ -93,8 +93,8 @@ open class SyncService : Service() { } fun doSync(once: Boolean = false) { - if (!networkConnectivityChecker.isConnected()) { - Timber.v("Sync is Paused. Waiting...") + if (!networkConnectivityChecker.hasInternetAccess) { + Timber.v("No internet access. Waiting...") //TODO Retry in ? timer.schedule(object : TimerTask() { override fun run() { diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt index 7433a70a50..1cb6575161 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/sync/job/SyncThread.kt @@ -50,6 +50,8 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask, private val lock = Object() private var cancelableTask: Cancelable? = null + private var isStarted = false + init { updateStateTo(SyncState.IDLE) } @@ -60,19 +62,18 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask, } fun restart() = synchronized(lock) { - if (state is SyncState.PAUSED) { + if (!isStarted) { Timber.v("Resume sync...") - updateStateTo(SyncState.RUNNING(afterPause = true)) + isStarted = true lock.notify() } } fun pause() = synchronized(lock) { - if (state is SyncState.RUNNING) { + if (isStarted) { Timber.v("Pause sync...") - updateStateTo(SyncState.PAUSED) + isStarted = false cancelableTask?.cancel() - lock.notify() } } @@ -87,21 +88,31 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask, return liveState } + override fun onConnect() { + Timber.v("Network is back") + synchronized(lock) { + lock.notify() + } + } + override fun run() { Timber.v("Start syncing...") + isStarted = true networkConnectivityChecker.register(this) backgroundDetectionObserver.register(this) while (state != SyncState.KILLING) { Timber.v("Entering loop, state: $state") - if (!networkConnectivityChecker.isConnected() || state == SyncState.PAUSED) { - Timber.v("No network or sync is Paused. Waiting...") + if (!networkConnectivityChecker.hasInternetAccess) { + Timber.v("No network. Waiting...") updateStateTo(SyncState.NO_NETWORK) - - synchronized(lock) { - lock.wait() - } + synchronized(lock) { lock.wait() } + Timber.v("...unlocked") + } else if (!isStarted) { + Timber.v("Sync is Paused. Waiting...") + updateStateTo(SyncState.PAUSED) + synchronized(lock) { lock.wait() } Timber.v("...unlocked") } else { if (state !is SyncState.RUNNING) { @@ -169,16 +180,11 @@ internal class SyncThread @Inject constructor(private val syncTask: SyncTask, } private fun updateStateTo(newState: SyncState) { - Timber.v("Update state to $newState") + Timber.v("Update state from $state to $newState") state = newState liveState.postValue(newState) } - override fun onConnect() { - synchronized(lock) { - lock.notify() - } - } override fun onMoveToForeground() { restart() diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/BackgroundDetectionObserver.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/BackgroundDetectionObserver.kt index a426bdd084..9318b10717 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/BackgroundDetectionObserver.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/util/BackgroundDetectionObserver.kt @@ -33,7 +33,7 @@ internal class BackgroundDetectionObserver @Inject constructor() : LifecycleObse private set private - val listeners = ArrayList() + val listeners = LinkedHashSet() fun register(listener: Listener) { listeners.add(listener) diff --git a/vector/src/main/java/im/vector/riotx/features/sync/widget/SyncStateView.kt b/vector/src/main/java/im/vector/riotx/features/sync/widget/SyncStateView.kt index 2d474a13dd..43158f4e86 100755 --- a/vector/src/main/java/im/vector/riotx/features/sync/widget/SyncStateView.kt +++ b/vector/src/main/java/im/vector/riotx/features/sync/widget/SyncStateView.kt @@ -38,6 +38,6 @@ class SyncStateView @JvmOverloads constructor(context: Context, attrs: Attribute is SyncState.RUNNING -> if (newState.afterPause) View.VISIBLE else View.GONE else -> View.GONE } - syncStateNoNetwork.isVisible = newState is SyncState.NO_NETWORK + syncStateNoNetwork.isVisible = newState == SyncState.NO_NETWORK } }