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

Commit db115452 authored by Fabián Kozynski's avatar Fabián Kozynski Committed by Fabian Kozynski
Browse files

Fix click on TileService bug when closing shade 

If an active (unbound) tile is clicked and then the shade is closed
immediately, the click would never be sent to the tile. This was because
a tile that has `onStopListening` called will immediately stopListening
(even if not bound) and reject the click when bound finally happens.

Instead, queue the stopListening to happen right after the click is
dispatched once the tile is bound.

Also, fix when we unbind from active tiles (as together with this it was
causing multiple calls to `onStopListening`). Now:
* If an active tile requests listening, it will be unbound right after
  they send a status update.
* If an active tile is bound because of a click, it will stop listening
  and be unbound as if it was not active.

Test: atest com.android.systemui.qs
Test: atest CtsTileServiceTestCases CtsSystemUiHostTestCases
Flag: ACONFIG com.android.systemui.qs_custom_tile_click_guaranteed_bug_fix DISABLED
Fixes: 339290820

Change-Id: I0d0a87304e252ad68c48145819098115b00399a1
parent f792c8cf

packages/SystemUI/aconfig/systemui.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -882,3 +882,13 @@ flag { 
  description: "Enables Backlinks improvement feature in App Clips"

  bug: "300307759"

}



flag {

  name: "qs_custom_tile_click_guaranteed_bug_fix"

  namespace: "systemui"

  description: "Guarantee that clicks on a tile always happen by postponing onStopListening until after the click."

  bug: "339290820"

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}
 
 No newline at end of file

packages/SystemUI/multivalentTests/src/com/android/systemui/qs/external/CloseShadeRightAfterClickTestB339290820.kt

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

 * Copyright (C) 2024 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.systemui.qs.external



import android.app.PendingIntent

import android.content.ComponentName

import android.content.Context

import android.content.ContextWrapper

import android.content.Intent

import android.content.ServiceConnection

import android.content.applicationContext

import android.content.packageManager

import android.os.Binder

import android.os.Handler

import android.os.RemoteException

import android.os.UserHandle

import android.platform.test.annotations.EnableFlags

import android.service.quicksettings.Tile

import android.testing.TestableContext

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

import androidx.test.filters.SmallTest

import com.android.systemui.Flags.FLAG_QS_CUSTOM_TILE_CLICK_GUARANTEED_BUG_FIX

import com.android.systemui.SysuiTestCase

import com.android.systemui.concurrency.fakeExecutor

import com.android.systemui.kosmos.testCase

import com.android.systemui.qs.pipeline.shared.TileSpec

import com.android.systemui.qs.tiles.impl.custom.packageManagerAdapterFacade

import com.android.systemui.qs.tiles.impl.custom.customTileSpec

import com.android.systemui.testKosmos

import com.android.systemui.util.concurrency.FakeExecutor

import com.android.systemui.util.mockito.whenever

import com.android.systemui.util.time.fakeSystemClock

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

import org.junit.Before

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.ArgumentMatchers.anyInt

import org.mockito.ArgumentMatchers.anyString



@RunWith(AndroidJUnit4::class)

@SmallTest

class CloseShadeRightAfterClickTestB339290820 : SysuiTestCase() {



    private val testableContext: TestableContext

    private val bindDelayExecutor: FakeExecutor

    private val kosmos =

        testKosmos().apply {

            testableContext = testCase.context

            bindDelayExecutor = FakeExecutor(fakeSystemClock)

            testableContext.setMockPackageManager(packageManager)

            customTileSpec = TileSpec.create(testComponentName)

            applicationContext = ContextWrapperDelayedBind(testableContext, bindDelayExecutor)

        }



    @Before

    fun setUp() {

        kosmos.apply {

            whenever(packageManager.getPackageUidAsUser(anyString(), anyInt(), anyInt()))

                .thenReturn(Binder.getCallingUid())

            packageManagerAdapterFacade.setIsActive(true)

            testableContext.addMockService(testComponentName, iQSTileService.asBinder())

        }

    }



    @Test

    @EnableFlags(FLAG_QS_CUSTOM_TILE_CLICK_GUARANTEED_BUG_FIX)

    fun testStopListeningShortlyAfterClick_clickIsSent() {

        with(kosmos) {

            val tile = FakeCustomTileInterface(tileServices)

            // Flush any bind from startup

            FakeExecutor.exhaustExecutors(fakeExecutor, bindDelayExecutor)



            // Open QS

            tile.setListening(true)

            fakeExecutor.runAllReady()

            tile.click()

            fakeExecutor.runAllReady()



            // No clicks yet because the latch is preventing the bind

            assertThat(iQSTileService.clicks).isEmpty()



            // Close QS

            tile.setListening(false)

            fakeExecutor.runAllReady()

            // And finally bind

            FakeExecutor.exhaustExecutors(fakeExecutor, bindDelayExecutor)



            assertThat(iQSTileService.clicks).containsExactly(tile.token)

        }

    }

}



private val testComponentName = ComponentName("pkg", "srv")



// This is a fake `CustomTile` that implements what we need for the test. Mainly setListening and

// click

private class FakeCustomTileInterface(tileServices: TileServices) : CustomTileInterface {

    override val user: Int

        get() = 0

    override val qsTile: Tile = Tile()

    override val component: ComponentName = testComponentName

    private var listening = false

    private val serviceManager = tileServices.getTileWrapper(this)

    private val serviceInterface = serviceManager.tileService



    val token = Binder()



    override fun getTileSpec(): String {

        return CustomTile.toSpec(component)

    }



    override fun refreshState() {}



    override fun updateTileState(tile: Tile, uid: Int) {}



    override fun onDialogShown() {}



    override fun onDialogHidden() {}



    override fun startActivityAndCollapse(pendingIntent: PendingIntent) {}



    override fun startUnlockAndRun() {}



    fun setListening(listening: Boolean) {

        if (listening == this.listening) return

        this.listening = listening



        try {

            if (listening) {

                if (!serviceManager.isActiveTile) {

                    serviceManager.setBindRequested(true)

                    serviceInterface.onStartListening()

                }

            } else {

                serviceInterface.onStopListening()

                serviceManager.setBindRequested(false)

            }

        } catch (e: RemoteException) {

            // Called through wrapper, won't happen here.

        }

    }



    fun click() {

        try {

            if (serviceManager.isActiveTile) {

                serviceManager.setBindRequested(true)

                serviceInterface.onStartListening()

            }

            serviceInterface.onClick(token)

        } catch (e: RemoteException) {

            // Called through wrapper, won't happen here.

        }

    }

}



private class ContextWrapperDelayedBind(

    val context: Context,

    val executor: FakeExecutor,

) : ContextWrapper(context) {

    override fun bindServiceAsUser(

        service: Intent,

        conn: ServiceConnection,

        flags: Int,

        user: UserHandle

    ): Boolean {

        executor.execute { super.bindServiceAsUser(service, conn, flags, user) }

        return true

    }



    override fun bindServiceAsUser(

        service: Intent,

        conn: ServiceConnection,

        flags: BindServiceFlags,

        user: UserHandle

    ): Boolean {

        executor.execute { super.bindServiceAsUser(service, conn, flags, user) }

        return true

    }



    override fun bindServiceAsUser(

        service: Intent?,

        conn: ServiceConnection?,

        flags: Int,

        handler: Handler?,

        user: UserHandle?

    ): Boolean {

        executor.execute { super.bindServiceAsUser(service, conn, flags, handler, user) }

        return true

    }



    override fun bindServiceAsUser(

        service: Intent,

        conn: ServiceConnection,

        flags: BindServiceFlags,

        handler: Handler,

        user: UserHandle

    ): Boolean {

        executor.execute { super.bindServiceAsUser(service, conn, flags, handler, user) }

        return true

    }

}

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

+22 −4
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@ import static android.os.PowerWhitelistManager.REASON_TILE_ONCLICK; 
import static android.provider.DeviceConfig.NAMESPACE_SYSTEMUI;

import static android.service.quicksettings.TileService.START_ACTIVITY_NEEDS_PENDING_INTENT;



import static com.android.systemui.Flags.qsCustomTileClickGuaranteedBugFix;



import android.app.ActivityManager;

import android.app.compat.CompatChanges;

import android.content.BroadcastReceiver;
@@ -88,6 +90,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements 
    private static final int MSG_ON_REMOVED = 1;

    private static final int MSG_ON_CLICK = 2;

    private static final int MSG_ON_UNLOCK_COMPLETE = 3;

    private static final int MSG_ON_STOP_LISTENING = 4;



    // Bind retry control.

    private static final int MAX_BIND_RETRIES = 5;
@@ -368,6 +371,16 @@ public class TileLifecycleManager extends BroadcastReceiver implements 
                onUnlockComplete();

            }

        }

        if (qsCustomTileClickGuaranteedBugFix()) {

            if (queue.contains(MSG_ON_STOP_LISTENING)) {

                if (mDebug) Log.d(TAG, "Handling pending onStopListening " + getComponent());

                if (mListening) {

                    onStopListening();

                } else {

                    Log.w(TAG, "Trying to stop listening when not listening " + getComponent());

                }

            }

        }

        if (queue.contains(MSG_ON_REMOVED)) {

            if (mDebug) Log.d(TAG, "Handling pending onRemoved " + getComponent());

            if (mListening) {
@@ -586,12 +599,17 @@ public class TileLifecycleManager extends BroadcastReceiver implements 


    @Override

    public void onStopListening() {

        if (qsCustomTileClickGuaranteedBugFix() && hasPendingClick()) {

            Log.d(TAG, "Enqueue stop listening");

            queueMessage(MSG_ON_STOP_LISTENING);

        } else {

            if (mDebug) Log.d(TAG, "onStopListening " + getComponent());

            mListening = false;

            if (isNotNullAndFailedAction(mOptionalWrapper, QSTileServiceWrapper::onStopListening)) {

                handleDeath();

            }

        }

    }



    @Override

    public void onClick(IBinder iBinder) {

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

+22 −2
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@ 
 */

package com.android.systemui.qs.external;



import static com.android.systemui.Flags.qsCustomTileClickGuaranteedBugFix;



import android.content.BroadcastReceiver;

import android.content.ComponentName;

import android.content.Context;
@@ -37,6 +39,7 @@ import com.android.systemui.settings.UserTracker; 


import java.util.List;

import java.util.Objects;

import java.util.concurrent.atomic.AtomicBoolean;



/**

 * Manages the priority which lets {@link TileServices} make decisions about which tiles
@@ -72,6 +75,8 @@ public class TileServiceManager { 
    private boolean mPendingBind = true;

    private boolean mStarted = false;



    private final AtomicBoolean mListeningFromRequest = new AtomicBoolean(false);



    TileServiceManager(TileServices tileServices, Handler handler, ComponentName component,

            UserTracker userTracker, TileLifecycleManager.Factory tileLifecycleManagerFactory,

            CustomTileAddedRepository customTileAddedRepository) {
@@ -159,15 +164,30 @@ public class TileServiceManager { 
        }

    }



    void onStartListeningFromRequest() {

        mListeningFromRequest.set(true);

        mStateManager.onStartListening();

    }



    public void setLastUpdate(long lastUpdate) {

        mLastUpdate = lastUpdate;

        if (mBound && isActiveTile()) {

            mStateManager.onStopListening();

            setBindRequested(false);

            if (qsCustomTileClickGuaranteedBugFix()) {

                if (mListeningFromRequest.compareAndSet(true, false)) {

                    stopListeningAndUnbind();

                }

            } else {

                stopListeningAndUnbind();

            }

        }

        mServices.recalculateBindAllowance();

    }



    private void stopListeningAndUnbind() {

        mStateManager.onStopListening();

        setBindRequested(false);

    }



    public void handleDestroy() {

        setBindAllowed(false);

        mServices.getContext().unregisterReceiver(mUninstallReceiver);

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

+9 −3
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@ 
 */

package com.android.systemui.qs.external;



import static com.android.systemui.Flags.qsCustomTileClickGuaranteedBugFix;



import android.app.PendingIntent;

import android.content.ComponentName;

import android.content.Context;
@@ -222,12 +224,16 @@ public class TileServices extends IQSService.Stub { 
                return;

            }

            service.setBindRequested(true);

            if (qsCustomTileClickGuaranteedBugFix()) {

                service.onStartListeningFromRequest();

            } else {

                try {

                    service.getTileService().onStartListening();

                } catch (RemoteException e) {

                }

            }

        }

    }



    @Override

    public void updateQsTile(Tile tile, IBinder token) {