Check activity state before showing phone disambig dialog
- This is a monkey bug on Crespo that happens often. The
monkey is likely tapping on a favorite contact in the Phone
app, which is trying to either dial the contact or
show a disambiguation dialog if the contact has multiple
phone numbers. There's an IllegalStateException with trying
to commit the dialog fragment transaction because the
saved state has already been called.
- After a PhoneNumberInteraction is started, a CursorLoader
is started, and when that returns, potentially we try to
commit a FragmentTransaction (showing the DialogFragment).
At this point, the activity may be long gone, so we need to
know if the activity has saved state or not. Hence,
we add a new base activity class (TransactionSafeActivity)
that keeps track of this information for us.
- The PhoneNumberInteractionTests still rely on passing in
a context to PhoneNumberInteraction (in order to check
that the queries and subsequent activities are correct),
but we typecast the static methods the app uses to
create these PhoneNumberInteraction objects.
Bug: 5132436
Change-Id: Id9d887bd55235b07133568a38d4922dc7fce24a7
diff --git a/src/com/android/contacts/ContactsActivity.java b/src/com/android/contacts/ContactsActivity.java
index 020d135..371b0b7 100644
--- a/src/com/android/contacts/ContactsActivity.java
+++ b/src/com/android/contacts/ContactsActivity.java
@@ -17,8 +17,8 @@
package com.android.contacts;
import com.android.contacts.test.InjectedServices;
+import com.android.contacts.activities.TransactionSafeActivity;
-import android.app.Activity;
import android.app.Fragment;
import android.app.FragmentManager;
import android.app.FragmentTransaction;
@@ -31,7 +31,7 @@
/**
* A common superclass for Contacts activities that handles application-wide services.
*/
-public abstract class ContactsActivity extends Activity
+public abstract class ContactsActivity extends TransactionSafeActivity
implements ContactSaveService.Listener
{
diff --git a/src/com/android/contacts/activities/DialtactsActivity.java b/src/com/android/contacts/activities/DialtactsActivity.java
index 1de7b76..6d99129 100644
--- a/src/com/android/contacts/activities/DialtactsActivity.java
+++ b/src/com/android/contacts/activities/DialtactsActivity.java
@@ -26,6 +26,7 @@
import com.android.contacts.list.OnPhoneNumberPickerActionListener;
import com.android.contacts.list.PhoneFavoriteFragment;
import com.android.contacts.list.PhoneNumberPickerFragment;
+import com.android.contacts.activities.TransactionSafeActivity;
import com.android.contacts.util.AccountFilterUtil;
import com.android.internal.telephony.ITelephony;
@@ -74,7 +75,7 @@
* embedded using intents.
* The dialer tab's title is 'phone', a more common name (see strings.xml).
*/
-public class DialtactsActivity extends Activity {
+public class DialtactsActivity extends TransactionSafeActivity {
private static final String TAG = "DialtactsActivity";
/** Used to open Call Setting */
diff --git a/src/com/android/contacts/activities/TransactionSafeActivity.java b/src/com/android/contacts/activities/TransactionSafeActivity.java
new file mode 100644
index 0000000..ab80b42
--- /dev/null
+++ b/src/com/android/contacts/activities/TransactionSafeActivity.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.contacts.activities;
+
+import android.app.Activity;
+import android.os.Bundle;
+
+/**
+ * A common superclass that keeps track of whether an {@link Activity} has saved its state yet or
+ * not.
+ */
+public class TransactionSafeActivity extends Activity {
+
+ private boolean mIsSafeToCommitTransactions;
+
+ @Override
+ protected void onCreate(Bundle savedInstanceState) {
+ super.onCreate(savedInstanceState);
+ mIsSafeToCommitTransactions = true;
+ }
+
+ @Override
+ protected void onStart() {
+ super.onStart();
+ mIsSafeToCommitTransactions = true;
+ }
+
+ @Override
+ protected void onResume() {
+ super.onResume();
+ mIsSafeToCommitTransactions = true;
+ }
+
+ @Override
+ protected void onSaveInstanceState(Bundle outState) {
+ super.onSaveInstanceState(outState);
+ mIsSafeToCommitTransactions = false;
+ }
+
+ /**
+ * Returns true if it is safe to commit {@link FragmentTransaction}s at this time, based on
+ * whether {@link Activity#onSaveInstanceState} has been called or not.
+ *
+ * Make sure that the current activity calls into
+ * {@link super.onSaveInstanceState(Bundle outState)} (if that method is overridden),
+ * so the flag is properly set.
+ */
+ public boolean isSafeToCommitTransactions() {
+ return mIsSafeToCommitTransactions;
+ }
+}
diff --git a/src/com/android/contacts/interactions/PhoneNumberInteraction.java b/src/com/android/contacts/interactions/PhoneNumberInteraction.java
index d10ec06..4c75896 100644
--- a/src/com/android/contacts/interactions/PhoneNumberInteraction.java
+++ b/src/com/android/contacts/interactions/PhoneNumberInteraction.java
@@ -15,13 +15,13 @@
*/
package com.android.contacts.interactions;
-
import com.android.contacts.Collapser;
import com.android.contacts.Collapser.Collapsible;
import com.android.contacts.ContactSaveService;
import com.android.contacts.ContactsUtils;
import com.android.contacts.R;
import com.android.contacts.activities.DialtactsActivity;
+import com.android.contacts.activities.TransactionSafeActivity;
import com.android.contacts.model.AccountType;
import com.android.contacts.model.AccountType.StringInflater;
import com.android.contacts.model.AccountTypeManager;
@@ -63,7 +63,8 @@
/**
* Initiates phone calls or a text message. If there are multiple candidates, this class shows a
- * dialog to pick one.
+ * dialog to pick one. Creating one of these interactions should be done through the static
+ * factory methods.
*/
public class PhoneNumberInteraction implements OnLoadCompleteListener<Cursor> {
private static final String TAG = PhoneNumberInteraction.class.getSimpleName();
@@ -183,9 +184,10 @@
* one will be chosen to make a call or initiate an sms message.
*
* It is recommended to use
- * {@link PhoneNumberInteraction#startInteractionForPhoneCall(Activity, Uri)} or
- * {@link PhoneNumberInteraction#startInteractionForTextMessage(Activity, Uri)} instead of
- * directly using this class, as those methods handle one or multiple data cases appropriately.
+ * {@link PhoneNumberInteraction#startInteractionForPhoneCall(TransactionSafeActivity, Uri)} or
+ * {@link PhoneNumberInteraction#startInteractionForTextMessage(TransactionSafeActivity, Uri)}
+ * instead of directly using this class, as those methods handle one or multiple data cases
+ * appropriately.
*/
/* Made public to let the system reach this class */
public static class PhoneDisambiguationDialogFragment extends DialogFragment
@@ -272,6 +274,12 @@
private CursorLoader mLoader;
+ /**
+ * Constructs a new {@link PhoneNumberInteraction}. The constructor takes in a {@link Context}
+ * instead of a {@link TransactionSafeActivity} for testing purposes to verify the functionality
+ * of this class. However, all factory methods for creating {@link PhoneNumberInteraction}s
+ * require a {@link TransactionSafeActivity} (i.e. see {@link #startInteractionForPhoneCall}).
+ */
@VisibleForTesting
/* package */ PhoneNumberInteraction(Context context, InteractionType interactionType,
DialogInterface.OnDismissListener dismissListener) {
@@ -347,7 +355,7 @@
@Override
public void onLoadComplete(Loader<Cursor> loader, Cursor cursor) {
- if (cursor == null) {
+ if (cursor == null || !isSafeToCommitTransactions()) {
onDismiss();
return;
}
@@ -396,6 +404,11 @@
}
}
+ private boolean isSafeToCommitTransactions() {
+ return mContext instanceof TransactionSafeActivity ?
+ ((TransactionSafeActivity) mContext).isSafeToCommitTransactions() : true;
+ }
+
private void onDismiss() {
if (mDismissListener != null) {
mDismissListener.onDismiss(null);
@@ -406,21 +419,27 @@
* Start call action using given contact Uri. If there are multiple candidates for the phone
* call, dialog is automatically shown and the user is asked to choose one.
*
+ * @param activity that is calling this interaction. This must be of type
+ * {@link TransactionSafeActivity} because we need to check on the activity state after the
+ * phone numbers have been queried for.
* @param uri contact Uri (built from {@link Contacts#CONTENT_URI}) or data Uri
* (built from {@link Data#CONTENT_URI}). Contact Uri may show the disambiguation dialog while
* data Uri won't.
*/
- public static void startInteractionForPhoneCall(Activity activity, Uri uri) {
+ public static void startInteractionForPhoneCall(TransactionSafeActivity activity, Uri uri) {
(new PhoneNumberInteraction(activity, InteractionType.PHONE_CALL, null))
.startInteraction(uri);
}
/**
+ * @param activity that is calling this interaction. This must be of type
+ * {@link TransactionSafeActivity} because we need to check on the activity state after the
+ * phone numbers have been queried for.
* @param callOrigin If non null, {@link DialtactsActivity#EXTRA_CALL_ORIGIN} will be
* appended to the Intent initiating phone call. See comments in Phone package (PhoneApp)
* for more detail.
*/
- public static void startInteractionForPhoneCall(Activity activity, Uri uri,
+ public static void startInteractionForPhoneCall(TransactionSafeActivity activity, Uri uri,
String callOrigin) {
(new PhoneNumberInteraction(activity, InteractionType.PHONE_CALL, null, callOrigin))
.startInteraction(uri);
@@ -431,11 +450,14 @@
* candidates for the phone call, dialog is automatically shown and the user is asked to choose
* one.
*
+ * @param activity that is calling this interaction. This must be of type
+ * {@link TransactionSafeActivity} because we need to check on the activity state after the
+ * phone numbers have been queried for.
* @param uri contact Uri (built from {@link Contacts#CONTENT_URI}) or data Uri
* (built from {@link Data#CONTENT_URI}). Contact Uri may show the disambiguation dialog while
* data Uri won't.
*/
- public static void startInteractionForTextMessage(Activity activity, Uri uri) {
+ public static void startInteractionForTextMessage(TransactionSafeActivity activity, Uri uri) {
(new PhoneNumberInteraction(activity, InteractionType.SMS, null)).startInteraction(uri);
}