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

Commit 8764413d authored by Fabian Kozynski's avatar Fabian Kozynski
Browse files

Fixed threading issue in LocationControllerImpl 

Callback list is now only modifiable through the Handler.

New tests and deflaking of existing ones.

Test: atest LocationControllerImpl
Fixes: 153623565
Change-Id: Ia70ec2b2dc948457d26fe74b7c38021c6995a7e5
parent f7b69012

packages/SystemUI/src/com/android/systemui/statusbar/policy/LocationControllerImpl.java

+23 −7
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import androidx.annotation.VisibleForTesting; 
import com.android.systemui.BootCompleteCache;

import com.android.systemui.broadcast.BroadcastDispatcher;

import com.android.systemui.dagger.qualifiers.Background;

import com.android.systemui.dagger.qualifiers.Main;

import com.android.systemui.util.Utils;



import java.util.ArrayList;
@@ -64,16 +65,16 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio 


    private boolean mAreActiveLocationRequests;



    private ArrayList<LocationChangeCallback> mSettingsChangeCallbacks =

            new ArrayList<LocationChangeCallback>();

    private final H mHandler = new H();

    private final H mHandler;



    @Inject

    public LocationControllerImpl(Context context, @Background Looper bgLooper,

            BroadcastDispatcher broadcastDispatcher, BootCompleteCache bootCompleteCache) {

    public LocationControllerImpl(Context context, @Main Looper mainLooper,

            @Background Looper bgLooper, BroadcastDispatcher broadcastDispatcher,

            BootCompleteCache bootCompleteCache) {

        mContext = context;

        mBroadcastDispatcher = broadcastDispatcher;

        mBootCompleteCache = bootCompleteCache;

        mHandler = new H(mainLooper);



        // Register to listen for changes in location settings.

        IntentFilter filter = new IntentFilter();
@@ -94,12 +95,12 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio 
     * Add a callback to listen for changes in location settings.

     */

    public void addCallback(LocationChangeCallback cb) {

        mSettingsChangeCallbacks.add(cb);

        mHandler.obtainMessage(H.MSG_ADD_CALLBACK, cb).sendToTarget();

        mHandler.sendEmptyMessage(H.MSG_LOCATION_SETTINGS_CHANGED);

    }



    public void removeCallback(LocationChangeCallback cb) {

        mSettingsChangeCallbacks.remove(cb);

        mHandler.obtainMessage(H.MSG_REMOVE_CALLBACK, cb).sendToTarget();

    }



    /**
@@ -208,6 +209,14 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio 
    private final class H extends Handler {

        private static final int MSG_LOCATION_SETTINGS_CHANGED = 1;

        private static final int MSG_LOCATION_ACTIVE_CHANGED = 2;

        private static final int MSG_ADD_CALLBACK = 3;

        private static final int MSG_REMOVE_CALLBACK = 4;



        private ArrayList<LocationChangeCallback> mSettingsChangeCallbacks = new ArrayList<>();



        H(Looper looper) {

            super(looper);

        }



        @Override

        public void handleMessage(Message msg) {
@@ -218,6 +227,13 @@ public class LocationControllerImpl extends BroadcastReceiver implements Locatio 
                case MSG_LOCATION_ACTIVE_CHANGED:

                    locationActiveChanged();

                    break;

                case MSG_ADD_CALLBACK:

                    mSettingsChangeCallbacks.add((LocationChangeCallback) msg.obj);

                    break;

                case MSG_REMOVE_CALLBACK:

                    mSettingsChangeCallbacks.remove((LocationChangeCallback) msg.obj);

                    break;



            }

        }

packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/LocationControllerImplTest.java

+50 −6
Original line number Diff line number Diff line
@@ -14,8 +14,11 @@ 


package com.android.systemui.statusbar.policy;



import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.Mockito.mock;

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.Intent;
@@ -32,7 +35,6 @@ import com.android.systemui.broadcast.BroadcastDispatcher; 
import com.android.systemui.statusbar.policy.LocationController.LocationChangeCallback;



import org.junit.Before;

import org.junit.Ignore;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -42,17 +44,19 @@ import org.junit.runner.RunWith; 
public class LocationControllerImplTest extends SysuiTestCase {



    private LocationControllerImpl mLocationController;

    private TestableLooper mTestableLooper;



    @Before

    public void setup() {

        mTestableLooper = TestableLooper.get(this);

        mLocationController = spy(new LocationControllerImpl(mContext,

                TestableLooper.get(this).getLooper(),

                mTestableLooper.getLooper(),

                mTestableLooper.getLooper(),

                mock(BroadcastDispatcher.class),

                mock(BootCompleteCache.class)));

    }



    @Test

    @Ignore("flaky")

    public void testRemoveSelfActive_DoesNotCrash() {

        LocationController.LocationChangeCallback callback = new LocationChangeCallback() {

            @Override
@@ -70,11 +74,10 @@ public class LocationControllerImplTest extends SysuiTestCase { 
        mLocationController.onReceive(mContext, new Intent(

                LocationManager.HIGH_POWER_REQUEST_CHANGE_ACTION));



        TestableLooper.get(this).processAllMessages();

        mTestableLooper.processAllMessages();

    }



    @Test

    @Ignore("flaky")

    public void testRemoveSelfSettings_DoesNotCrash() {

        LocationController.LocationChangeCallback callback = new LocationChangeCallback() {

            @Override
@@ -85,6 +88,47 @@ public class LocationControllerImplTest extends SysuiTestCase { 
        mLocationController.addCallback(callback);

        mLocationController.addCallback(mock(LocationChangeCallback.class));



        TestableLooper.get(this).processAllMessages();

        mTestableLooper.processAllMessages();

    }



    @Test

    public void testAddCallback_notifiedImmediately() {

        LocationChangeCallback callback = mock(LocationChangeCallback.class);



        mLocationController.addCallback(callback);



        mTestableLooper.processAllMessages();



        verify(callback).onLocationSettingsChanged(anyBoolean());

    }



    @Test

    public void testCallbackNotified() {

        LocationChangeCallback callback = mock(LocationChangeCallback.class);



        mLocationController.addCallback(callback);

        mLocationController.onReceive(mContext, new Intent(LocationManager.MODE_CHANGED_ACTION));



        mTestableLooper.processAllMessages();



        verify(callback, times(2)).onLocationSettingsChanged(anyBoolean());

    }



    @Test

    public void testCallbackRemoved() {

        LocationChangeCallback callback = mock(LocationChangeCallback.class);



        mLocationController.addCallback(callback);

        mTestableLooper.processAllMessages();



        verify(callback).onLocationSettingsChanged(anyBoolean());

        mLocationController.removeCallback(callback);



        mLocationController.onReceive(mContext, new Intent(LocationManager.MODE_CHANGED_ACTION));



        mTestableLooper.processAllMessages();



        // No new callbacks

        verify(callback).onLocationSettingsChanged(anyBoolean());

    }

}