Fix Developer options page memory leak

Change-Id: I12d53c41adee56850cd294ae89e7a56328639558
Fixes: 117583710
Test: manual
Test: make RunSettingsRoboTests -j ROBOTEST_FILTER=com.android.settings.development

diff --git a/src/com/android/settings/development/DevelopmentSettingsDashboardFragment.java b/src/com/android/settings/development/DevelopmentSettingsDashboardFragment.java
index 5f42e89..dbcb389 100644 (file)
--- a/src/com/android/settings/development/DevelopmentSettingsDashboardFragment.java
+++ b/src/com/android/settings/development/DevelopmentSettingsDashboardFragment.java
@@ -470,7 +470,7 @@ public class DevelopmentSettingsDashboardFragment extends RestrictedDashboardFra
         controllers.add(new DefaultLaunchPreferenceController(context, "density"));
         controllers.add(new DefaultLaunchPreferenceController(context, "background_check"));
         controllers.add(new DefaultLaunchPreferenceController(context, "inactive_apps"));
-        controllers.add(new AutofillLoggingLevelPreferenceController(context));
+        controllers.add(new AutofillLoggingLevelPreferenceController(context, lifecycle));
         controllers.add(new AutofillResetOptionsPreferenceController(context));
         return controllers;
     }

diff --git a/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceController.java b/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceController.java
index 8c739e3..f955f5e 100644 (file)
--- a/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceController.java
+++ b/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceController.java
@@ -27,11 +27,15 @@ import androidx.preference.Preference;
 
 import com.android.settings.R;
 import com.android.settings.core.PreferenceControllerMixin;
+import com.android.settingslib.core.lifecycle.Lifecycle;
+import com.android.settingslib.core.lifecycle.LifecycleObserver;
+import com.android.settingslib.core.lifecycle.events.OnDestroy;
 import com.android.settingslib.development.DeveloperOptionsPreferenceController;
 
 public final class AutofillLoggingLevelPreferenceController
         extends DeveloperOptionsPreferenceController
-        implements PreferenceControllerMixin, Preference.OnPreferenceChangeListener {
+        implements PreferenceControllerMixin, Preference.OnPreferenceChangeListener,
+        LifecycleObserver, OnDestroy {
 
     private static final String TAG = "AutofillLoggingLevelPreferenceController";
     private static final String AUTOFILL_LOGGING_LEVEL_KEY = "autofill_logging_level";
@@ -40,7 +44,7 @@ public final class AutofillLoggingLevelPreferenceController
     private final String[] mListSummaries;
     private final AutofillDeveloperSettingsObserver mObserver;
 
-    public AutofillLoggingLevelPreferenceController(Context context) {
+    public AutofillLoggingLevelPreferenceController(Context context, Lifecycle lifecycle) {
         super(context);
 
         Resources resources = context.getResources();
@@ -48,7 +52,15 @@ public final class AutofillLoggingLevelPreferenceController
         mListSummaries = resources.getStringArray(R.array.autofill_logging_level_entries);
         mObserver = new AutofillDeveloperSettingsObserver(mContext, () -> updateOptions());
         mObserver.register();
-        // TODO: there should be a hook on AbstractPreferenceController where we could unregister it
+
+        if (lifecycle != null) {
+            lifecycle.addObserver(this);
+        }
+    }
+
+    @Override
+    public void onDestroy() {
+        mObserver.unregister();
     }
 
     @Override

diff --git a/tests/robotests/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceControllerTest.java
index 2e538e0..1268797 100644 (file)
--- a/tests/robotests/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceControllerTest.java
+++ b/tests/robotests/src/com/android/settings/development/autofill/AutofillLoggingLevelPreferenceControllerTest.java
@@ -16,6 +16,9 @@
 
 package com.android.settings.development.autofill;
 
+import static androidx.lifecycle.Lifecycle.Event.ON_CREATE;
+import static androidx.lifecycle.Lifecycle.Event.ON_DESTROY;
+
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.Mockito.verify;
@@ -25,18 +28,25 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.view.autofill.AutofillManager;
 
+import androidx.lifecycle.LifecycleOwner;
 import androidx.preference.ListPreference;
 import androidx.preference.PreferenceScreen;
 
 import com.android.settings.R;
 import com.android.settings.testutils.SettingsRobolectricTestRunner;
+import com.android.settingslib.core.lifecycle.Lifecycle;
 
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
+import org.robolectric.annotation.Resetter;
 
 @RunWith(SettingsRobolectricTestRunner.class)
 public class AutofillLoggingLevelPreferenceControllerTest {
@@ -57,20 +67,30 @@ public class AutofillLoggingLevelPreferenceControllerTest {
     private String[] mListValues;
     private String[] mListSummaries;
 
+    private LifecycleOwner mLifecycleOwner;
+    private Lifecycle mLifecycle;
+
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this); // TODO: use @Rule
+        mLifecycleOwner = () -> mLifecycle;
+        mLifecycle = new Lifecycle(mLifecycleOwner);
         mContext = RuntimeEnvironment.application;
         mHelper = new AutofillTestingHelper(mContext);
         final Resources resources = mContext.getResources();
         mListValues = resources.getStringArray(R.array.autofill_logging_level_values);
         mListSummaries = resources.getStringArray(R.array.autofill_logging_level_entries);
-        mController = new AutofillLoggingLevelPreferenceController(mContext);
+        mController = new AutofillLoggingLevelPreferenceController(mContext, mLifecycle);
         when(mPreferenceScreen.findPreference(mController.getPreferenceKey()))
-            .thenReturn(mPreference);
+                .thenReturn(mPreference);
         mController.displayPreference(mPreferenceScreen);
     }
 
+    @After
+    public void tearDown() {
+        ShadowAutofillLoggingLevelPreferenceController.reset();
+    }
+
     @Test
     public void handlePreferenceTreeClick_differentPreferenceKey_shouldNotTrigger()
             throws Exception {
@@ -141,4 +161,28 @@ public class AutofillLoggingLevelPreferenceControllerTest {
         verify(mPreference).setValue(mListValues[IDX_VERBOSE]);
         verify(mPreference).setSummary(mListSummaries[IDX_VERBOSE]);
     }
+
+    @Test
+    @Config(shadows = ShadowAutofillLoggingLevelPreferenceController.class)
+    public void onDestory_shouldUnregisterObserver() {
+        mLifecycle.handleLifecycleEvent(ON_CREATE);
+        mLifecycle.handleLifecycleEvent(ON_DESTROY);
+
+        assertThat(ShadowAutofillLoggingLevelPreferenceController.isUnregisterObserver).isTrue();
+    }
+
+    @Implements(AutofillLoggingLevelPreferenceController.class)
+    public static class ShadowAutofillLoggingLevelPreferenceController {
+        private static boolean isUnregisterObserver = false;
+
+        @Implementation
+        public void onDestroy() {
+            isUnregisterObserver = true;
+        }
+
+        @Resetter
+        public static void reset() {
+            isUnregisterObserver = false;
+        }
+    }
 }