Merge "Fix binder death and disconnection." into main

2 files changed, 144 insertions, 63 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java
index 789a1e401ae6..e08eb37b79ba 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java
@@ -66,7 +66,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
  */
 public class TileLifecycleManager extends BroadcastReceiver implements
         IQSTileService, ServiceConnection, IBinder.DeathRecipient {
-    public static final boolean DEBUG = false;
+
+    private final boolean mDebug = Log.isLoggable(TAG, Log.DEBUG);
 
     private static final String TAG = "TileLifecycleManager";
 
@@ -101,7 +102,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     private Set<Integer> mQueuedMessages = new ArraySet<>();
     @Nullable
-    private QSTileServiceWrapper mWrapper;
+    private volatile QSTileServiceWrapper mWrapper;
     private boolean mListening;
     private IBinder mClickBinder;
 
@@ -132,7 +133,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
         mPackageManagerAdapter = packageManagerAdapter;
         mBroadcastDispatcher = broadcastDispatcher;
         mActivityManager = activityManager;
-        if (DEBUG) Log.d(TAG, "Creating " + mIntent + " " + mUser);
+        if (mDebug) Log.d(TAG, "Creating " + mIntent + " " + mUser);
     }
 
     /** Injectable factory for TileLifecycleManager. */
@@ -215,10 +216,11 @@ public class TileLifecycleManager extends BroadcastReceiver implements
             if (!checkComponentState()) {
                 return;
             }
-            if (DEBUG) Log.d(TAG, "Binding service " + mIntent + " " + mUser);
+            if (mDebug) Log.d(TAG, "Binding service " + mIntent + " " + mUser);
             mBindTryCount++;
             try {
-                mIsBound.set(bindServices());
+                // Only try a new binding if we are not currently bound.
+                mIsBound.compareAndSet(false, bindServices());
                 if (!mIsBound.get()) {
                     mContext.unbindService(this);
                 }
@@ -227,19 +229,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                 mIsBound.set(false);
             }
         } else {
-            if (DEBUG) Log.d(TAG, "Unbinding service " + mIntent + " " + mUser);
-            // Give it another chance next time it needs to be bound, out of kindness.
-            mBindTryCount = 0;
-            freeWrapper();
-            if (mIsBound.get()) {
-                try {
-                    mContext.unbindService(this);
-                } catch (Exception e) {
-                    Log.e(TAG, "Failed to unbind service "
-                            + mIntent.getComponent().flattenToShortString(), e);
-                }
-                mIsBound.set(false);
-            }
+            unbindService();
         }
     }
 
@@ -264,9 +254,26 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                 mUser);
     }
 
+    @WorkerThread
+    private void unbindService() {
+        if (mDebug) Log.d(TAG, "Unbinding service " + mIntent + " " + mUser);
+        // Give it another chance next time it needs to be bound, out of kindness.
+        mBindTryCount = 0;
+        freeWrapper();
+        if (mIsBound.get()) {
+            try {
+                mContext.unbindService(this);
+            } catch (Exception e) {
+                Log.e(TAG, "Failed to unbind service "
+                        + mIntent.getComponent().flattenToShortString(), e);
+            }
+            mIsBound.set(false);
+        }
+    }
+
     @Override
     public void onServiceConnected(ComponentName name, IBinder service) {
-        if (DEBUG) Log.d(TAG, "onServiceConnected " + name);
+        if (mDebug) Log.d(TAG, "onServiceConnected " + name);
         // Got a connection, set the binding count to 0.
         mBindTryCount = 0;
         final QSTileServiceWrapper wrapper = new QSTileServiceWrapper(Stub.asInterface(service));
@@ -284,11 +291,17 @@ public class TileLifecycleManager extends BroadcastReceiver implements
     }
 
     @Override
-    public void onServiceDisconnected(ComponentName name) {
-        if (DEBUG) Log.d(TAG, "onServiceDisconnected " + name);
+    public void onBindingDied(ComponentName name) {
+        if (mDebug) Log.d(TAG, "onBindingDied " + name);
         handleDeath();
     }
 
+    @Override
+    public void onServiceDisconnected(ComponentName name) {
+        if (mDebug) Log.d(TAG, "onServiceDisconnected " + name);
+        freeWrapper();
+    }
+
     private void handlePendingMessages() {
         // This ordering is laid out manually to make sure we preserve the TileService
         // lifecycle.
@@ -298,35 +311,36 @@ public class TileLifecycleManager extends BroadcastReceiver implements
             mQueuedMessages.clear();
         }
         if (queue.contains(MSG_ON_ADDED)) {
-            if (DEBUG) Log.d(TAG, "Handling pending onAdded");
+            if (mDebug) Log.d(TAG, "Handling pending onAdded " + getComponent());
             onTileAdded();
         }
         if (mListening) {
-            if (DEBUG) Log.d(TAG, "Handling pending onStartListening");
+            if (mDebug) Log.d(TAG, "Handling pending onStartListening " + getComponent());
             onStartListening();
         }
         if (queue.contains(MSG_ON_CLICK)) {
-            if (DEBUG) Log.d(TAG, "Handling pending onClick");
+            if (mDebug) Log.d(TAG, "Handling pending onClick " + getComponent());
             if (!mListening) {
-                Log.w(TAG, "Managed to get click on non-listening state...");
+                Log.w(TAG, "Managed to get click on non-listening state... " + getComponent());
                 // Skipping click since lost click privileges.
             } else {
                 onClick(mClickBinder);
             }
         }
         if (queue.contains(MSG_ON_UNLOCK_COMPLETE)) {
-            if (DEBUG) Log.d(TAG, "Handling pending onUnlockComplete");
+            if (mDebug) Log.d(TAG, "Handling pending onUnlockComplete " + getComponent());
             if (!mListening) {
-                Log.w(TAG, "Managed to get unlock on non-listening state...");
+                Log.w(TAG,
+                        "Managed to get unlock on non-listening state... " + getComponent());
                 // Skipping unlock since lost click privileges.
             } else {
                 onUnlockComplete();
             }
         }
         if (queue.contains(MSG_ON_REMOVED)) {
-            if (DEBUG) Log.d(TAG, "Handling pending onRemoved");
+            if (mDebug) Log.d(TAG, "Handling pending onRemoved " + getComponent());
             if (mListening) {
-                Log.w(TAG, "Managed to get remove in listening state...");
+                Log.w(TAG, "Managed to get remove in listening state... " + getComponent());
                 onStopListening();
             }
             onTileRemoved();
@@ -340,28 +354,44 @@ public class TileLifecycleManager extends BroadcastReceiver implements
     }
 
     public void handleDestroy() {
-        if (DEBUG) Log.d(TAG, "handleDestroy");
+        if (mDebug) Log.d(TAG, "handleDestroy");
         if (mPackageReceiverRegistered.get() || mUserReceiverRegistered.get()) {
             stopPackageListening();
         }
         mChangeListener = null;
     }
 
+    /**
+     * Handles a dead binder.
+     *
+     * It means that we need to clean up the binding (calling unbindService). After that, if we
+     * are supposed to be bound, we will try to bind after some amount of time.
+     */
     private void handleDeath() {
-        if (mWrapper == null) return;
-        freeWrapper();
-        // Clearly not bound anymore
-        mIsBound.set(false);
-        if (!mBound.get()) return;
-        if (DEBUG) Log.d(TAG, "handleDeath");
-        if (checkComponentState()) {
-            if (isDeathRebindScheduled.compareAndSet(false, true)) {
-                mExecutor.executeDelayed(() -> {
-                    setBindService(true);
-                    isDeathRebindScheduled.set(false);
-                }, getRebindDelay());
+        mExecutor.execute(() -> {
+            if (!mIsBound.get()) {
+                // If we are already not bound, don't do anything else.
+                return;
             }
-        }
+            // Clearly we shouldn't be bound anymore
+            if (mDebug) Log.d(TAG, "handleDeath " + getComponent());
+            // Binder died, make sure that we unbind. However, we don't want to call setBindService
+            // as we still may want to rebind.
+            unbindService();
+            // If mBound is true (meaning that we should be bound), then reschedule binding for
+            // later.
+            if (mBound.get() && checkComponentState()) {
+                if (isDeathRebindScheduled.compareAndSet(false, true)) {
+                    mExecutor.executeDelayed(() -> {
+                        // Only rebind if we are supposed to, but remove the scheduling anyway.
+                        if (mBound.get()) {
+                            setBindService(true);
+                        }
+                        isDeathRebindScheduled.set(false);
+                    }, getRebindDelay());
+                }
+            }
+        });
     }
 
     /**
@@ -379,7 +409,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
         } else {
             delay = mBindRetryDelay;
         }
-        Log.i(TAG, "Rebinding with a delay=" + delay);
+        if (mDebug) Log.i(TAG, "Rebinding with a delay=" + delay + " - " + getComponent());
         return delay;
     }
 
@@ -392,7 +422,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
     }
 
     private void startPackageListening() {
-        if (DEBUG) Log.d(TAG, "startPackageListening");
+        if (mDebug) Log.d(TAG, "startPackageListening " + getComponent());
         IntentFilter filter = new IntentFilter(Intent.ACTION_PACKAGE_ADDED);
         filter.addAction(Intent.ACTION_PACKAGE_CHANGED);
         filter.addDataScheme("package");
@@ -402,7 +432,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                     this, mUser, filter, null, mHandler, Context.RECEIVER_EXPORTED);
         } catch (Exception ex) {
             mPackageReceiverRegistered.set(false);
-            Log.e(TAG, "Could not register package receiver", ex);
+            Log.e(TAG, "Could not register package receiver " + getComponent(), ex);
         }
         filter = new IntentFilter(Intent.ACTION_USER_UNLOCKED);
         try {
@@ -410,12 +440,12 @@ public class TileLifecycleManager extends BroadcastReceiver implements
             mBroadcastDispatcher.registerReceiverWithHandler(this, filter, mHandler, mUser);
         } catch (Exception ex) {
             mUserReceiverRegistered.set(false);
-            Log.e(TAG, "Could not register unlock receiver", ex);
+            Log.e(TAG, "Could not register unlock receiver " + getComponent(), ex);
         }
     }
 
     private void stopPackageListening() {
-        if (DEBUG) Log.d(TAG, "stopPackageListening");
+        if (mDebug) Log.d(TAG, "stopPackageListening " + getComponent());
         if (mUserReceiverRegistered.compareAndSet(true, false)) {
             mBroadcastDispatcher.unregisterReceiver(this);
         }
@@ -430,7 +460,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onReceive(Context context, Intent intent) {
-        if (DEBUG) Log.d(TAG, "onReceive: " + intent);
+        if (mDebug) Log.d(TAG, "onReceive: " + intent);
         if (!Intent.ACTION_USER_UNLOCKED.equals(intent.getAction())) {
             Uri data = intent.getData();
             String pkgName = data.getEncodedSchemeSpecificPart();
@@ -446,7 +476,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
             if (mBound.get()) {
                 // Trying to bind again will check the state of the package before bothering to
                 // bind.
-                if (DEBUG) Log.d(TAG, "Trying to rebind");
+                if (mDebug) Log.d(TAG, "Trying to rebind " + getComponent());
                 setBindService(true);
             }
 
@@ -458,7 +488,9 @@ public class TileLifecycleManager extends BroadcastReceiver implements
         try {
             ServiceInfo si = mPackageManagerAdapter.getServiceInfo(mIntent.getComponent(),
                     0, mUser.getIdentifier());
-            if (DEBUG && si == null) Log.d(TAG, "Can't find component " + mIntent.getComponent());
+            if (mDebug && si == null) {
+                Log.d(TAG, "Can't find component " + mIntent.getComponent());
+            }
             return si != null;
         } catch (RemoteException e) {
             // Shouldn't happen.
@@ -472,7 +504,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
             mPackageManagerAdapter.getPackageInfoAsUser(packageName, 0, mUser.getIdentifier());
             return true;
         } catch (PackageManager.NameNotFoundException e) {
-            if (DEBUG) {
+            if (mDebug) {
                 Log.d(TAG, "Package not available: " + packageName, e);
             } else {
                 Log.d(TAG, "Package not available: " + packageName);
@@ -489,7 +521,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onTileAdded() {
-        if (DEBUG) Log.d(TAG, "onTileAdded");
+        if (mDebug) Log.d(TAG, "onTileAdded " + getComponent());
         if (mWrapper == null || !mWrapper.onTileAdded()) {
             queueMessage(MSG_ON_ADDED);
             handleDeath();
@@ -498,7 +530,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onTileRemoved() {
-        if (DEBUG) Log.d(TAG, "onTileRemoved");
+        if (mDebug) Log.d(TAG, "onTileRemoved " + getComponent());
         if (mWrapper == null || !mWrapper.onTileRemoved()) {
             queueMessage(MSG_ON_REMOVED);
             handleDeath();
@@ -507,7 +539,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onStartListening() {
-        if (DEBUG) Log.d(TAG, "onStartListening");
+        if (mDebug) Log.d(TAG, "onStartListening " + getComponent());
         mListening = true;
         if (mWrapper != null && !mWrapper.onStartListening()) {
             handleDeath();
@@ -516,7 +548,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onStopListening() {
-        if (DEBUG) Log.d(TAG, "onStopListening");
+        if (mDebug) Log.d(TAG, "onStopListening " + getComponent());
         mListening = false;
         if (mWrapper != null && !mWrapper.onStopListening()) {
             handleDeath();
@@ -525,7 +557,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onClick(IBinder iBinder) {
-        if (DEBUG) Log.d(TAG, "onClick " + iBinder + " " + mUser);
+        if (mDebug) Log.d(TAG, "onClick " + iBinder + " " + getComponent() + " " + mUser);
         if (mWrapper == null || !mWrapper.onClick(iBinder)) {
             mClickBinder = iBinder;
             queueMessage(MSG_ON_CLICK);
@@ -535,7 +567,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void onUnlockComplete() {
-        if (DEBUG) Log.d(TAG, "onUnlockComplete");
+        if (mDebug) Log.d(TAG, "onUnlockComplete " + getComponent());
         if (mWrapper == null || !mWrapper.onUnlockComplete()) {
             queueMessage(MSG_ON_UNLOCK_COMPLETE);
             handleDeath();
@@ -550,7 +582,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
 
     @Override
     public void binderDied() {
-        if (DEBUG) Log.d(TAG, "binderDeath");
+        if (mDebug) Log.d(TAG, "binderDeath " + getComponent());
         handleDeath();
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java
index fbd63c6bbdae..81424565daee 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java
@@ -29,6 +29,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -300,8 +301,10 @@ public class TileLifecycleManagerTest extends SysuiTestCase {
         mStateManager.onStartListening();
         mStateManager.executeSetBindService(true);
         mExecutor.runAllReady();
-        mStateManager.onServiceDisconnected(mTileServiceComponentName);
+        mStateManager.onBindingDied(mTileServiceComponentName);
+        mExecutor.runAllReady();
         mClock.advanceTime(5000);
+        mExecutor.runAllReady();
 
         // Two calls: one for the first bind, one for the restart.
         verifyBind(2);
@@ -318,20 +321,66 @@ public class TileLifecycleManagerTest extends SysuiTestCase {
         mStateManager.onStartListening();
         mStateManager.executeSetBindService(true);
         mExecutor.runAllReady();
-        mStateManager.onServiceDisconnected(mTileServiceComponentName);
+        verify(mMockTileService, times(1)).onStartListening();
+        mStateManager.onBindingDied(mTileServiceComponentName);
+        mExecutor.runAllReady();
 
         // Longer delay than a regular one
         mClock.advanceTime(5000);
-        verifyBind(1);
-        verify(mMockTileService, times(1)).onStartListening();
+        mExecutor.runAllReady();
+
+        assertFalse(mContext.isBound(mTileServiceComponentName));
 
         mClock.advanceTime(20000);
+        mExecutor.runAllReady();
         // Two calls: one for the first bind, one for the restart.
         verifyBind(2);
         verify(mMockTileService, times(2)).onStartListening();
     }
 
     @Test
+    public void testOnServiceDisconnectedDoesnUnbind_doesntForwardToBinder() throws Exception {
+        mStateManager.executeSetBindService(true);
+        mExecutor.runAllReady();
+
+        mStateManager.onStartListening();
+        verify(mMockTileService).onStartListening();
+
+        clearInvocations(mMockTileService);
+        mStateManager.onServiceDisconnected(mTileServiceComponentName);
+        mExecutor.runAllReady();
+
+        mStateManager.onStartListening();
+        verify(mMockTileService, never()).onStartListening();
+    }
+
+    @Test
+    public void testKillProcessLowMemory_unbound_doesntBindAgain() throws Exception {
+        doAnswer(invocation -> {
+            ActivityManager.MemoryInfo memoryInfo = invocation.getArgument(0);
+            memoryInfo.lowMemory = true;
+            return null;
+        }).when(mActivityManager).getMemoryInfo(any());
+        mStateManager.onStartListening();
+        mStateManager.executeSetBindService(true);
+        mExecutor.runAllReady();
+        verifyBind(1);
+        verify(mMockTileService, times(1)).onStartListening();
+
+        mStateManager.onBindingDied(mTileServiceComponentName);
+        mExecutor.runAllReady();
+
+        clearInvocations(mMockTileService);
+        mStateManager.executeSetBindService(false);
+        mExecutor.runAllReady();
+        mClock.advanceTime(30000);
+        mExecutor.runAllReady();
+
+        verifyBind(0);
+        verify(mMockTileService, never()).onStartListening();
+    }
+
+    @Test
     public void testToggleableTile() throws Exception {
         assertTrue(mStateManager.isToggleableTile());
     }