Fix stop behavior in PersistentConnectionManager.

PersistentConnectionManager tries to maintain a connection whenever
disconnected. Currently, there is no way to disconnect from this
connection as the manually unbind is not treated any differently than
any other disconnection. This change addresses this oversight by
suppressing reconnect logic on manual unbinds.

This change also fixes a callback issue where listeners were not
informed about disconnects when manually unbound.

Fixes: 237803638
Test: ObservableServiceConnectionTest#testUnbind
Test: PersistentConnectionManagerTest#testStopDoesNotReconnect
Change-Id: I0887e31754429cc02391df5783868d5c132a48eb

packages/SystemUI/src/com/android/systemui/util/service/ObservableServiceConnection.java

@@ -155,11 +155,6 @@ public class ObservableServiceConnection<T> implements ServiceConnection {
      * Disconnect from the service if bound.
      */
     public void unbind() {
-        if (!mBoundCalled) {
-            return;
-        }
-        mBoundCalled = false;
-        mContext.unbindService(this);
         onDisconnected(DISCONNECT_REASON_UNBIND);
     }
 
@@ -210,12 +205,15 @@ public class ObservableServiceConnection<T> implements ServiceConnection {
             Log.d(TAG, "onDisconnected:" + reason);
         }
 
+        // If not bound or already unbound, do not proceed setting reason, unbinding, and
+        // notifying
         if (!mBoundCalled) {
             return;
         }
 
+        mBoundCalled = false;
         mLastDisconnectReason = Optional.of(reason);
-        unbind();
+        mContext.unbindService(this);
         mProxy = null;
 
         applyToCallbacksLocked(callback-> callback.onDisconnected(this,

packages/SystemUI/src/com/android/systemui/util/service/PersistentConnectionManager.java

@@ -72,6 +72,11 @@ public class PersistentConnectionManager<T> {
 
         @Override
         public void onDisconnected(ObservableServiceConnection connection, int reason) {
+            // Do not attempt to reconnect if we were manually unbound
+            if (reason == ObservableServiceConnection.DISCONNECT_REASON_UNBIND) {
+                return;
+            }
+
             if (mSystemClock.currentTimeMillis() - mStartTime > mMinConnectionDuration) {
                 initiateConnectionAttempt();
             } else {

packages/SystemUI/tests/src/com/android/systemui/util/service/ObservableServiceConnectionTest.java

@@ -145,4 +145,28 @@ public class ObservableServiceConnectionTest extends SysuiTestCase {
         connection.unbind();
         verify(mContext, never()).unbindService(eq(connection));
     }
+
+    @Test
+    public void testUnbind() {
+        ObservableServiceConnection<Foo> connection = new ObservableServiceConnection<>(mContext,
+                mIntent, mExecutor, mTransformer);
+        connection.addCallback(mCallback);
+        connection.onServiceDisconnected(mComponentName);
+
+        // Disconnects before binds should be ignored.
+        verify(mCallback, never()).onDisconnected(eq(connection), anyInt());
+
+        when(mContext.bindService(eq(mIntent), anyInt(), eq(mExecutor), eq(connection)))
+                .thenReturn(true);
+        connection.bind();
+
+        mExecutor.runAllReady();
+
+        connection.unbind();
+
+        mExecutor.runAllReady();
+
+        verify(mCallback).onDisconnected(eq(connection),
+                eq(ObservableServiceConnection.DISCONNECT_REASON_UNBIND));
+    }
 }

packages/SystemUI/tests/src/com/android/systemui/util/service/PersistentConnectionManagerTest.java

@@ -16,6 +16,7 @@
 
 package com.android.systemui.util.service;
 
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
 import android.testing.AndroidTestingRunner;
@@ -119,6 +120,24 @@ public class PersistentConnectionManagerTest extends SysuiTestCase {
         }
     }
 
+    /**
+     * Ensures manual unbind does not reconnect.
+     */
+    @Test
+    public void testStopDoesNotReconnect() {
+        mConnectionManager.start();
+        ArgumentCaptor<ObservableServiceConnection.Callback<Proxy>> connectionCallbackCaptor =
+                ArgumentCaptor.forClass(ObservableServiceConnection.Callback.class);
+
+        verify(mConnection).addCallback(connectionCallbackCaptor.capture());
+        verify(mConnection).bind();
+        Mockito.clearInvocations(mConnection);
+        mConnectionManager.stop();
+        mFakeExecutor.advanceClockToNext();
+        mFakeExecutor.runAllReady();
+        verify(mConnection, never()).bind();
+    }
+
     /**
      * Ensures rebind on package change.
      */