Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ce9a126a authored by Antony Sargent's avatar Antony Sargent
Browse files

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
parent ca299cf2
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -59,8 +59,10 @@ public class SubscriptionsChangeListener extends ContentObserver {
        mBroadcastReceiver = new BroadcastReceiver() {
            @Override
            public void onReceive(Context context, Intent intent) {
                if (!isInitialStickyBroadcast()) {
                    subscriptionsChangedCallback();
                }
            }
        };
    }

+37 −11
Original line number Diff line number Diff line
@@ -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();
            }
        }
    }
+16 −0
Original line number Diff line number Diff line
@@ -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);
+25 −4
Original line number Diff line number Diff line
@@ -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();