commit 2cacc55964e737078f982e19dcda74439eabb932
author Nicolas Catania <niko@google.com> Wed Mar 10 10:41:48 2010 -0800
committer Nicolas Catania <niko@google.com> Fri Mar 12 11:16:55 2010 -0800
tree ed4e429b3eea8e7fb17734ffeaa7898adc8437db
parent 9b75f74cd796543e6e88984984f19fd0c2cf9f38

Fixed crash Unable to add window

Renamed onStop to onPause. onStop is not guaranteed to be called,
which is probably why we have some extra code duplication in the
finalize() method.

Added a new onDestroy to clean up all the pending callbacks.
At this stage, there is no window anymore and we should not
call showDialog.

Release the Handler in onDestroy.
Not doing so has been the source of memory leaks in the past.

Bug: 2501845
Change-Id: I35125fda1b53a6996b933056bafe2f0c04aea80a

src/com/android/contacts/ImportVCardActivity.java

diff --git a/src/com/android/contacts/ImportVCardActivity.java b/src/com/android/contacts/ImportVCardActivity.java
index 30b6cbf..d91bb5b 100644
--- a/src/com/android/contacts/ImportVCardActivity.java
+++ b/src/com/android/contacts/ImportVCardActivity.java
@@ -104,7 +104,7 @@
     private static final String LOG_TAG = "ImportVCardActivity";
     private static final boolean DO_PERFORMANCE_PROFILE = false;
 
-    private Handler mHandler = new Handler();
+    private Handler mHandler = new Handler();  // Run on the UI thread.
     private Account mAccount;
 
     private ProgressDialog mProgressDialogForScanVCard;
@@ -118,6 +118,7 @@
 
     private boolean mNeedReview = false;
 
+    // Runs on the UI thread.
     private class DialogDisplayer implements Runnable {
         private final int mResId;
         public DialogDisplayer(int resId) {
@@ -128,10 +129,7 @@
             mErrorMessage = errorMessage;
         }
         public void run() {
-            // Show the Dialog only when the parent Activity is still alive.
-            if (!ImportVCardActivity.this.isFinishing()) {
-                showDialog(mResId);
-            }
+            showDialog(mResId);
         }
     }
 
@@ -828,8 +826,8 @@
     }
 
     @Override
-    protected void onStop() {
-        super.onStop();
+    protected void onPause() {
+        super.onPause();
         if (mVCardReadThread != null) {
             // The Activity is no longer visible. Stop the thread.
             mVCardReadThread.cancel();
@@ -837,7 +835,7 @@
         }
 
         // ImportVCardActivity should not be persistent. In other words, if there's some
-        // event calling onStop(), this Activity should finish its work and give the main
+        // event calling onPause(), this Activity should finish its work and give the main
         // screen back to the caller Activity.
         if (!isFinishing()) {
             finish();
@@ -845,10 +843,27 @@
     }
 
     @Override
+    protected void onDestroy() {
+        // The code assumes the handler runs on the UI thread. If not,
+        // clearing the message queue is not enough, one would have to
+        // make sure that the handler does not run any callback when
+        // this activity isFinishing().
+
+        // Callbacks messages have what == 0.
+        if (mHandler.hasMessages(0)) {
+            mHandler.removeMessages(0);
+        }
+
+        mHandler = null;  // Prevents memory leaks by breaking any circular dependency.
+        super.onDestroy();
+    }
+
+    @Override
     public void finalize() {
+        // TODO: This should not be needed. Throw exception instead.
         if (mVCardReadThread != null) {
             // Not sure this procedure is really needed, but just in case...
-            Log.w(LOG_TAG, "VCardReadThread exists while this Activity is now being killed!");
+            Log.e(LOG_TAG, "VCardReadThread exists while this Activity is now being killed!");
             mVCardReadThread.cancel();
             mVCardReadThread = null;
         }