From 74bcc44f3e25dc4cc21ebf5b4f53529819e99939 Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 29 Aug 2017 17:44:18 +0200 Subject: [PATCH 1/3] Clean up ActivityListener No functional changes --- .../fsck/k9/activity/ActivityListener.java | 203 ++++++++---------- 1 file changed, 95 insertions(+), 108 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java index 1d494bc8d..33bc0f7a5 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java @@ -1,5 +1,6 @@ package com.fsck.k9.activity; + import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -13,215 +14,203 @@ import com.fsck.k9.R; import com.fsck.k9.controller.SimpleMessagingListener; import com.fsck.k9.service.MailService; -public class ActivityListener extends SimpleMessagingListener { - private Account mAccount = null; - private String mLoadingFolderName = null; - private String mLoadingHeaderFolderName = null; - private String mLoadingAccountDescription = null; - private String mSendingAccountDescription = null; - private int mFolderCompleted = 0; - private int mFolderTotal = 0; - private String mProcessingAccountDescription = null; - private String mProcessingCommandTitle = null; - private BroadcastReceiver mTickReceiver = new BroadcastReceiver() { +public class ActivityListener extends SimpleMessagingListener { + private Account account = null; + private String loadingFolderName = null; + private String loadingHeaderFolderName = null; + private String loadingAccountDescription = null; + private String sendingAccountDescription = null; + private int folderCompleted = 0; + private int folderTotal = 0; + private String processingAccountDescription = null; + private String processingCommandTitle = null; + + private BroadcastReceiver tickReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { informUserOfStatus(); } }; + public String getOperation(Context context) { - if (mLoadingAccountDescription != null - || mSendingAccountDescription != null - || mLoadingHeaderFolderName != null - || mProcessingAccountDescription != null) { - + if (loadingAccountDescription != null || + sendingAccountDescription != null || + loadingHeaderFolderName != null || + processingAccountDescription != null) { return getActionInProgressOperation(context); + } - } else { - long nextPollTime = MailService.getNextPollTime(); - if (nextPollTime != -1) { - return context.getString(R.string.status_next_poll, - DateUtils.getRelativeTimeSpanString(nextPollTime, System.currentTimeMillis(), - DateUtils.MINUTE_IN_MILLIS, 0)); - } else if (K9.isDebug() && MailService.isSyncDisabled()) { - if (MailService.hasNoConnectivity()) { - return context.getString(R.string.status_no_network); - } else if (MailService.isSyncNoBackground()) { - return context.getString(R.string.status_no_background); - } else if (MailService.isSyncBlocked()) { - return context.getString(R.string.status_syncing_blocked); - } else if (MailService.isPollAndPushDisabled()) { - return context.getString(R.string.status_poll_and_push_disabled); - } else { - return context.getString(R.string.status_syncing_off); - } - } else if (MailService.isSyncDisabled()) { - return context.getString(R.string.status_syncing_off); + long nextPollTime = MailService.getNextPollTime(); + if (nextPollTime != -1) { + CharSequence relativeTimeSpanString = DateUtils.getRelativeTimeSpanString( + nextPollTime, System.currentTimeMillis(), DateUtils.MINUTE_IN_MILLIS, 0); + return context.getString(R.string.status_next_poll, relativeTimeSpanString); + } else if (K9.isDebug() && MailService.isSyncDisabled()) { + if (MailService.hasNoConnectivity()) { + return context.getString(R.string.status_no_network); + } else if (MailService.isSyncNoBackground()) { + return context.getString(R.string.status_no_background); + } else if (MailService.isSyncBlocked()) { + return context.getString(R.string.status_syncing_blocked); + } else if (MailService.isPollAndPushDisabled()) { + return context.getString(R.string.status_poll_and_push_disabled); } else { - return ""; + return context.getString(R.string.status_syncing_off); } + } else if (MailService.isSyncDisabled()) { + return context.getString(R.string.status_syncing_off); + } else { + return ""; } } private String getActionInProgressOperation(Context context) { - String progress = (mFolderTotal > 0 ? - context.getString(R.string.folder_progress, mFolderCompleted, mFolderTotal) : ""); + String progress = folderTotal > 0 ? + context.getString(R.string.folder_progress, folderCompleted, folderTotal) : ""; - if (mLoadingFolderName != null || mLoadingHeaderFolderName != null) { + if (loadingFolderName != null || loadingHeaderFolderName != null) { String displayName = null; - if (mLoadingHeaderFolderName != null) { - displayName = mLoadingHeaderFolderName; - } else if (mLoadingFolderName != null) { - displayName = mLoadingFolderName; + if (loadingHeaderFolderName != null) { + displayName = loadingHeaderFolderName; + } else if (loadingFolderName != null) { + displayName = loadingFolderName; } - if ((mAccount != null) && (mAccount.getInboxFolderName() != null) - && mAccount.getInboxFolderName().equalsIgnoreCase(displayName)) { + if (account != null && account.getInboxFolderName() != null && + account.getInboxFolderName().equalsIgnoreCase(displayName)) { displayName = context.getString(R.string.special_mailbox_name_inbox); - } else if ((mAccount != null) && (mAccount.getOutboxFolderName() != null) - && mAccount.getOutboxFolderName().equals(displayName)) { + } else if (account != null && account.getOutboxFolderName() != null && + account.getOutboxFolderName().equals(displayName)) { displayName = context.getString(R.string.special_mailbox_name_outbox); } - if (mLoadingHeaderFolderName != null) { + if (loadingHeaderFolderName != null) { return context.getString(R.string.status_loading_account_folder_headers, - mLoadingAccountDescription, displayName, progress); + loadingAccountDescription, displayName, progress); } else { return context.getString(R.string.status_loading_account_folder, - mLoadingAccountDescription, displayName, progress); + loadingAccountDescription, displayName, progress); } - } - - else if (mSendingAccountDescription != null) { - return context.getString(R.string.status_sending_account, mSendingAccountDescription, progress); - } else if (mProcessingAccountDescription != null) { - return context.getString(R.string.status_processing_account, mProcessingAccountDescription, - mProcessingCommandTitle != null ? mProcessingCommandTitle : "", - progress); + } else if (sendingAccountDescription != null) { + return context.getString(R.string.status_sending_account, sendingAccountDescription, progress); + } else if (processingAccountDescription != null) { + return context.getString(R.string.status_processing_account, processingAccountDescription, + processingCommandTitle != null ? processingCommandTitle : "", progress); } else { return ""; } } public void onResume(Context context) { - context.registerReceiver(mTickReceiver, new IntentFilter(Intent.ACTION_TIME_TICK)); + context.registerReceiver(tickReceiver, new IntentFilter(Intent.ACTION_TIME_TICK)); } public void onPause(Context context) { - context.unregisterReceiver(mTickReceiver); + context.unregisterReceiver(tickReceiver); } public void informUserOfStatus() { } @Override - public void synchronizeMailboxFinished( - Account account, - String folder, - int totalMessagesInMailbox, - int numNewMessages) { - mLoadingAccountDescription = null; - mLoadingFolderName = null; - mAccount = null; + public void synchronizeMailboxFinished(Account account, String folder, int totalMessagesInMailbox, + int numNewMessages) { + loadingAccountDescription = null; + loadingFolderName = null; + this.account = null; informUserOfStatus(); } @Override public void synchronizeMailboxStarted(Account account, String folder) { - mLoadingAccountDescription = account.getDescription(); - mLoadingFolderName = folder; - mAccount = account; - mFolderCompleted = 0; - mFolderTotal = 0; + loadingAccountDescription = account.getDescription(); + loadingFolderName = folder; + this.account = account; + folderCompleted = 0; + folderTotal = 0; informUserOfStatus(); } - @Override public void synchronizeMailboxHeadersStarted(Account account, String folder) { - mLoadingAccountDescription = account.getDescription(); - mLoadingHeaderFolderName = folder; + loadingAccountDescription = account.getDescription(); + loadingHeaderFolderName = folder; informUserOfStatus(); } - @Override public void synchronizeMailboxHeadersProgress(Account account, String folder, int completed, int total) { - mFolderCompleted = completed; - mFolderTotal = total; + folderCompleted = completed; + folderTotal = total; informUserOfStatus(); } @Override - public void synchronizeMailboxHeadersFinished(Account account, String folder, - int total, int completed) { - mLoadingHeaderFolderName = null; - mFolderCompleted = 0; - mFolderTotal = 0; + public void synchronizeMailboxHeadersFinished(Account account, String folder, int total, int completed) { + loadingHeaderFolderName = null; + folderCompleted = 0; + folderTotal = 0; informUserOfStatus(); } - @Override public void synchronizeMailboxProgress(Account account, String folder, int completed, int total) { - mFolderCompleted = completed; - mFolderTotal = total; + folderCompleted = completed; + folderTotal = total; informUserOfStatus(); } @Override - public void synchronizeMailboxFailed(Account account, String folder, - String message) { - mLoadingAccountDescription = null; - mLoadingHeaderFolderName = null; - mLoadingFolderName = null; - mAccount = null; + public void synchronizeMailboxFailed(Account account, String folder, String message) { + loadingAccountDescription = null; + loadingHeaderFolderName = null; + loadingFolderName = null; + this.account = null; informUserOfStatus(); - } @Override public void sendPendingMessagesStarted(Account account) { - mSendingAccountDescription = account.getDescription(); + sendingAccountDescription = account.getDescription(); informUserOfStatus(); } @Override public void sendPendingMessagesCompleted(Account account) { - mSendingAccountDescription = null; + sendingAccountDescription = null; informUserOfStatus(); } @Override public void sendPendingMessagesFailed(Account account) { - mSendingAccountDescription = null; + sendingAccountDescription = null; informUserOfStatus(); } @Override public void pendingCommandsProcessing(Account account) { - mProcessingAccountDescription = account.getDescription(); - mFolderCompleted = 0; - mFolderTotal = 0; + processingAccountDescription = account.getDescription(); + folderCompleted = 0; + folderTotal = 0; informUserOfStatus(); } @Override public void pendingCommandsFinished(Account account) { - mProcessingAccountDescription = null; + processingAccountDescription = null; informUserOfStatus(); } @Override public void pendingCommandStarted(Account account, String commandTitle) { - mProcessingCommandTitle = commandTitle; + processingCommandTitle = commandTitle; informUserOfStatus(); } @Override public void pendingCommandCompleted(Account account, String commandTitle) { - mProcessingCommandTitle = null; + processingCommandTitle = null; informUserOfStatus(); } @@ -241,12 +230,10 @@ public class ActivityListener extends SimpleMessagingListener { } public int getFolderCompleted() { - return mFolderCompleted; + return folderCompleted; } - public int getFolderTotal() { - return mFolderTotal; + return folderTotal; } - } From 349b7bbcee002f22ce838a0f4abeb7fdd74927ba Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 29 Aug 2017 18:04:39 +0200 Subject: [PATCH 2/3] Prevent concurrent access to fields in ActivityListener --- k9mail/build.gradle | 1 + .../fsck/k9/activity/ActivityListener.java | 147 ++++++++++++------ 2 files changed, 100 insertions(+), 48 deletions(-) diff --git a/k9mail/build.gradle b/k9mail/build.gradle index 09c10a99b..4a4bef840 100644 --- a/k9mail/build.gradle +++ b/k9mail/build.gradle @@ -35,6 +35,7 @@ dependencies { compile 'com.github.amlcurran.showcaseview:library:5.4.1' compile 'com.squareup.moshi:moshi:1.2.0' compile "com.jakewharton.timber:timber:${timberVersion}" + compile 'net.jcip:jcip-annotations:1.0' androidTestCompile 'com.android.support.test.espresso:espresso-core:2.2.2' diff --git a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java index 33bc0f7a5..04eebdd44 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java @@ -13,18 +13,21 @@ import com.fsck.k9.K9; import com.fsck.k9.R; import com.fsck.k9.controller.SimpleMessagingListener; import com.fsck.k9.service.MailService; +import net.jcip.annotations.GuardedBy; public class ActivityListener extends SimpleMessagingListener { - private Account account = null; - private String loadingFolderName = null; - private String loadingHeaderFolderName = null; - private String loadingAccountDescription = null; - private String sendingAccountDescription = null; - private int folderCompleted = 0; - private int folderTotal = 0; - private String processingAccountDescription = null; - private String processingCommandTitle = null; + private final Object lock = new Object(); + + @GuardedBy("lock") private Account account = null; + @GuardedBy("lock") private String loadingFolderName = null; + @GuardedBy("lock") private String loadingHeaderFolderName = null; + @GuardedBy("lock") private String loadingAccountDescription = null; + @GuardedBy("lock") private String sendingAccountDescription = null; + @GuardedBy("lock") private int folderCompleted = 0; + @GuardedBy("lock") private int folderTotal = 0; + @GuardedBy("lock") private String processingAccountDescription = null; + @GuardedBy("lock") private String processingCommandTitle = null; private BroadcastReceiver tickReceiver = new BroadcastReceiver() { @Override @@ -35,11 +38,13 @@ public class ActivityListener extends SimpleMessagingListener { public String getOperation(Context context) { - if (loadingAccountDescription != null || - sendingAccountDescription != null || - loadingHeaderFolderName != null || - processingAccountDescription != null) { - return getActionInProgressOperation(context); + synchronized (lock) { + if (loadingAccountDescription != null || + sendingAccountDescription != null || + loadingHeaderFolderName != null || + processingAccountDescription != null) { + return getActionInProgressOperation(context); + } } long nextPollTime = MailService.getNextPollTime(); @@ -66,15 +71,16 @@ public class ActivityListener extends SimpleMessagingListener { } } + @GuardedBy("lock") private String getActionInProgressOperation(Context context) { String progress = folderTotal > 0 ? context.getString(R.string.folder_progress, folderCompleted, folderTotal) : ""; if (loadingFolderName != null || loadingHeaderFolderName != null) { - String displayName = null; + String displayName; if (loadingHeaderFolderName != null) { displayName = loadingHeaderFolderName; - } else if (loadingFolderName != null) { + } else { displayName = loadingFolderName; } if (account != null && account.getInboxFolderName() != null && @@ -116,101 +122,142 @@ public class ActivityListener extends SimpleMessagingListener { @Override public void synchronizeMailboxFinished(Account account, String folder, int totalMessagesInMailbox, int numNewMessages) { - loadingAccountDescription = null; - loadingFolderName = null; - this.account = null; + synchronized (lock) { + loadingAccountDescription = null; + loadingFolderName = null; + this.account = null; + } + informUserOfStatus(); } @Override public void synchronizeMailboxStarted(Account account, String folder) { - loadingAccountDescription = account.getDescription(); - loadingFolderName = folder; - this.account = account; - folderCompleted = 0; - folderTotal = 0; + synchronized (lock) { + loadingAccountDescription = account.getDescription(); + loadingFolderName = folder; + this.account = account; + folderCompleted = 0; + folderTotal = 0; + } + informUserOfStatus(); } @Override public void synchronizeMailboxHeadersStarted(Account account, String folder) { - loadingAccountDescription = account.getDescription(); - loadingHeaderFolderName = folder; + synchronized (lock) { + loadingAccountDescription = account.getDescription(); + loadingHeaderFolderName = folder; + } + informUserOfStatus(); } @Override public void synchronizeMailboxHeadersProgress(Account account, String folder, int completed, int total) { - folderCompleted = completed; - folderTotal = total; + synchronized (lock) { + folderCompleted = completed; + folderTotal = total; + } + informUserOfStatus(); } @Override public void synchronizeMailboxHeadersFinished(Account account, String folder, int total, int completed) { - loadingHeaderFolderName = null; - folderCompleted = 0; - folderTotal = 0; + synchronized (lock) { + loadingHeaderFolderName = null; + folderCompleted = 0; + folderTotal = 0; + } + informUserOfStatus(); } @Override public void synchronizeMailboxProgress(Account account, String folder, int completed, int total) { - folderCompleted = completed; - folderTotal = total; + synchronized (lock) { + folderCompleted = completed; + folderTotal = total; + } + informUserOfStatus(); } @Override public void synchronizeMailboxFailed(Account account, String folder, String message) { - loadingAccountDescription = null; - loadingHeaderFolderName = null; - loadingFolderName = null; - this.account = null; + synchronized (lock) { + loadingAccountDescription = null; + loadingHeaderFolderName = null; + loadingFolderName = null; + this.account = null; + } informUserOfStatus(); } @Override public void sendPendingMessagesStarted(Account account) { - sendingAccountDescription = account.getDescription(); + synchronized (lock) { + sendingAccountDescription = account.getDescription(); + } + informUserOfStatus(); } @Override public void sendPendingMessagesCompleted(Account account) { - sendingAccountDescription = null; + synchronized (lock) { + sendingAccountDescription = null; + } + informUserOfStatus(); } @Override public void sendPendingMessagesFailed(Account account) { - sendingAccountDescription = null; + synchronized (lock) { + sendingAccountDescription = null; + } + informUserOfStatus(); } @Override public void pendingCommandsProcessing(Account account) { - processingAccountDescription = account.getDescription(); - folderCompleted = 0; - folderTotal = 0; + synchronized (lock) { + processingAccountDescription = account.getDescription(); + folderCompleted = 0; + folderTotal = 0; + } + informUserOfStatus(); } @Override public void pendingCommandsFinished(Account account) { - processingAccountDescription = null; + synchronized (lock) { + processingAccountDescription = null; + } + informUserOfStatus(); } @Override public void pendingCommandStarted(Account account, String commandTitle) { - processingCommandTitle = commandTitle; + synchronized (lock) { + processingCommandTitle = commandTitle; + + } informUserOfStatus(); } @Override public void pendingCommandCompleted(Account account, String commandTitle) { - processingCommandTitle = null; + synchronized (lock) { + processingCommandTitle = null; + } + informUserOfStatus(); } @@ -230,10 +277,14 @@ public class ActivityListener extends SimpleMessagingListener { } public int getFolderCompleted() { - return folderCompleted; + synchronized (lock) { + return folderCompleted; + } } public int getFolderTotal() { - return folderTotal; + synchronized (lock) { + return folderTotal; + } } } From 1b364755df91a8a74040103963f2d03c2ace0a0e Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 29 Aug 2017 18:12:58 +0200 Subject: [PATCH 3/3] Simplify logic in ActivityListener.getActionInProgressOperation() --- .../java/com/fsck/k9/activity/ActivityListener.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java index 04eebdd44..93631085d 100644 --- a/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java +++ b/k9mail/src/main/java/com/fsck/k9/activity/ActivityListener.java @@ -83,12 +83,13 @@ public class ActivityListener extends SimpleMessagingListener { } else { displayName = loadingFolderName; } - if (account != null && account.getInboxFolderName() != null && - account.getInboxFolderName().equalsIgnoreCase(displayName)) { - displayName = context.getString(R.string.special_mailbox_name_inbox); - } else if (account != null && account.getOutboxFolderName() != null && - account.getOutboxFolderName().equals(displayName)) { - displayName = context.getString(R.string.special_mailbox_name_outbox); + + if (account != null) { + if (displayName.equalsIgnoreCase(account.getInboxFolderName())) { + displayName = context.getString(R.string.special_mailbox_name_inbox); + } else if (displayName.equalsIgnoreCase(account.getOutboxFolderName())) { + displayName = context.getString(R.string.special_mailbox_name_outbox); + } } if (loadingHeaderFolderName != null) {