From ce9a126add91ea26207b55e1734f43b4f44f2a33 Mon Sep 17 00:00:00 2001 From: Antony Sargent Date: Mon, 4 Mar 2019 13:21:20 -0800 Subject: [PATCH] Prevent extra reloads in mobile networking page/controllers On the mobile network details page, as well as in several preference controllers used in various page, we had listeners for the ACTION_RADIO_TECHNOLOGY_CHANGED broadcast that when fired would cause a reload. It turns out that this gets broadcast as a sticky intent, so our callbaks would fire just after registering to listen, resulting in lots of unnecessary extra reloading. This was particularly noticable on the mobile network details page because the entire page gets reloaded. The fix is to make our listeners ignore the broadcast if it's the initial sticky one. Bug: 126419558 Test: make RunSettingsRoboTests Change-Id: I6ab7b43d74b07a839e45ce5368e45809be658b9d --- .../network/SubscriptionsChangeListener.java | 4 +- .../telephony/MobileNetworkActivity.java | 48 ++++++++++++++----- .../SubscriptionsChangeListenerTest.java | 16 +++++++ .../telephony/MobileNetworkActivityTest.java | 29 +++++++++-- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/com/android/settings/network/SubscriptionsChangeListener.java b/src/com/android/settings/network/SubscriptionsChangeListener.java index c3bb22bbb62..1ecd7705405 100644 --- a/src/com/android/settings/network/SubscriptionsChangeListener.java +++ b/src/com/android/settings/network/SubscriptionsChangeListener.java @@ -59,7 +59,9 @@ public class SubscriptionsChangeListener extends ContentObserver { mBroadcastReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - subscriptionsChangedCallback(); + if (!isInitialStickyBroadcast()) { + subscriptionsChangedCallback(); + } } }; } diff --git a/src/com/android/settings/network/telephony/MobileNetworkActivity.java b/src/com/android/settings/network/telephony/MobileNetworkActivity.java index 3c0940dbea5..47eb66be400 100644 --- a/src/com/android/settings/network/telephony/MobileNetworkActivity.java +++ b/src/com/android/settings/network/telephony/MobileNetworkActivity.java @@ -85,7 +85,14 @@ public class MobileNetworkActivity extends SettingsBaseActivity { setContentView(R.layout.mobile_network_settings_container); } setActionBar(findViewById(R.id.mobile_action_bar)); - mPhoneChangeReceiver = new PhoneChangeReceiver(); + mPhoneChangeReceiver = new PhoneChangeReceiver(this, () -> { + if (mCurSubscriptionId != SUB_ID_NULL) { + // When the radio changes (ex: CDMA->GSM), refresh the fragment. + // This is very rare. + switchFragment(new MobileNetworkSettings(), mCurSubscriptionId, + true /* forceUpdate */); + } + }); mSubscriptionManager = getSystemService(SubscriptionManager.class); mSubscriptionInfos = mSubscriptionManager.getActiveSubscriptionInfoList(true); mCurSubscriptionId = savedInstanceState != null @@ -103,16 +110,14 @@ public class MobileNetworkActivity extends SettingsBaseActivity { @Override protected void onStart() { super.onStart(); - final IntentFilter intentFilter = new IntentFilter( - TelephonyIntents.ACTION_RADIO_TECHNOLOGY_CHANGED); - registerReceiver(mPhoneChangeReceiver, intentFilter); + mPhoneChangeReceiver.register(); mSubscriptionManager.addOnSubscriptionsChangedListener(mOnSubscriptionsChangeListener); } @Override protected void onStop() { super.onStop(); - unregisterReceiver(mPhoneChangeReceiver); + mPhoneChangeReceiver.unregister(); mSubscriptionManager.removeOnSubscriptionsChangedListener(mOnSubscriptionsChangeListener); } @@ -235,14 +240,35 @@ public class MobileNetworkActivity extends SettingsBaseActivity { return MOBILE_SETTINGS_TAG + subscriptionId; } - private class PhoneChangeReceiver extends BroadcastReceiver { + @VisibleForTesting + static class PhoneChangeReceiver extends BroadcastReceiver { + private static final IntentFilter RADIO_TECHNOLOGY_CHANGED_FILTER = new IntentFilter( + TelephonyIntents.ACTION_RADIO_TECHNOLOGY_CHANGED); + + private Context mContext; + private Client mClient; + + interface Client { + void onPhoneChange(); + } + + public PhoneChangeReceiver(Context context, Client client) { + mContext = context; + mClient = client; + } + + public void register() { + mContext.registerReceiver(this, RADIO_TECHNOLOGY_CHANGED_FILTER); + } + + public void unregister() { + mContext.unregisterReceiver(this); + } + @Override public void onReceive(Context context, Intent intent) { - // When the radio changes (ex: CDMA->GSM), refresh the fragment. - // This is very rare to happen. - if (mCurSubscriptionId != SUB_ID_NULL) { - switchFragment(new MobileNetworkSettings(), mCurSubscriptionId, - true /* forceUpdate */); + if (!isInitialStickyBroadcast()) { + mClient.onPhoneChange(); } } } diff --git a/tests/robotests/src/com/android/settings/network/SubscriptionsChangeListenerTest.java b/tests/robotests/src/com/android/settings/network/SubscriptionsChangeListenerTest.java index 79d3fdc0ed5..9f302dfdee4 100644 --- a/tests/robotests/src/com/android/settings/network/SubscriptionsChangeListenerTest.java +++ b/tests/robotests/src/com/android/settings/network/SubscriptionsChangeListenerTest.java @@ -23,17 +23,20 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.BroadcastReceiver; import android.content.ContentResolver; import android.content.Context; +import android.content.Intent; import android.database.ContentObserver; import android.net.Uri; import android.provider.Settings; import android.telephony.SubscriptionManager; +import com.android.internal.telephony.TelephonyIntents; import com.android.settings.network.SubscriptionsChangeListener.SubscriptionsChangeListenerClient; import org.junit.Before; @@ -94,6 +97,19 @@ public class SubscriptionsChangeListenerTest { verify(mClient).onSubscriptionsChanged(); } + @Test + public void + onSubscriptionsChangedEvent_ignoresStickyBroadcastFromBeforeRegistering() { + final Intent intent = new Intent(TelephonyIntents.ACTION_RADIO_TECHNOLOGY_CHANGED); + mContext.sendStickyBroadcast(intent); + + initListener(true); + verify(mClient, never()).onSubscriptionsChanged(); + + mContext.sendStickyBroadcast(intent); + verify(mClient, times(1)).onSubscriptionsChanged(); + } + @Test public void onSubscriptionsChangedEvent_radioTechnologyChangedBroadcast_eventDeliveredToUs() { initListener(true); diff --git a/tests/robotests/src/com/android/settings/network/telephony/MobileNetworkActivityTest.java b/tests/robotests/src/com/android/settings/network/telephony/MobileNetworkActivityTest.java index 3b5cdf9a390..68f8c912972 100644 --- a/tests/robotests/src/com/android/settings/network/telephony/MobileNetworkActivityTest.java +++ b/tests/robotests/src/com/android/settings/network/telephony/MobileNetworkActivityTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.app.Activity; import android.content.Context; import android.content.Intent; import android.os.Bundle; @@ -35,10 +36,7 @@ import android.telephony.SubscriptionManager; import android.view.Menu; import android.view.View; -import androidx.fragment.app.Fragment; -import androidx.fragment.app.FragmentManager; -import androidx.fragment.app.FragmentTransaction; - +import com.android.internal.telephony.TelephonyIntents; import com.android.internal.view.menu.ContextMenuBuilder; import com.android.settings.R; @@ -56,6 +54,10 @@ import org.robolectric.RuntimeEnvironment; import java.util.ArrayList; import java.util.List; +import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentManager; +import androidx.fragment.app.FragmentTransaction; + @RunWith(RobolectricTestRunner.class) public class MobileNetworkActivityTest { @@ -141,6 +143,25 @@ public class MobileNetworkActivityTest { MOBILE_SETTINGS_TAG + CURRENT_SUB_ID); } + @Test + public void phoneChangeReceiver_ignoresStickyBroadcastFromBeforeRegistering() { + Activity activity = Robolectric.setupActivity(Activity.class); + final int[] onChangeCallbackCount = {0}; + MobileNetworkActivity.PhoneChangeReceiver receiver = + new MobileNetworkActivity.PhoneChangeReceiver(activity, () -> { + onChangeCallbackCount[0]++; + }); + Intent intent = new Intent(TelephonyIntents.ACTION_RADIO_TECHNOLOGY_CHANGED); + activity.sendStickyBroadcast(intent); + + receiver.register(); + assertThat(onChangeCallbackCount[0]).isEqualTo(0); + + activity.sendStickyBroadcast(intent); + assertThat(onChangeCallbackCount[0]).isEqualTo(1); + } + + @Test public void getSubscriptionId_hasIntent_getIdFromIntent() { final Intent intent = new Intent();