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

Commit e6395dd7 authored by Fabian Kozynski's avatar Fabian Kozynski
Browse files

Fix race condition in TileServices 

TileServices#getTileWrapper created a TileServiceManager (that triggered
the binding of TileService through a Handler) and after populated the
maps with the respective tokens. As one of the keys is the
TileServiceManager, this had to do after creating it.

This created the situation that in certain cases, the binding would
happen before all the maps were populated (in particular mTokenMap but
sometimes mServices).

By decoupling the binding triggering from TileServiceManager constructor
into a separate method in TileServiceManager we can guarantee that by
the time the service is bound, the maps are populated. Also added some
guards to make sure that this method has been called and the tile has
been added.

The new method is called after populating the maps, and it would not
make sense for the TileService to be bound if the maps are not
populated.

Test: atest TileServiceMaanger TileLifecycleManager TileServicesTest
Test: atest CtsAppTestCases:android.app.cts.TileServiceTest (approx 50
times)
Test: test TileServiceTest ActiveTileServiceTest
Test: Development tiles and adding Grayscale tile work perfectly
Fixes: 124735442
Fixes: 129684780
Fixes: 130796699

Change-Id: Ie3c7277756c74551ba2e6b3e88dad4a98aeafc96
parent fad62e74

core/java/android/service/quicksettings/TileService.java

+17 −0
Original line number Original line Diff line number Diff line
@@ -32,6 +32,7 @@ import android.os.IBinder; 
import android.os.Looper;

import android.os.Looper;

import android.os.Message;

import android.os.Message;

import android.os.RemoteException;

import android.os.RemoteException;

import android.util.Log;

import android.view.View;

import android.view.View;

import android.view.View.OnAttachStateChangeListener;

import android.view.View.OnAttachStateChangeListener;

import android.view.WindowManager;

import android.view.WindowManager;
@@ -79,6 +80,9 @@ import com.android.internal.R; 
 */

 */

public class TileService extends Service {

public class TileService extends Service {





    private static final String TAG = "TileService";

    private static final boolean DEBUG = false;



    /**

    /**

     * An activity that provides a user interface for adjusting TileService

     * An activity that provides a user interface for adjusting TileService

     * preferences. Optional but recommended for apps that implement a

     * preferences. Optional but recommended for apps that implement a
@@ -381,18 +385,26 @@ public class TileService extends Service { 
        private static final int MSG_TILE_CLICKED = 5;

        private static final int MSG_TILE_CLICKED = 5;

        private static final int MSG_UNLOCK_COMPLETE = 6;

        private static final int MSG_UNLOCK_COMPLETE = 6;

        private static final int MSG_START_SUCCESS = 7;

        private static final int MSG_START_SUCCESS = 7;

        private final String mTileServiceName;





        public H(Looper looper) {

        public H(Looper looper) {

            super(looper);

            super(looper);

            mTileServiceName = TileService.this.getClass().getSimpleName();

        }



        private void logMessage(String message) {

            Log.d(TAG, mTileServiceName + " Handler - " + message);

        }

        }





        @Override

        @Override

        public void handleMessage(Message msg) {

        public void handleMessage(Message msg) {

            switch (msg.what) {

            switch (msg.what) {

                case MSG_TILE_ADDED:

                case MSG_TILE_ADDED:

                    if (DEBUG) logMessage("MSG_TILE_ADDED");

                    TileService.this.onTileAdded();

                    TileService.this.onTileAdded();

                    break;

                    break;

                case MSG_TILE_REMOVED:

                case MSG_TILE_REMOVED:

                    if (DEBUG) logMessage("MSG_TILE_REMOVED");

                    if (mListening) {

                    if (mListening) {

                        mListening = false;

                        mListening = false;

                        TileService.this.onStopListening();

                        TileService.this.onStopListening();
@@ -400,27 +412,32 @@ public class TileService extends Service { 
                    TileService.this.onTileRemoved();

                    TileService.this.onTileRemoved();

                    break;

                    break;

                case MSG_STOP_LISTENING:

                case MSG_STOP_LISTENING:

                    if (DEBUG) logMessage("MSG_STOP_LISTENING");

                    if (mListening) {

                    if (mListening) {

                        mListening = false;

                        mListening = false;

                        TileService.this.onStopListening();

                        TileService.this.onStopListening();

                    }

                    }

                    break;

                    break;

                case MSG_START_LISTENING:

                case MSG_START_LISTENING:

                    if (DEBUG) logMessage("MSG_START_LISTENING");

                    if (!mListening) {

                    if (!mListening) {

                        mListening = true;

                        mListening = true;

                        TileService.this.onStartListening();

                        TileService.this.onStartListening();

                    }

                    }

                    break;

                    break;

                case MSG_TILE_CLICKED:

                case MSG_TILE_CLICKED:

                    if (DEBUG) logMessage("MSG_TILE_CLICKED");

                    mToken = (IBinder) msg.obj;

                    mToken = (IBinder) msg.obj;

                    TileService.this.onClick();

                    TileService.this.onClick();

                    break;

                    break;

                case MSG_UNLOCK_COMPLETE:

                case MSG_UNLOCK_COMPLETE:

                    if (DEBUG) logMessage("MSG_UNLOCK_COMPLETE");

                    if (mUnlockRunnable != null) {

                    if (mUnlockRunnable != null) {

                        mUnlockRunnable.run();

                        mUnlockRunnable.run();

                    }

                    }

                    break;

                    break;

                case MSG_START_SUCCESS:

                case MSG_START_SUCCESS:

                    if (DEBUG) logMessage("MSG_START_SUCCESS");

                    try {

                    try {

                        mService.onStartSuccessful(mTileToken);

                        mService.onStartSuccessful(mTileToken);

                    } catch (RemoteException e) {

                    } catch (RemoteException e) {

packages/SystemUI/src/com/android/systemui/qs/external/TileServiceManager.java

+18 −1
Original line number Original line Diff line number Diff line
@@ -69,6 +69,7 @@ public class TileServiceManager { 
    // Whether we have a pending bind going out to the service without a response yet.

    // Whether we have a pending bind going out to the service without a response yet.

    // This defaults to true to ensure tiles start out unavailable.

    // This defaults to true to ensure tiles start out unavailable.

    private boolean mPendingBind = true;

    private boolean mPendingBind = true;

    private boolean mStarted = false;





    TileServiceManager(TileServices tileServices, Handler handler, ComponentName component,

    TileServiceManager(TileServices tileServices, Handler handler, ComponentName component,

            Tile tile) {

            Tile tile) {
@@ -90,7 +91,23 @@ public class TileServiceManager { 
        Context context = mServices.getContext();

        Context context = mServices.getContext();

        context.registerReceiverAsUser(mUninstallReceiver,

        context.registerReceiverAsUser(mUninstallReceiver,

                new UserHandle(ActivityManager.getCurrentUser()), filter, null, mHandler);

                new UserHandle(ActivityManager.getCurrentUser()), filter, null, mHandler);

        ComponentName component = tileLifecycleManager.getComponent();

    }



    boolean isLifecycleStarted() {

        return mStarted;

    }



    /**

     * Starts the TileLifecycleManager by adding the corresponding component as a Tile and

     * binding to it if needed.

     *

     * This method should be called after constructing a TileServiceManager to guarantee that the

     * TileLifecycleManager has added the tile and bound to it at least once.

     */

    void startLifecycleManagerAndAddTile() {

        mStarted = true;

        ComponentName component = mStateManager.getComponent();

        Context context = mServices.getContext();

        if (!TileLifecycleManager.isTileAdded(context, component)) {

        if (!TileLifecycleManager.isTileAdded(context, component)) {

            TileLifecycleManager.setTileAdded(context, component, true);

            TileLifecycleManager.setTileAdded(context, component, true);

            mStateManager.onTileAdded();

            mStateManager.onTileAdded();

packages/SystemUI/src/com/android/systemui/qs/external/TileServices.java

+15 −0
Original line number Original line Diff line number Diff line
@@ -51,6 +51,7 @@ import java.util.Comparator; 
public class TileServices extends IQSService.Stub {

public class TileServices extends IQSService.Stub {

    static final int DEFAULT_MAX_BOUND = 3;

    static final int DEFAULT_MAX_BOUND = 3;

    static final int REDUCED_MAX_BOUND = 1;

    static final int REDUCED_MAX_BOUND = 1;

    private static final String TAG = "TileServices";





    private final ArrayMap<CustomTile, TileServiceManager> mServices = new ArrayMap<>();

    private final ArrayMap<CustomTile, TileServiceManager> mServices = new ArrayMap<>();

    private final ArrayMap<ComponentName, CustomTile> mTiles = new ArrayMap<>();

    private final ArrayMap<ComponentName, CustomTile> mTiles = new ArrayMap<>();
@@ -87,6 +88,8 @@ public class TileServices extends IQSService.Stub { 
            mTiles.put(component, tile);

            mTiles.put(component, tile);

            mTokenMap.put(service.getToken(), tile);

            mTokenMap.put(service.getToken(), tile);

        }

        }

        // Makes sure binding only happens after the maps have been populated

        service.startLifecycleManagerAndAddTile();

        return service;

        return service;

    }

    }
@@ -179,6 +182,11 @@ public class TileServices extends IQSService.Stub { 
            verifyCaller(customTile);

            verifyCaller(customTile);

            synchronized (mServices) {

            synchronized (mServices) {

                final TileServiceManager tileServiceManager = mServices.get(customTile);

                final TileServiceManager tileServiceManager = mServices.get(customTile);

                if (tileServiceManager == null || !tileServiceManager.isLifecycleStarted()) {

                    Log.e(TAG, "TileServiceManager not started for " + customTile.getComponent(),

                            new IllegalStateException());

                    return;

                }

                tileServiceManager.clearPendingBind();

                tileServiceManager.clearPendingBind();

                tileServiceManager.setLastUpdate(System.currentTimeMillis());

                tileServiceManager.setLastUpdate(System.currentTimeMillis());

            }

            }
@@ -194,6 +202,13 @@ public class TileServices extends IQSService.Stub { 
            verifyCaller(customTile);

            verifyCaller(customTile);

            synchronized (mServices) {

            synchronized (mServices) {

                final TileServiceManager tileServiceManager = mServices.get(customTile);

                final TileServiceManager tileServiceManager = mServices.get(customTile);

                // This should not happen as the TileServiceManager should have been started for the

                // first bind to happen.

                if (tileServiceManager == null || !tileServiceManager.isLifecycleStarted()) {

                    Log.e(TAG, "TileServiceManager not started for " + customTile.getComponent(),

                            new IllegalStateException());

                    return;

                }

                tileServiceManager.clearPendingBind();

                tileServiceManager.clearPendingBind();

            }

            }

            customTile.refreshState();

            customTile.refreshState();

packages/SystemUI/tests/src/com/android/systemui/qs/external/TileServicesTest.java

+3 −1
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ import static junit.framework.Assert.assertEquals; 
import static junit.framework.Assert.assertTrue;

import static junit.framework.Assert.assertTrue;





import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;





import android.content.ComponentName;

import android.content.ComponentName;

import android.os.Handler;

import android.os.Handler;
@@ -88,7 +89,7 @@ public class TileServicesTest extends SysuiTestCase { 
        assertEquals(NUM_FAKES, mManagers.size());

        assertEquals(NUM_FAKES, mManagers.size());





        for (int i = 0; i < NUM_FAKES; i++) {

        for (int i = 0; i < NUM_FAKES; i++) {

            Mockito.when(mManagers.get(i).getBindPriority()).thenReturn(i);

            when(mManagers.get(i).getBindPriority()).thenReturn(i);

        }

        }

        mTileService.recalculateBindAllowance();

        mTileService.recalculateBindAllowance();

        for (int i = 0; i < NUM_FAKES; i++) {

        for (int i = 0; i < NUM_FAKES; i++) {
@@ -145,6 +146,7 @@ public class TileServicesTest extends SysuiTestCase { 
        protected TileServiceManager onCreateTileService(ComponentName component, Tile qsTile) {

        protected TileServiceManager onCreateTileService(ComponentName component, Tile qsTile) {

            TileServiceManager manager = mock(TileServiceManager.class);

            TileServiceManager manager = mock(TileServiceManager.class);

            mManagers.add(manager);

            mManagers.add(manager);

            when(manager.isLifecycleStarted()).thenReturn(true);

            return manager;

            return manager;

        }

        }

    }

    }