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

Commit 29846984 authored by Weng Su's avatar Weng Su
Browse files

Refine TetherSettings to avoid activity leaks 

- Get system service by application context instead of fragment context to avoid the context being occupied

- Use the application main executor instead of creating new executor locally to avoid the executor being occupied

Bug: 246531382
Test: manual test
make RunSettingsRoboTests ROBOTEST_FILTER=TetherSettingsTest
atest -c TetheringHelperTest

Change-Id: I0de5e9c2bedfc8a224b3f8306224542212c9fcf5
parent a3df41ca

src/com/android/settings/network/tether/TetherSettings.java

+9 −27
Original line number Diff line number Diff line
@@ -77,7 +77,7 @@ import java.util.concurrent.atomic.AtomicReference; 
 */

@SearchIndexable

public class TetherSettings extends RestrictedSettingsFragment

        implements DataSaverBackend.Listener {

        implements DataSaverBackend.Listener, TetheringManager.TetheringEventCallback {



    @VisibleForTesting

    static final String KEY_TETHER_PREFS_SCREEN = "tether_prefs_screen";
@@ -111,7 +111,6 @@ public class TetherSettings extends RestrictedSettingsFragment 
    private OnStartTetheringCallback mStartTetheringCallback;

    private ConnectivityManager mCm;

    private EthernetManager mEm;

    private TetheringEventCallback mTetheringEventCallback;

    private EthernetListener mEthernetListener;

    private final HashSet<String> mAvailableInterfaces = new HashSet<>();
@@ -133,6 +132,7 @@ public class TetherSettings extends RestrictedSettingsFragment 
    Context mContext;

    @VisibleForTesting

    TetheringManager mTm;

    private TetheringHelper mTetheringHelper;



    @Override

    public int getMetricsCategory() {
@@ -148,6 +148,8 @@ public class TetherSettings extends RestrictedSettingsFragment 
        super.onAttach(context);

        mWifiTetherPreferenceController =

                new WifiTetherPreferenceController(context, getSettingsLifecycle());

        mTetheringHelper = TetheringHelper.getInstance(context, this /* TetheringEventCallback */,

                getSettingsLifecycle());

    }



    @Override
@@ -186,7 +188,7 @@ public class TetherSettings extends RestrictedSettingsFragment 
        mDataSaverBackend.addListener(this);



        mCm = (ConnectivityManager) getSystemService(Context.CONNECTIVITY_SERVICE);

        mTm = (TetheringManager) getSystemService(Context.TETHERING_SERVICE);

        mTm = mTetheringHelper.getTetheringManager();

        // Some devices do not have available EthernetManager. In that case getSystemService will

        // return null.

        mEm = mContext.getSystemService(EthernetManager.class);
@@ -364,15 +366,14 @@ public class TetherSettings extends RestrictedSettingsFragment 




        mStartTetheringCallback = new OnStartTetheringCallback(this);

        mTetheringEventCallback = new TetheringEventCallback(this);

        mTm.registerTetheringEventCallback(r -> mHandler.post(r), mTetheringEventCallback);



        mMassStorageActive = Environment.MEDIA_SHARED.equals(Environment.getExternalStorageState());

        registerReceiver();



        mEthernetListener = new EthernetListener(this);

        if (mEm != null) {

            mEm.addInterfaceStateListener(r -> mHandler.post(r), mEthernetListener);

            mEm.addInterfaceStateListener(mContext.getApplicationContext().getMainExecutor(),

                    mEthernetListener);

        }



        updateUsbState();
@@ -387,13 +388,11 @@ public class TetherSettings extends RestrictedSettingsFragment 
            return;

        }

        getActivity().unregisterReceiver(mTetherChangeReceiver);

        mTm.unregisterTetheringEventCallback(mTetheringEventCallback);

        if (mEm != null) {

            mEm.removeInterfaceStateListener(mEthernetListener);

        }

        mTetherChangeReceiver = null;

        mStartTetheringCallback = null;

        mTetheringEventCallback = null;

    }



    @VisibleForTesting
@@ -688,25 +687,8 @@ public class TetherSettings extends RestrictedSettingsFragment 
        }

    }



    private static final class TetheringEventCallback implements

            TetheringManager.TetheringEventCallback {

        final WeakReference<TetherSettings> mTetherSettings;



        TetheringEventCallback(TetherSettings settings) {

            mTetherSettings = new WeakReference<>(settings);

        }



    @Override

    public void onTetheredInterfacesChanged(List<String> interfaces) {

            final TetherSettings tetherSettings = mTetherSettings.get();

            if (tetherSettings == null) {

                return;

            }

            tetherSettings.onTetheredInterfacesChanged(interfaces);

        }

    }



    void onTetheredInterfacesChanged(List<String> interfaces) {

        Log.d(TAG, "onTetheredInterfacesChanged() interfaces : " + interfaces.toString());

        final String[] tethered = interfaces.toArray(new String[interfaces.size()]);

        updateUsbState(tethered);

src/com/android/settings/network/tether/TetheringHelper.java

0 → 100644
+157 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.settings.network.tether;



import android.annotation.NonNull;

import android.annotation.TestApi;

import android.content.Context;

import android.net.TetheringManager;

import android.util.Log;



import androidx.annotation.VisibleForTesting;

import androidx.lifecycle.DefaultLifecycleObserver;

import androidx.lifecycle.Lifecycle;

import androidx.lifecycle.LifecycleOwner;



import com.android.internal.annotations.GuardedBy;



import java.util.ArrayList;

import java.util.List;

import java.util.Map;

import java.util.concurrent.ConcurrentHashMap;



/**

 * Helper class for TetheringManager.

 */

public class TetheringHelper implements DefaultLifecycleObserver {

    private static final String TAG = "TetheringHelper";



    private static final Object sInstanceLock = new Object();

    @TestApi

    @GuardedBy("sInstanceLock")

    private static Map<Context, TetheringHelper> sTestInstances;



    protected static Context sAppContext;

    protected static TetheringManager sTetheringManager;

    protected static TetheringEventCallback sTetheringEventCallback = new TetheringEventCallback();

    protected static ArrayList<TetheringManager.TetheringEventCallback> sEventCallbacks =

            new ArrayList<>();



    protected TetheringManager.TetheringEventCallback mCallback;



    /**

     * Static method to create a singleton class for TetheringHelper.

     *

     * @param context The Context this is associated with.

     * @return an instance of {@link TetheringHelper} object.

     */

    @NonNull

    public static TetheringHelper getInstance(@NonNull Context context,

            @NonNull TetheringManager.TetheringEventCallback callback,

            @NonNull Lifecycle lifecycle) {

        synchronized (sInstanceLock) {

            if (sTestInstances != null && sTestInstances.containsKey(context)) {

                TetheringHelper testInstance = sTestInstances.get(context);

                Log.w(TAG, "The context owner use a test instance:" + testInstance);

                return testInstance;

            }



            if (sAppContext == null) {

                sAppContext = context.getApplicationContext();

                sTetheringManager = sAppContext.getSystemService(TetheringManager.class);

            }

            TetheringHelper helper = new TetheringHelper();

            helper.mCallback = callback;

            lifecycle.addObserver(helper);

            return helper;

        }

    }



    /**

     * A convenience method to set pre-prepared instance or mock class for testing.

     *

     * @param context  The Context this is associated with.

     * @param instance of {@link TetheringHelper} object.

     * @hide

     */

    @TestApi

    @VisibleForTesting

    public static void setTestInstance(@NonNull Context context, TetheringHelper instance) {

        synchronized (sInstanceLock) {

            if (sTestInstances == null) sTestInstances = new ConcurrentHashMap<>();

            Log.w(TAG, "Set a test instance by context:" + context);

            sTestInstances.put(context, instance);

        }

    }



    /**

     * The constructor can only be accessed from static method inside the class itself, this is

     * to avoid creating a class by adding a private constructor.

     */

    private TetheringHelper() {

        // Do nothing.

    }



    /**

     * Returns the TetheringManager If the system service is successfully obtained.

     */

    public TetheringManager getTetheringManager() {

        return sTetheringManager;

    }



    @Override

    public void onStart(@NonNull LifecycleOwner owner) {

        if (sEventCallbacks.contains(mCallback)) {

            Log.w(TAG, "The callback already contains, don't register callback:" + mCallback);

            return;

        }

        sEventCallbacks.add(mCallback);

        if (sEventCallbacks.size() == 1) {

            sTetheringManager.registerTetheringEventCallback(sAppContext.getMainExecutor(),

                    sTetheringEventCallback);

        }

    }



    @Override

    public void onStop(@NonNull LifecycleOwner owner) {

        if (!sEventCallbacks.remove(mCallback)) {

            Log.w(TAG, "The callback does not contain, don't unregister callback:" + mCallback);

            return;

        }

        if (sEventCallbacks.size() == 0) {

            sTetheringManager.unregisterTetheringEventCallback(sTetheringEventCallback);

        }

    }



    @Override

    public void onDestroy(@NonNull LifecycleOwner owner) {

        mCallback = null;

    }



    protected static final class TetheringEventCallback implements

            TetheringManager.TetheringEventCallback {

        @Override

        public void onTetheredInterfacesChanged(List<String> interfaces) {

            for (TetheringManager.TetheringEventCallback callback : sEventCallbacks) {

                if (callback == null) continue;

                callback.onTetheredInterfacesChanged(interfaces);

            }

        }

    }

}

tests/robotests/src/com/android/settings/network/tether/TetheringHelperTest.java

0 → 100644
+150 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.settings.network.tether;



import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;



import android.content.Context;

import android.net.TetheringManager;



import androidx.lifecycle.Lifecycle;

import androidx.lifecycle.LifecycleOwner;

import androidx.test.core.app.ApplicationProvider;

import androidx.test.ext.junit.runners.AndroidJUnit4;



import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.Mock;

import org.mockito.Spy;

import org.mockito.junit.MockitoJUnit;

import org.mockito.junit.MockitoRule;



import java.util.List;

import java.util.concurrent.Executor;



@RunWith(AndroidJUnit4.class)

public class TetheringHelperTest {

    @Rule

    public final MockitoRule mMockitoRule = MockitoJUnit.rule();

    @Spy

    Context mContext = ApplicationProvider.getApplicationContext();

    @Mock

    Lifecycle mLifecycle;

    @Mock

    LifecycleOwner mLifecycleOwner;

    @Mock

    TetheringManager mTetheringManager;

    @Mock

    TetheringManager.TetheringEventCallback mTetheringEventCallback;

    @Mock

    List<String> mInterfaces;



    TetheringHelper mHelper;



    @Before

    public void setUp() {

        mHelper = TetheringHelper.getInstance(mContext, mTetheringEventCallback, mLifecycle);

        mHelper.sTetheringManager = mTetheringManager;

    }



    @Test

    public void constructor_propertiesShouldBeReady() {

        assertThat(mHelper.sAppContext).isNotNull();

        assertThat(mHelper.sTetheringManager).isNotNull();

        assertThat(mHelper.mCallback).isNotNull();

        verify(mLifecycle).addObserver(any());

    }



    @Test

    public void getTetheringManager_isNotNull() {

        assertThat(mHelper.getTetheringManager()).isNotNull();

    }



    @Test

    public void onStart_eventCallbacksClear_addAndRegisterCallback() {

        mHelper.sEventCallbacks.clear();



        mHelper.onStart(mLifecycleOwner);



        assertThat(mHelper.sEventCallbacks.contains(mTetheringEventCallback)).isTrue();

        assertThat(mHelper.sEventCallbacks.size()).isEqualTo(1);

        verify(mTetheringManager).registerTetheringEventCallback(any(Executor.class),

                eq(TetheringHelper.sTetheringEventCallback));

    }



    @Test

    public void onStart_eventCallbacksContains_doNotRegisterCallback() {

        mHelper.sEventCallbacks.clear();

        mHelper.sEventCallbacks.add(mTetheringEventCallback);



        mHelper.onStart(mLifecycleOwner);



        verify(mTetheringManager, never()).registerTetheringEventCallback(any(Executor.class),

                eq(TetheringHelper.sTetheringEventCallback));

    }



    @Test

    public void onStop_eventCallbacksContains_removeAndUnregisterCallback() {

        mHelper.sEventCallbacks.clear();

        mHelper.sEventCallbacks.add(mTetheringEventCallback);



        mHelper.onStop(mLifecycleOwner);



        assertThat(mHelper.sEventCallbacks.contains(mTetheringEventCallback)).isFalse();

        assertThat(mHelper.sEventCallbacks.size()).isEqualTo(0);

        verify(mTetheringManager).unregisterTetheringEventCallback(

                eq(TetheringHelper.sTetheringEventCallback));

    }



    @Test

    public void onStop_eventCallbacksClear_doNotUnregisterCallback() {

        mHelper.sEventCallbacks.clear();



        mHelper.onStop(mLifecycleOwner);



        assertThat(mHelper.sEventCallbacks.contains(mTetheringEventCallback)).isFalse();

        assertThat(mHelper.sEventCallbacks.size()).isEqualTo(0);

        verify(mTetheringManager, never()).unregisterTetheringEventCallback(

                eq(TetheringHelper.sTetheringEventCallback));

    }



    @Test

    public void onTetheredInterfacesChanged_eventCallbacksContains_doCallback() {

        mHelper.sEventCallbacks.clear();

        mHelper.sEventCallbacks.add(mTetheringEventCallback);



        mHelper.sTetheringEventCallback.onTetheredInterfacesChanged(mInterfaces);



        verify(mTetheringEventCallback).onTetheredInterfacesChanged(eq(mInterfaces));

    }



    @Test

    public void onTetheredInterfacesChanged_eventCallbacksClear_doNotCallback() {

        mHelper.sEventCallbacks.clear();



        mHelper.sTetheringEventCallback.onTetheredInterfacesChanged(mInterfaces);



        verify(mTetheringEventCallback, never()).onTetheredInterfacesChanged(eq(mInterfaces));

    }

}