From 8b886fab5496b0b1f5193f21855220176deddc37 Mon Sep 17 00:00:00 2001 From: Jim Miller Date: Thu, 13 Jan 2011 17:56:35 -0800 Subject: [PATCH] Fix 3106227: use WeakReferences for receivers in DigitalClock class This works around a bug in the framework where LockScreen wouldn't get GC'd under certain circumstances which would lead to an OOM crash. It now uses WeakReferences for observers inside the DigitalClock container class and unregisters them if the containing DigitalClock goes away. Also removed mLive variable which was unused and could potentially leak the receivers. Left mAttached for debugging so we can use it to determine if the calls to onAttachToWindow() and onDetachFromWindow() are grossly unbalanced which may be the root cause of the original problem. Have cleanUp() explicitly clear unused references to make tracing through hprof references easier. Change-Id: I99a7e0c356001b05eab5aa729564553666febfea --- .../android/internal/widget/DigitalClock.java | 103 +++++++++++------- .../policy/impl/AccountUnlockScreen.java | 9 +- .../policy/impl/KeyguardViewManager.java | 2 +- .../policy/impl/LockPatternKeyguardView.java | 2 + .../internal/policy/impl/LockScreen.java | 15 ++- .../policy/impl/PatternUnlockScreen.java | 9 +- 6 files changed, 87 insertions(+), 53 deletions(-) diff --git a/core/java/com/android/internal/widget/DigitalClock.java b/core/java/com/android/internal/widget/DigitalClock.java index fa47ff638cd51..303a1bfe077f3 100644 --- a/core/java/com/android/internal/widget/DigitalClock.java +++ b/core/java/com/android/internal/widget/DigitalClock.java @@ -22,7 +22,6 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.res.Resources; import android.database.ContentObserver; import android.graphics.Typeface; import android.os.Handler; @@ -33,6 +32,7 @@ import android.view.View; import android.widget.LinearLayout; import android.widget.TextView; +import java.lang.ref.WeakReference; import java.text.DateFormatSymbols; import java.util.Calendar; @@ -49,26 +49,41 @@ public class DigitalClock extends LinearLayout { private TextView mTimeDisplay; private AmPm mAmPm; private ContentObserver mFormatChangeObserver; - private boolean mLive = true; - private boolean mAttached; + private int mAttached = 0; // for debugging - tells us whether attach/detach is unbalanced /* called by system on minute ticks */ private final Handler mHandler = new Handler(); - private final BroadcastReceiver mIntentReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - if (mLive && intent.getAction().equals( - Intent.ACTION_TIMEZONE_CHANGED)) { - mCalendar = Calendar.getInstance(); - } - // Post a runnable to avoid blocking the broadcast. - mHandler.post(new Runnable() { - public void run() { - updateTime(); + private BroadcastReceiver mIntentReceiver; + + private static class TimeChangedReceiver extends BroadcastReceiver { + private WeakReference mClock; + private Context mContext; + + public TimeChangedReceiver(DigitalClock clock) { + mClock = new WeakReference(clock); + mContext = clock.getContext(); + } + + @Override + public void onReceive(Context context, Intent intent) { + // Post a runnable to avoid blocking the broadcast. + final boolean timezoneChanged = + intent.getAction().equals(Intent.ACTION_TIMEZONE_CHANGED); + final DigitalClock clock = mClock.get(); + if (clock != null) { + clock.mHandler.post(new Runnable() { + public void run() { + if (timezoneChanged) { + clock.mCalendar = Calendar.getInstance(); } + clock.updateTime(); + } }); + } else { + mContext.unregisterReceiver(this); } - }; + } + }; static class AmPm { private TextView mAmPm; @@ -94,14 +109,23 @@ public class DigitalClock extends LinearLayout { } } - private class FormatChangeObserver extends ContentObserver { - public FormatChangeObserver() { + private static class FormatChangeObserver extends ContentObserver { + private WeakReference mClock; + private Context mContext; + public FormatChangeObserver(DigitalClock clock) { super(new Handler()); + mClock = new WeakReference(clock); + mContext = clock.getContext(); } @Override public void onChange(boolean selfChange) { - setDateFormat(); - updateTime(); + DigitalClock digitalClock = mClock.get(); + if (digitalClock != null) { + digitalClock.setDateFormat(); + digitalClock.updateTime(); + } else { + mContext.getContentResolver().unregisterContentObserver(this); + } } } @@ -129,11 +153,11 @@ public class DigitalClock extends LinearLayout { protected void onAttachedToWindow() { super.onAttachedToWindow(); - if (mAttached) return; - mAttached = true; + mAttached++; - if (mLive) { - /* monitor time ticks, time changed, timezone */ + /* monitor time ticks, time changed, timezone */ + if (mIntentReceiver == null) { + mIntentReceiver = new TimeChangedReceiver(this); IntentFilter filter = new IntentFilter(); filter.addAction(Intent.ACTION_TIME_TICK); filter.addAction(Intent.ACTION_TIME_CHANGED); @@ -142,9 +166,11 @@ public class DigitalClock extends LinearLayout { } /* monitor 12/24-hour display preference */ - mFormatChangeObserver = new FormatChangeObserver(); - mContext.getContentResolver().registerContentObserver( - Settings.System.CONTENT_URI, true, mFormatChangeObserver); + if (mFormatChangeObserver == null) { + mFormatChangeObserver = new FormatChangeObserver(this); + mContext.getContentResolver().registerContentObserver( + Settings.System.CONTENT_URI, true, mFormatChangeObserver); + } updateTime(); } @@ -153,16 +179,19 @@ public class DigitalClock extends LinearLayout { protected void onDetachedFromWindow() { super.onDetachedFromWindow(); - if (!mAttached) return; - mAttached = false; + mAttached--; - if (mLive) { + if (mIntentReceiver != null) { mContext.unregisterReceiver(mIntentReceiver); } - mContext.getContentResolver().unregisterContentObserver( - mFormatChangeObserver); - } + if (mFormatChangeObserver != null) { + mContext.getContentResolver().unregisterContentObserver( + mFormatChangeObserver); + } + mFormatChangeObserver = null; + mIntentReceiver = null; + } void updateTime(Calendar c) { mCalendar = c; @@ -170,9 +199,7 @@ public class DigitalClock extends LinearLayout { } private void updateTime() { - if (mLive) { - mCalendar.setTimeInMillis(System.currentTimeMillis()); - } + mCalendar.setTimeInMillis(System.currentTimeMillis()); CharSequence newTime = DateFormat.format(mFormat, mCalendar); mTimeDisplay.setText(newTime); @@ -180,12 +207,8 @@ public class DigitalClock extends LinearLayout { } private void setDateFormat() { - mFormat = android.text.format.DateFormat.is24HourFormat(getContext()) + mFormat = android.text.format.DateFormat.is24HourFormat(getContext()) ? M24 : M12; mAmPm.setShowAmPm(mFormat.equals(M12)); } - - void setLive(boolean live) { - mLive = live; - } } diff --git a/policy/src/com/android/internal/policy/impl/AccountUnlockScreen.java b/policy/src/com/android/internal/policy/impl/AccountUnlockScreen.java index 840c5e198e21c..c4feefd2e32c8 100644 --- a/policy/src/com/android/internal/policy/impl/AccountUnlockScreen.java +++ b/policy/src/com/android/internal/policy/impl/AccountUnlockScreen.java @@ -62,8 +62,8 @@ public class AccountUnlockScreen extends RelativeLayout implements KeyguardScree */ private static final int AWAKE_POKE_MILLIS = 30000; - private final KeyguardScreenCallback mCallback; - private final LockPatternUtils mLockPatternUtils; + private KeyguardScreenCallback mCallback; + private LockPatternUtils mLockPatternUtils; private KeyguardUpdateMonitor mUpdateMonitor; private TextView mTopHeader; @@ -159,7 +159,10 @@ public class AccountUnlockScreen extends RelativeLayout implements KeyguardScree if (mCheckingDialog != null) { mCheckingDialog.hide(); } - mUpdateMonitor.removeCallback(this); + mUpdateMonitor.removeCallback(this); // this must be first + mCallback = null; + mLockPatternUtils = null; + mUpdateMonitor = null; } /** {@inheritDoc} */ diff --git a/policy/src/com/android/internal/policy/impl/KeyguardViewManager.java b/policy/src/com/android/internal/policy/impl/KeyguardViewManager.java index ba1d7f5302946..708e89d252641 100644 --- a/policy/src/com/android/internal/policy/impl/KeyguardViewManager.java +++ b/policy/src/com/android/internal/policy/impl/KeyguardViewManager.java @@ -223,8 +223,8 @@ public class KeyguardViewManager implements KeyguardWindowController { mKeyguardHost.postDelayed(new Runnable() { public void run() { synchronized (KeyguardViewManager.this) { - mKeyguardHost.removeView(lastView); lastView.cleanUp(); + mKeyguardHost.removeView(lastView); } } }, 500); diff --git a/policy/src/com/android/internal/policy/impl/LockPatternKeyguardView.java b/policy/src/com/android/internal/policy/impl/LockPatternKeyguardView.java index 27706eff298d7..f3d07ab82b91e 100644 --- a/policy/src/com/android/internal/policy/impl/LockPatternKeyguardView.java +++ b/policy/src/com/android/internal/policy/impl/LockPatternKeyguardView.java @@ -495,8 +495,10 @@ public class LockPatternKeyguardView extends KeyguardViewBase { public void cleanUp() { ((KeyguardScreen) mLockScreen).onPause(); ((KeyguardScreen) mLockScreen).cleanUp(); + this.removeView(mLockScreen); ((KeyguardScreen) mUnlockScreen).onPause(); ((KeyguardScreen) mUnlockScreen).cleanUp(); + this.removeView(mUnlockScreen); } private boolean isSecure() { diff --git a/policy/src/com/android/internal/policy/impl/LockScreen.java b/policy/src/com/android/internal/policy/impl/LockScreen.java index 635b7d308a353..5a20b9392e0cc 100644 --- a/policy/src/com/android/internal/policy/impl/LockScreen.java +++ b/policy/src/com/android/internal/policy/impl/LockScreen.java @@ -55,9 +55,9 @@ class LockScreen extends LinearLayout implements KeyguardScreen, KeyguardUpdateM private Status mStatus = Status.Normal; - private final LockPatternUtils mLockPatternUtils; - private final KeyguardUpdateMonitor mUpdateMonitor; - private final KeyguardScreenCallback mCallback; + private LockPatternUtils mLockPatternUtils; + private KeyguardUpdateMonitor mUpdateMonitor; + private KeyguardScreenCallback mCallback; private TextView mCarrier; private SlidingTab mSelector; @@ -225,8 +225,8 @@ class LockScreen extends LinearLayout implements KeyguardScreen, KeyguardUpdateM setFocusableInTouchMode(true); setDescendantFocusability(ViewGroup.FOCUS_BLOCK_DESCENDANTS); - updateMonitor.registerInfoCallback(this); - updateMonitor.registerSimStateCallback(this); + mUpdateMonitor.registerInfoCallback(this); + mUpdateMonitor.registerSimStateCallback(this); mAudioManager = (AudioManager) getContext().getSystemService(Context.AUDIO_SERVICE); mSilentMode = isSilentMode(); @@ -668,7 +668,10 @@ class LockScreen extends LinearLayout implements KeyguardScreen, KeyguardUpdateM /** {@inheritDoc} */ public void cleanUp() { - mUpdateMonitor.removeCallback(this); + mUpdateMonitor.removeCallback(this); // this must be first + mLockPatternUtils = null; + mUpdateMonitor = null; + mCallback = null; } /** {@inheritDoc} */ diff --git a/policy/src/com/android/internal/policy/impl/PatternUnlockScreen.java b/policy/src/com/android/internal/policy/impl/PatternUnlockScreen.java index 418e243922795..ffd0a47418fd3 100644 --- a/policy/src/com/android/internal/policy/impl/PatternUnlockScreen.java +++ b/policy/src/com/android/internal/policy/impl/PatternUnlockScreen.java @@ -66,9 +66,9 @@ class PatternUnlockScreen extends LinearLayoutWithDefaultTouchRecepient private int mTotalFailedPatternAttempts = 0; private CountDownTimer mCountdownTimer = null; - private final LockPatternUtils mLockPatternUtils; - private final KeyguardUpdateMonitor mUpdateMonitor; - private final KeyguardScreenCallback mCallback; + private LockPatternUtils mLockPatternUtils; + private KeyguardUpdateMonitor mUpdateMonitor; + private KeyguardScreenCallback mCallback; /** * whether there is a fallback option available when the pattern is forgotten. @@ -478,6 +478,9 @@ class PatternUnlockScreen extends LinearLayoutWithDefaultTouchRecepient /** {@inheritDoc} */ public void cleanUp() { mUpdateMonitor.removeCallback(this); + mLockPatternUtils = null; + mUpdateMonitor = null; + mCallback = null; } @Override