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

Commit 476e5324 authored by Nathan Harold's avatar Nathan Harold
Browse files

SST - Remove Double-Cached Previous SubId 

For reasons that are a little unclear, a change of subId was
causing an atomic integer to be set, presumably because the
only OnSubscriptionsChangedListener didn't allow callbacks
on a particular thread, which created races. This patch
serializes updates to the subId caching, removing the atomic
in the middle entirely, and ensuring that updates to the
current and previous subId values in the SST class are done
on the Handler (and consequently thread) of the SST.

Logic is accordingly refactored slightly to simplify the
cascading assignment of curSubId -> prevSubId and then
newSubId -> curSubId.

Bug: 160681475
Test: atest ServiceStateTrackerTest
Test: manual (steps from b/160681475)
      1) selected a forbidden PLMN
      2) waited for a notification
      3) removed SIM and verified notification gone
Change-Id: Ic310a216c64f8f2f9c18c26f4300d9d100327a6c
parent 18c3831f

src/java/com/android/internal/telephony/ServiceStateTracker.java

+73 −65
Original line number Diff line number Diff line
@@ -126,7 +126,6 @@ import java.util.LinkedList; 
import java.util.List;

import java.util.Set;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.atomic.AtomicInteger;

import java.util.regex.Matcher;

import java.util.regex.Pattern;

import java.util.regex.PatternSyntaxException;
@@ -369,8 +368,6 @@ public class ServiceStateTracker extends Handler { 
    private Pattern mOperatorNameStringPattern;



    private class SstSubscriptionsChangedListener extends OnSubscriptionsChangedListener {

        public final AtomicInteger mPreviousSubId =

                new AtomicInteger(SubscriptionManager.INVALID_SUBSCRIPTION_ID);



        /**

         * Callback invoked when there is any change to any SubscriptionInfo. Typically
@@ -379,19 +376,36 @@ public class ServiceStateTracker extends Handler { 
        @Override

        public void onSubscriptionsChanged() {

            if (DBG) log("SubscriptionListener.onSubscriptionInfoChanged");

            // Set the network type, in case the radio does not restore it.

            int subId = mPhone.getSubId();

            ServiceStateTracker.this.mPrevSubId = mPreviousSubId.get();

            if (mPreviousSubId.getAndSet(subId) != subId) {

                if (SubscriptionManager.isValidSubscriptionId(subId)) {



            final int curSubId = mPhone.getSubId();



            // If the sub info changed, but the subId is the same, then we're done.

            if (mSubId == curSubId) return;



            // If not, then the subId has changed, so we need to remember the old subId,

            // even if the new subId is invalid (likely).

            mPrevSubId = mSubId;

            mSubId = curSubId;



            // Update voicemail count and notify message waiting changed regardless of

            // whether the new subId is valid. This is an exception to the general logic

            // of only updating things if the new subscription is valid. The result is that

            // VoiceMail counts (and UI indicators) are cleared when the SIM is removed,

            // which seems desirable.

            mPhone.updateVoiceMail();



            // If the new subscription ID isn't valid, then we don't need to do all the

            // UI updating, so we're done.

            if (!SubscriptionManager.isValidSubscriptionId(mSubId)) return;



            Context context = mPhone.getContext();



            mPhone.notifyPhoneStateChanged();

            mPhone.notifyCallForwardingIndicator();

                    if (!SubscriptionManager.isValidSubscriptionId(

                            ServiceStateTracker.this.mPrevSubId)) {



            if (!SubscriptionManager.isValidSubscriptionId(mPrevSubId)) {

                // just went from invalid to valid subId, so notify with current service

                        // state in case our service stat was never broadcasted (we don't notify

                // state in case our service state was never broadcasted (we don't notify

                // service states when the subId is invalid)

                mPhone.notifyServiceStateChanged(mSS);

            }
@@ -419,15 +433,15 @@ public class ServiceStateTracker extends Handler { 
                    Phone.NETWORK_SELECTION_NAME_KEY, "");

            String oldNetworkSelectionShort = sp.getString(

                    Phone.NETWORK_SELECTION_SHORT_KEY, "");

                    if (!TextUtils.isEmpty(oldNetworkSelection) ||

                            !TextUtils.isEmpty(oldNetworkSelectionName) ||

                            !TextUtils.isEmpty(oldNetworkSelectionShort)) {

            if (!TextUtils.isEmpty(oldNetworkSelection)

                    || !TextUtils.isEmpty(oldNetworkSelectionName)

                    || !TextUtils.isEmpty(oldNetworkSelectionShort)) {

                SharedPreferences.Editor editor = sp.edit();

                        editor.putString(Phone.NETWORK_SELECTION_KEY + subId,

                editor.putString(Phone.NETWORK_SELECTION_KEY + mSubId,

                        oldNetworkSelection);

                        editor.putString(Phone.NETWORK_SELECTION_NAME_KEY + subId,

                editor.putString(Phone.NETWORK_SELECTION_NAME_KEY + mSubId,

                        oldNetworkSelectionName);

                        editor.putString(Phone.NETWORK_SELECTION_SHORT_KEY + subId,

                editor.putString(Phone.NETWORK_SELECTION_SHORT_KEY + mSubId,

                        oldNetworkSelectionShort);

                editor.remove(Phone.NETWORK_SELECTION_KEY);

                editor.remove(Phone.NETWORK_SELECTION_NAME_KEY);
@@ -440,10 +454,6 @@ public class ServiceStateTracker extends Handler { 
            // MobileSignalController earlier were actually ignored due to invalid sub id.

            updateSpnDisplay();

        }

                // update voicemail count and notify message waiting changed

                mPhone.updateVoiceMail();

            }

        }

    };



    //Common
@@ -630,8 +640,8 @@ public class ServiceStateTracker extends Handler { 


        mSubscriptionController = SubscriptionController.getInstance();

        mSubscriptionManager = SubscriptionManager.from(phone.getContext());

        mSubscriptionManager

                .addOnSubscriptionsChangedListener(mOnSubscriptionsChangedListener);

        mSubscriptionManager.addOnSubscriptionsChangedListener(

                new android.os.HandlerExecutor(this), mOnSubscriptionsChangedListener);

        mRestrictedState = new RestrictedState();



        mTransportManager = mPhone.getTransportManager();
@@ -1251,10 +1261,8 @@ public class ServiceStateTracker extends Handler { 


            // GSM

            case EVENT_SIM_READY:

                // Reset the mPreviousSubId so we treat a SIM power bounce

                // Reset the mPrevSubId so we treat a SIM power bounce

                // as a first boot.  See b/19194287

                mOnSubscriptionsChangedListener.mPreviousSubId.set(

                        SubscriptionManager.INVALID_SUBSCRIPTION_ID);

                mPrevSubId = SubscriptionManager.INVALID_SUBSCRIPTION_ID;

                mIsSimReady = true;

                pollStateInternal(false);