From d7fe1fb958ab6e46bd3cc31b1137f0bf499ad55f Mon Sep 17 00:00:00 2001 From: Salvador Martinez Date: Fri, 16 Dec 2016 17:34:11 -0800 Subject: [PATCH] Fix null pointer exception when surveys are turned off Under certain conditions the provider could be null on download completion which could lead to a NPE if surveys are disabled. The method to remove the receiver has been made static so an instance is no longer required. Test: robotests Bug: 33707203 Change-Id: Icfb545697f24172db734dd7dad421796edf68186 --- .../overlay/SurveyFeatureProvider.java | 9 ++++++++- .../android/settings/survey/SurveyMixin.java | 5 ++--- .../settings/survey/SurveyMixinTest.java | 20 ++++++++++++++----- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/overlay/SurveyFeatureProvider.java b/src/com/android/settings/overlay/SurveyFeatureProvider.java index 907bed8f7bf..416a602bdb1 100644 --- a/src/com/android/settings/overlay/SurveyFeatureProvider.java +++ b/src/com/android/settings/overlay/SurveyFeatureProvider.java @@ -20,6 +20,7 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.IntentFilter; import android.support.annotation.Nullable; +import android.support.v4.content.LocalBroadcastManager; /** * An interface for classes wishing to provide the ability to serve surveys to implement. @@ -81,5 +82,11 @@ public interface SurveyFeatureProvider { * after a call to {@link #createAndRegisterReceiver(Activity)}. * @param activity The activity that was used to register the BroadcastReceiver. */ - void unregisterReceiver(Activity activity, BroadcastReceiver receiver); + static void unregisterReceiver(Activity activity, BroadcastReceiver receiver) { + if (activity == null) { + throw new IllegalStateException("Cannot unregister receiver if activity is null"); + } + + LocalBroadcastManager.getInstance(activity).unregisterReceiver(receiver); + } } diff --git a/src/com/android/settings/survey/SurveyMixin.java b/src/com/android/settings/survey/SurveyMixin.java index 74b7f1024cf..72cd634c851 100644 --- a/src/com/android/settings/survey/SurveyMixin.java +++ b/src/com/android/settings/survey/SurveyMixin.java @@ -74,9 +74,8 @@ public class SurveyMixin implements LifecycleObserver, OnResume, OnPause { public void onPause() { Activity activity = mFragment.getActivity(); if (mReceiver != null && activity != null) { - SurveyFeatureProvider provider = - FeatureFactory.getFactory(activity).getSurveyFeatureProvider(activity); - provider.unregisterReceiver(activity, mReceiver); + SurveyFeatureProvider.unregisterReceiver(activity, mReceiver); + mReceiver = null; } } } diff --git a/tests/robotests/src/com/android/settings/survey/SurveyMixinTest.java b/tests/robotests/src/com/android/settings/survey/SurveyMixinTest.java index 20c50cea7e4..73fe09e126c 100644 --- a/tests/robotests/src/com/android/settings/survey/SurveyMixinTest.java +++ b/tests/robotests/src/com/android/settings/survey/SurveyMixinTest.java @@ -1,5 +1,6 @@ package com.android.settings.survey; +import static com.google.common.truth.Truth.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.never; @@ -11,11 +12,15 @@ import static org.mockito.Mockito.when; import android.app.Activity; import android.content.BroadcastReceiver; import android.content.Context; +import android.content.IntentFilter; +import android.support.v4.content.LocalBroadcastManager; import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.core.InstrumentedPreferenceFragment; import com.android.settings.overlay.SurveyFeatureProvider; import com.android.settings.testutils.FakeFeatureFactory; +import java.util.ArrayList; +import java.util.HashMap; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,6 +29,7 @@ import org.mockito.MockitoAnnotations; import org.robolectric.Robolectric; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; +import org.robolectric.util.ReflectionHelpers; @RunWith(SettingsRobolectricTestRunner.class) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) @@ -102,7 +108,7 @@ public class SurveyMixinTest { } @Test - public void onPause_removesReceiverWhenInstantiated() { + public void onPause_removesReceiverIfPreviouslySet() { // Pretend there is a survey in memory when(mProvider.getSurveyExpirationDate(any(), any())).thenReturn(-1L); @@ -110,12 +116,16 @@ public class SurveyMixinTest { Activity temp = Robolectric.setupActivity(Activity.class); when(mFragment.getActivity()).thenReturn(temp); when(mProvider.createAndRegisterReceiver(any())).thenReturn(mReceiver); + LocalBroadcastManager manager = LocalBroadcastManager.getInstance(temp); SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY); mixin.onResume(); + manager.registerReceiver(mReceiver, new IntentFilter()); mixin.onPause(); // Verify we remove the receiver - verify(mProvider, times(1)).unregisterReceiver(any(), any()); + HashMap> map = + ReflectionHelpers.getField(manager, "mReceivers"); + assertThat(map.containsKey(mReceiver)).isFalse(); } @Test @@ -129,15 +139,15 @@ public class SurveyMixinTest { SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY); mixin.onPause(); - // Verify we do nothing - verify(mProvider, never()).unregisterReceiver(any(), any()); + // Verify we do nothing; + verify(mProvider, never()).showSurveyIfAvailable(any(), any()); // pretend the activity died before onPause when(mFragment.getActivity()).thenReturn(null); mixin.onPause(); // Verify we do nothing - verify(mProvider, never()).unregisterReceiver(any(), any()); + verify(mProvider, never()).showSurveyIfAvailable(any(), any()); } }