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

Commit 4380918a authored by Malcolm Chen's avatar Malcolm Chen
Browse files

Avoid send back and forth message. 

If there's back to back setUserDataEnabled request to DcTracker with
opposite decision, it may form an infinite loop between DcTracker and
MultiSimSettingController such that data enablement keeps flickering.

Steps to repro:
1) DcTracker received user data enable to true and sends a message to
MultiSimSettingController about it.
2) DcTracker received user data enable to false and sends a message to
MultiSimSettingController about it.
3) MultiSimSettingController handles message of data turning on, which
in return turns on DcTracker if subscription is grouped, and queues a
message in MultiSimSettingController to turn it on.
4) MultiSimSettingController handles message of data turning off, which
in return turns off DcTracker if subscription is grouped, and queues a
message in MultiSimSettingController to turn it on.

3 and 4 enters an ifinite loop of flickering it on and off.

Fix is do not re-propagate the message from DcTracker to MultiSimSettingController
if it's already handling a userDataEnable event.

Bug: 138933419
Test: unittest
Change-Id: I4a89fd753c14d4e23cdd7a79c1c950c8cdd8f07b
Merged-In: I4a89fd753c14d4e23cdd7a79c1c950c8cdd8f07b
parent cd25d3a0

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

+1 −1
Original line number Diff line number Diff line
@@ -694,7 +694,7 @@ public class MultiSimSettingController extends Handler { 
                // If enable is true and it's not opportunistic subscription, we don't enable it,

                // as there can't e two

                if (phone != null) {

                    phone.getDataEnabledSettings().setUserDataEnabled(enable);

                    phone.getDataEnabledSettings().setUserDataEnabled(enable, false);

                }

            } else {

                // For inactive subscription, directly write into global settings.

src/java/com/android/internal/telephony/dataconnection/DataEnabledSettings.java

+16 −2
Original line number Diff line number Diff line
@@ -203,6 +203,18 @@ public class DataEnabledSettings { 
    }



    public synchronized void setUserDataEnabled(boolean enabled) {

        // By default the change should propagate to the group.

        setUserDataEnabled(enabled, true);

    }



    /**

     * @param notifyMultiSimSettingController if setUserDataEnabled is already from propagating

     *        from MultiSimSettingController, don't notify MultiSimSettingController again.

     *        For example, if sub1 and sub2 are in the same group and user enables data for sub

     *        1, sub 2 will also be enabled but with propagateToGroup = false.

     */

    public synchronized void setUserDataEnabled(boolean enabled,

            boolean notifyMultiSimSettingController) {

        // Can't disable data for stand alone opportunistic subscription.

        if (isStandAloneOpportunistic(mPhone.getSubId(), mPhone.getContext()) && !enabled) return;
@@ -212,8 +224,10 @@ public class DataEnabledSettings { 
            localLog("UserDataEnabled", enabled);

            mPhone.notifyUserMobileDataStateChanged(enabled);

            updateDataEnabledAndNotify(REASON_USER_DATA_ENABLED);

            MultiSimSettingController.getInstance().notifyUserDataEnabled(mPhone.getSubId(),

                    enabled);

            if (notifyMultiSimSettingController) {

                MultiSimSettingController.getInstance().notifyUserDataEnabled(

                        mPhone.getSubId(), enabled);

            }

        }

    }

tests/telephonytests/src/com/android/internal/telephony/MultiSimSettingControllerTest.java

+4 −4
Original line number Diff line number Diff line
@@ -491,7 +491,7 @@ public class MultiSimSettingControllerTest extends TelephonyTest { 
        mMultiSimSettingControllerUT.notifySubscriptionGroupChanged(mGroupUuid1);

        waitABit();

        // This should result in setting sync.

        verify(mDataEnabledSettingsMock1).setUserDataEnabled(false);

        verify(mDataEnabledSettingsMock1).setUserDataEnabled(false, false);

        assertFalse(GlobalSettingsHelper.getBoolean(

                mContext, Settings.Global.DATA_ROAMING, 1, true));
@@ -500,7 +500,7 @@ public class MultiSimSettingControllerTest extends TelephonyTest { 
        // Turning data on on sub 2. Sub 1 should also be turned on.

        mMultiSimSettingControllerUT.notifyUserDataEnabled(2, true);

        waitABit();

        verify(mDataEnabledSettingsMock1).setUserDataEnabled(true);

        verify(mDataEnabledSettingsMock1).setUserDataEnabled(true, false);

        // No user selection needed, no intent should be sent.

        verify(mContext, never()).sendBroadcast(any());

    }
@@ -531,7 +531,7 @@ public class MultiSimSettingControllerTest extends TelephonyTest { 
        mMultiSimSettingControllerUT.notifySubscriptionGroupChanged(mGroupUuid1);

        waitABit();

        // This should result in setting sync.

        verify(mDataEnabledSettingsMock2).setUserDataEnabled(true);

        verify(mDataEnabledSettingsMock2).setUserDataEnabled(true, false);

        assertFalse(GlobalSettingsHelper.getBoolean(

                mContext, Settings.Global.DATA_ROAMING, 2, true));

        verify(mSubControllerMock).setDataRoaming(/*enable*/0, /*subId*/1);
@@ -540,7 +540,7 @@ public class MultiSimSettingControllerTest extends TelephonyTest { 
        doReturn(false).when(mPhoneMock1).isUserDataEnabled();

        mMultiSimSettingControllerUT.notifyUserDataEnabled(1, false);

        waitABit();

        verify(mDataEnabledSettingsMock2).setUserDataEnabled(false);

        verify(mDataEnabledSettingsMock2).setUserDataEnabled(false, false);

    }



    @Test