Fix NPE for tasks without a subId
UploadTask does not have a valid subId because it is broadcasted
toward all accounts. ACTION_SYNC_VOICEMAIL is also toward all accounts
and lacks the subId.
In ag/1245021 we passed in a voicemail status editor with the invalid
subId while creating the upload task, which causes NPE.
After this CL OmtpVvmSyncService.sync() no longer accepts a null subId.
Events that targets all accounts will launch a individual task for each
account instead.
- TaskId.SUB_ID_ANY is removed. All task should have a explicit subId
Change-Id: Ib4892daf4f9cb732dbce9710f87754cd780fa535
Fixes: 30367830
diff --git a/src/com/android/phone/vvm/omtp/scheduling/BaseTask.java b/src/com/android/phone/vvm/omtp/scheduling/BaseTask.java
index 273d2d3..76537fa 100644
--- a/src/com/android/phone/vvm/omtp/scheduling/BaseTask.java
+++ b/src/com/android/phone/vvm/omtp/scheduling/BaseTask.java
@@ -23,6 +23,7 @@
import android.content.Intent;
import android.os.SystemClock;
import android.support.annotation.NonNull;
+import android.telephony.SubscriptionManager;
import com.android.phone.Assert;
import com.android.phone.NeededForTesting;
import java.util.ArrayList;
@@ -39,7 +40,7 @@
private Context mContext;
private int mId;
- private int mSubId = TaskId.SUB_ID_ANY;
+ private int mSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
private boolean mHasStarted;
private volatile boolean mHasFailed;
@@ -144,7 +145,7 @@
@CallSuper
public void onCreate(Context context, Intent intent, int flags, int startId) {
mContext = context;
- mSubId = intent.getIntExtra(EXTRA_SUB_ID, TaskId.SUB_ID_ANY);
+ mSubId = intent.getIntExtra(EXTRA_SUB_ID, SubscriptionManager.INVALID_SUBSCRIPTION_ID);
for (Policy policy : mPolicies) {
policy.onCreate(this, intent, flags, startId);
}
diff --git a/src/com/android/phone/vvm/omtp/scheduling/Task.java b/src/com/android/phone/vvm/omtp/scheduling/Task.java
index 68dbad9..05d86fd 100644
--- a/src/com/android/phone/vvm/omtp/scheduling/Task.java
+++ b/src/com/android/phone/vvm/omtp/scheduling/Task.java
@@ -20,7 +20,6 @@
import android.annotation.WorkerThread;
import android.content.Context;
import android.content.Intent;
-
import java.util.Objects;
/**
@@ -57,12 +56,6 @@
class TaskId {
/**
- * Special subId value to indicate unspecified subId. Having SUB_ID_ANY does NOT prevent
- * task on other subId from executing.
- */
- public static final int SUB_ID_ANY = -1;
-
- /**
* Indicates the operation type of the task.
*/
public final int id;
diff --git a/src/com/android/phone/vvm/omtp/scheduling/TaskSchedulerService.java b/src/com/android/phone/vvm/omtp/scheduling/TaskSchedulerService.java
index a965795..1e099e8 100644
--- a/src/com/android/phone/vvm/omtp/scheduling/TaskSchedulerService.java
+++ b/src/com/android/phone/vvm/omtp/scheduling/TaskSchedulerService.java
@@ -30,13 +30,11 @@
import android.os.Message;
import android.os.PowerManager;
import android.os.PowerManager.WakeLock;
-
import com.android.internal.annotations.VisibleForTesting;
import com.android.phone.Assert;
import com.android.phone.NeededForTesting;
import com.android.phone.vvm.omtp.VvmLog;
import com.android.phone.vvm.omtp.scheduling.Task.TaskId;
-
import java.util.ArrayDeque;
import java.util.Queue;
@@ -109,7 +107,7 @@
VvmLog.v(TAG, "executing task " + task);
task.onExecuteInBackgroundThread();
} catch (Throwable throwable) {
- VvmLog.e(TAG, "Exception while executing task " + task + ":" + throwable);
+ VvmLog.e(TAG, "Exception while executing task " + task + ":", throwable);
}
Message schedulerMessage = mMainThreadHandler.obtainMessage();
diff --git a/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncReceiver.java b/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncReceiver.java
index b424281..41178eb 100644
--- a/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncReceiver.java
+++ b/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncReceiver.java
@@ -20,8 +20,13 @@
import android.content.Context;
import android.content.Intent;
import android.provider.VoicemailContract;
-
+import android.telecom.PhoneAccountHandle;
+import android.telecom.TelecomManager;
+import com.android.phone.settings.VisualVoicemailSettingsUtil;
+import com.android.phone.vvm.omtp.ActivationTask;
import com.android.phone.vvm.omtp.VvmLog;
+import com.android.phone.vvm.omtp.utils.PhoneAccountHandleConverter;
+import java.util.List;
public class OmtpVvmSyncReceiver extends BroadcastReceiver {
@@ -31,7 +36,26 @@
public void onReceive(final Context context, Intent intent) {
if (VoicemailContract.ACTION_SYNC_VOICEMAIL.equals(intent.getAction())) {
VvmLog.v(TAG, "Sync intent received");
- SyncTask.start(context, null, OmtpVvmSyncService.SYNC_FULL_SYNC);
+ for (PhoneAccountHandle source : OmtpVvmSourceManager.getInstance(context)
+ .getOmtpVvmSources()) {
+ SyncTask.start(context, source, OmtpVvmSyncService.SYNC_FULL_SYNC);
+ }
+ activateUnactivatedAccounts(context);
+ }
+ }
+
+ private static void activateUnactivatedAccounts(Context context) {
+ List<PhoneAccountHandle> accounts =
+ context.getSystemService(TelecomManager.class).getCallCapablePhoneAccounts();
+ for (PhoneAccountHandle phoneAccount : accounts) {
+ if (!VisualVoicemailSettingsUtil.isEnabled(context, phoneAccount)) {
+ continue;
+ }
+ int subId = PhoneAccountHandleConverter.toSubId(phoneAccount);
+ if (!OmtpVvmSourceManager.getInstance(context).isVvmSourceRegistered(phoneAccount)) {
+ VvmLog.i(TAG, "Unactivated account " + phoneAccount + " found, activating");
+ ActivationTask.start(context, subId, null);
+ }
}
}
}
diff --git a/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncService.java b/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncService.java
index 2dc27b2..87b68e5 100644
--- a/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncService.java
+++ b/src/com/android/phone/vvm/omtp/sync/OmtpVvmSyncService.java
@@ -20,9 +20,9 @@
import android.net.Uri;
import android.provider.VoicemailContract;
import android.telecom.PhoneAccountHandle;
-import android.telecom.TelecomManager;
import android.telecom.Voicemail;
import android.text.TextUtils;
+import com.android.phone.Assert;
import com.android.phone.PhoneUtils;
import com.android.phone.VoicemailStatus;
import com.android.phone.settings.VisualVoicemailSettingsUtil;
@@ -40,7 +40,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Set;
/**
* Sync OMTP visual voicemail.
@@ -83,36 +82,9 @@
public void sync(BaseTask task, String action, PhoneAccountHandle phoneAccount,
Voicemail voicemail, VoicemailStatus.Editor status) {
- VvmLog.log(TAG, "Sync requested: " + action +
- " for all accounts: " + String.valueOf(phoneAccount == null));
- if (phoneAccount != null) {
- VvmLog.v(TAG, "Sync requested: " + action + " - for account: " + phoneAccount);
- setupAndSendRequest(task, phoneAccount, voicemail, action, status);
- } else {
- VvmLog.v(TAG, "Sync requested: " + action + " - for all accounts");
- OmtpVvmSourceManager vvmSourceManager =
- OmtpVvmSourceManager.getInstance(mContext);
- Set<PhoneAccountHandle> sources = vvmSourceManager.getOmtpVvmSources();
- for (PhoneAccountHandle source : sources) {
- setupAndSendRequest(task, source, null, action, status);
- }
- activateUnactivatedAccounts();
- }
- }
-
- private void activateUnactivatedAccounts() {
- List<PhoneAccountHandle> accounts =
- mContext.getSystemService(TelecomManager.class).getCallCapablePhoneAccounts();
- for (PhoneAccountHandle phoneAccount : accounts) {
- if (!VisualVoicemailSettingsUtil.isEnabled(mContext, phoneAccount)) {
- continue;
- }
- int subId = PhoneAccountHandleConverter.toSubId(phoneAccount);
- if (!OmtpVvmSourceManager.getInstance(mContext).isVvmSourceRegistered(phoneAccount)) {
- VvmLog.i(TAG, "Unactivated account " + phoneAccount + " found, activating");
- ActivationTask.start(mContext, subId, null);
- }
- }
+ Assert.isTrue(phoneAccount != null);
+ VvmLog.v(TAG, "Sync requested: " + action + " - for account: " + phoneAccount);
+ setupAndSendRequest(task, phoneAccount, voicemail, action, status);
}
private void setupAndSendRequest(BaseTask task, PhoneAccountHandle phoneAccount,
diff --git a/src/com/android/phone/vvm/omtp/sync/UploadTask.java b/src/com/android/phone/vvm/omtp/sync/UploadTask.java
index 7945c08..fc28acf 100644
--- a/src/com/android/phone/vvm/omtp/sync/UploadTask.java
+++ b/src/com/android/phone/vvm/omtp/sync/UploadTask.java
@@ -18,9 +18,11 @@
import android.content.Context;
import android.content.Intent;
+import android.telecom.PhoneAccountHandle;
import com.android.phone.VoicemailStatus;
import com.android.phone.vvm.omtp.scheduling.BaseTask;
import com.android.phone.vvm.omtp.scheduling.PostponePolicy;
+import com.android.phone.vvm.omtp.utils.PhoneAccountHandleConverter;
/**
* Upload task triggered by database changes. Will wait until the database has been stable for
@@ -35,16 +37,23 @@
addPolicy(new PostponePolicy(POSTPONE_MILLIS));
}
- public static void start(Context context) {
+ public static void start(Context context, PhoneAccountHandle phoneAccountHandle) {
Intent intent = BaseTask
- .createIntent(context, UploadTask.class, TaskId.SUB_ID_ANY);
+ .createIntent(context, UploadTask.class,
+ PhoneAccountHandleConverter.toSubId(phoneAccountHandle));
context.startService(intent);
}
@Override
+ public void onCreate(Context context, Intent intent, int flags, int startId) {
+ super.onCreate(context, intent, flags, startId);
+ }
+
+ @Override
public void onExecuteInBackgroundThread() {
OmtpVvmSyncService service = new OmtpVvmSyncService(getContext());
- service.sync(this, OmtpVvmSyncService.SYNC_UPLOAD_ONLY, null, null,
+ service.sync(this, OmtpVvmSyncService.SYNC_UPLOAD_ONLY,
+ PhoneAccountHandleConverter.fromSubId(getSubId()), null,
VoicemailStatus.edit(getContext(), getSubId()));
}
}
diff --git a/src/com/android/phone/vvm/omtp/sync/VoicemailProviderChangeReceiver.java b/src/com/android/phone/vvm/omtp/sync/VoicemailProviderChangeReceiver.java
index bf2e125..bc9e6e1 100644
--- a/src/com/android/phone/vvm/omtp/sync/VoicemailProviderChangeReceiver.java
+++ b/src/com/android/phone/vvm/omtp/sync/VoicemailProviderChangeReceiver.java
@@ -19,6 +19,7 @@
import android.content.Context;
import android.content.Intent;
import android.provider.VoicemailContract;
+import android.telecom.PhoneAccountHandle;
/**
* Receives changes to the voicemail provider so they can be sent to the voicemail server.
@@ -31,7 +32,10 @@
OmtpVvmSourceManager vvmSourceManager =
OmtpVvmSourceManager.getInstance(context);
if (vvmSourceManager.getOmtpVvmSources().size() > 0 && !isSelfChanged) {
- UploadTask.start(context);
+ for (PhoneAccountHandle source : OmtpVvmSourceManager.getInstance(context)
+ .getOmtpVvmSources()) {
+ UploadTask.start(context, source);
+ }
}
}
}