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
This commit is contained in:
Salvador Martinez
2016-12-16 17:34:11 -08:00
parent d9d463be8b
commit d7fe1fb958
3 changed files with 25 additions and 9 deletions

View File

@@ -20,6 +20,7 @@ import android.content.BroadcastReceiver;
import android.content.Context; import android.content.Context;
import android.content.IntentFilter; import android.content.IntentFilter;
import android.support.annotation.Nullable; 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. * 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)}. * after a call to {@link #createAndRegisterReceiver(Activity)}.
* @param activity The activity that was used to register the BroadcastReceiver. * @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);
}
} }

View File

@@ -74,9 +74,8 @@ public class SurveyMixin implements LifecycleObserver, OnResume, OnPause {
public void onPause() { public void onPause() {
Activity activity = mFragment.getActivity(); Activity activity = mFragment.getActivity();
if (mReceiver != null && activity != null) { if (mReceiver != null && activity != null) {
SurveyFeatureProvider provider = SurveyFeatureProvider.unregisterReceiver(activity, mReceiver);
FeatureFactory.getFactory(activity).getSurveyFeatureProvider(activity); mReceiver = null;
provider.unregisterReceiver(activity, mReceiver);
} }
} }
} }

View File

@@ -1,5 +1,6 @@
package com.android.settings.survey; package com.android.settings.survey;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Matchers.any; import static org.mockito.Matchers.any;
import static org.mockito.Matchers.eq; import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
@@ -11,11 +12,15 @@ import static org.mockito.Mockito.when;
import android.app.Activity; import android.app.Activity;
import android.content.BroadcastReceiver; import android.content.BroadcastReceiver;
import android.content.Context; import android.content.Context;
import android.content.IntentFilter;
import android.support.v4.content.LocalBroadcastManager;
import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.SettingsRobolectricTestRunner;
import com.android.settings.TestConfig; import com.android.settings.TestConfig;
import com.android.settings.core.InstrumentedPreferenceFragment; import com.android.settings.core.InstrumentedPreferenceFragment;
import com.android.settings.overlay.SurveyFeatureProvider; import com.android.settings.overlay.SurveyFeatureProvider;
import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.FakeFeatureFactory;
import java.util.ArrayList;
import java.util.HashMap;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
@@ -24,6 +29,7 @@ import org.mockito.MockitoAnnotations;
import org.robolectric.Robolectric; import org.robolectric.Robolectric;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import org.robolectric.util.ReflectionHelpers;
@RunWith(SettingsRobolectricTestRunner.class) @RunWith(SettingsRobolectricTestRunner.class)
@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
@@ -102,7 +108,7 @@ public class SurveyMixinTest {
} }
@Test @Test
public void onPause_removesReceiverWhenInstantiated() { public void onPause_removesReceiverIfPreviouslySet() {
// Pretend there is a survey in memory // Pretend there is a survey in memory
when(mProvider.getSurveyExpirationDate(any(), any())).thenReturn(-1L); when(mProvider.getSurveyExpirationDate(any(), any())).thenReturn(-1L);
@@ -110,12 +116,16 @@ public class SurveyMixinTest {
Activity temp = Robolectric.setupActivity(Activity.class); Activity temp = Robolectric.setupActivity(Activity.class);
when(mFragment.getActivity()).thenReturn(temp); when(mFragment.getActivity()).thenReturn(temp);
when(mProvider.createAndRegisterReceiver(any())).thenReturn(mReceiver); when(mProvider.createAndRegisterReceiver(any())).thenReturn(mReceiver);
LocalBroadcastManager manager = LocalBroadcastManager.getInstance(temp);
SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY); SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY);
mixin.onResume(); mixin.onResume();
manager.registerReceiver(mReceiver, new IntentFilter());
mixin.onPause(); mixin.onPause();
// Verify we remove the receiver // Verify we remove the receiver
verify(mProvider, times(1)).unregisterReceiver(any(), any()); HashMap<BroadcastReceiver, ArrayList<IntentFilter>> map =
ReflectionHelpers.getField(manager, "mReceivers");
assertThat(map.containsKey(mReceiver)).isFalse();
} }
@Test @Test
@@ -129,15 +139,15 @@ public class SurveyMixinTest {
SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY); SurveyMixin mixin = new SurveyMixin(mFragment, FAKE_KEY);
mixin.onPause(); mixin.onPause();
// Verify we do nothing // Verify we do nothing;
verify(mProvider, never()).unregisterReceiver(any(), any()); verify(mProvider, never()).showSurveyIfAvailable(any(), any());
// pretend the activity died before onPause // pretend the activity died before onPause
when(mFragment.getActivity()).thenReturn(null); when(mFragment.getActivity()).thenReturn(null);
mixin.onPause(); mixin.onPause();
// Verify we do nothing // Verify we do nothing
verify(mProvider, never()).unregisterReceiver(any(), any()); verify(mProvider, never()).showSurveyIfAvailable(any(), any());
} }
} }