Merge "Fix stop behavior in PersistentConnectionManager." into tm-qpr-dev am: 91079f446e

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/19153178

Change-Id: Ic4a96ae32ae6f9d33ff0d625b4d4811fa2d81d94
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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.
      */