OSDN Git Service

Binding on-demand #9: selectBackupTransport[Async]
authorBernardo Rufino <brufino@google.com>
Tue, 2 Jan 2018 15:53:44 +0000 (15:53 +0000)
committerBernardo Rufino <brufino@google.com>
Tue, 9 Jan 2018 12:37:06 +0000 (12:37 +0000)
Migrate selectBackupTransport()/selectBackupTransportAsync() to binding
on-demand. To mimic the bind-if-needed behavior that existed before for
selectBackupTransportAsync we now register-if-needed in the selection.
This means a new registerTransport() method was created in the
TransportManager. I intend to use this method with only few
modifications for the first-binding code.

Change-Id: I39661cff0f7b2f8a27da37905dcd93e0aa9b1178
Ref: http://go/br-binding-on-demand
Bug: 17140907
Test: m -j RunFrameworksServicesRoboTest
Test: adb shell bmgr transport <transport>, observe logs
Test: Robolectric tests for TransportManager will go in registration CL

services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java
services/backup/java/com/android/server/backup/TransportManager.java
services/backup/java/com/android/server/backup/internal/PerformBackupTask.java
services/robotests/src/com/android/server/backup/BackupManagerServiceRoboTest.java
services/robotests/src/com/android/server/backup/TransportManagerTest.java
services/robotests/src/com/android/server/backup/testing/TransportReadyCallbackStub.java [deleted file]

index b99d341..03591a8 100644 (file)
@@ -3001,63 +3001,63 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
         }
     }
 
-    // Select which transport to use for the next backup operation.
+    /** Selects transport {@code transportName} and returns previous selected transport. */
     @Override
-    public String selectBackupTransport(String transport) {
-        mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP,
-                "selectBackupTransport");
+    public String selectBackupTransport(String transportName) {
+        mContext.enforceCallingOrSelfPermission(
+                android.Manifest.permission.BACKUP, "selectBackupTransport");
 
         final long oldId = Binder.clearCallingIdentity();
         try {
-            String prevTransport = mTransportManager.selectTransport(transport);
-            updateStateForTransport(transport);
-            Slog.v(TAG, "selectBackupTransport() set " + mTransportManager.getCurrentTransportName()
-                    + " returning " + prevTransport);
-            return prevTransport;
+            String previousTransportName = mTransportManager.selectTransport(transportName);
+            updateStateForTransport(transportName);
+            Slog.v(TAG, "selectBackupTransport(transport = " + transportName
+                    + "): previous transport = " + previousTransportName);
+            return previousTransportName;
         } finally {
             Binder.restoreCallingIdentity(oldId);
         }
     }
 
     @Override
-    public void selectBackupTransportAsync(final ComponentName transport,
-            final ISelectBackupTransportCallback listener) {
-        mContext.enforceCallingOrSelfPermission(android.Manifest.permission.BACKUP,
-                "selectBackupTransportAsync");
+    public void selectBackupTransportAsync(
+            ComponentName transportComponent, ISelectBackupTransportCallback listener) {
+        mContext.enforceCallingOrSelfPermission(
+                android.Manifest.permission.BACKUP, "selectBackupTransportAsync");
 
         final long oldId = Binder.clearCallingIdentity();
-
-        Slog.v(TAG, "selectBackupTransportAsync() called with transport " +
-                transport.flattenToShortString());
-
-        mTransportManager.ensureTransportReady(transport,
-                new TransportManager.TransportReadyCallback() {
-                    @Override
-                    public void onSuccess(String transportName) {
-                        mTransportManager.selectTransport(transportName);
-                        updateStateForTransport(mTransportManager.getCurrentTransportName());
-                        Slog.v(TAG, "Transport successfully selected: "
-                                + transport.flattenToShortString());
-                        try {
-                            listener.onSuccess(transportName);
-                        } catch (RemoteException e) {
-                            // Nothing to do here.
+        try {
+            String transportString = transportComponent.flattenToShortString();
+            Slog.v(TAG, "selectBackupTransportAsync(transport = " + transportString + ")");
+            mBackupHandler.post(
+                    () -> {
+                        String transportName = null;
+                        int result =
+                                mTransportManager.registerAndSelectTransport(transportComponent);
+                        if (result == BackupManager.SUCCESS) {
+                            try {
+                                transportName =
+                                        mTransportManager.getTransportName(transportComponent);
+                                updateStateForTransport(transportName);
+                            } catch (TransportNotRegisteredException e) {
+                                Slog.e(TAG, "Transport got unregistered");
+                                result = BackupManager.ERROR_TRANSPORT_UNAVAILABLE;
+                            }
                         }
-                    }
 
-                    @Override
-                    public void onFailure(int reason) {
-                        Slog.v(TAG,
-                                "Failed to select transport: " + transport.flattenToShortString());
                         try {
-                            listener.onFailure(reason);
+                            if (transportName != null) {
+                                listener.onSuccess(transportName);
+                            } else {
+                                listener.onFailure(result);
+                            }
                         } catch (RemoteException e) {
-                            // Nothing to do here.
+                            Slog.e(TAG, "ISelectBackupTransportCallback listener not available");
                         }
-                    }
-                });
-
-        Binder.restoreCallingIdentity(oldId);
+                    });
+        } finally {
+            Binder.restoreCallingIdentity(oldId);
+        }
     }
 
     private void updateStateForTransport(String newTransportName) {
@@ -3066,18 +3066,23 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter
                 Settings.Secure.BACKUP_TRANSPORT, newTransportName);
 
         // And update our current-dataset bookkeeping
-        IBackupTransport transport = mTransportManager.getTransportBinder(newTransportName);
-        if (transport != null) {
+        String callerLogString = "BMS.updateStateForTransport()";
+        TransportClient transportClient =
+                mTransportManager.getTransportClient(newTransportName, callerLogString);
+        if (transportClient != null) {
             try {
+                IBackupTransport transport = transportClient.connectOrThrow(callerLogString);
                 mCurrentToken = transport.getCurrentRestoreSet();
             } catch (Exception e) {
                 // Oops.  We can't know the current dataset token, so reset and figure it out
                 // when we do the next k/v backup operation on this transport.
                 mCurrentToken = 0;
+                Slog.w(TAG, "Transport " + newTransportName + " not available: current token = 0");
             }
         } else {
-            // The named transport isn't bound at this particular moment, so we can't
-            // know yet what its current dataset token is.  Reset as above.
+            Slog.w(TAG, "Transport " + newTransportName + " not registered: current token = 0");
+            // The named transport isn't registered, so we can't know what its current dataset token
+            // is. Reset as above.
             mCurrentToken = 0;
         }
     }
index 72a3a32..34b8935 100644 (file)
@@ -112,23 +112,6 @@ public class TransportManager {
     @GuardedBy("mTransportLock")
     private volatile String mCurrentTransportName;
 
-
-    /**
-     * Callback interface for {@link #ensureTransportReady(ComponentName, TransportReadyCallback)}.
-     */
-    public interface TransportReadyCallback {
-
-        /**
-         * Will be called when the transport is ready.
-         */
-        void onSuccess(String transportName);
-
-        /**
-         * Will be called when it's not possible to make transport ready.
-         */
-        void onFailure(int reason);
-    }
-
     TransportManager(
             Context context,
             Set<ComponentName> whitelist,
@@ -240,6 +223,17 @@ public class TransportManager {
     }
 
     /**
+     * Returns the transport name associated with {@code transportComponent}.
+     * @throws TransportNotRegisteredException if the transport is not registered.
+     */
+    public String getTransportName(ComponentName transportComponent)
+            throws TransportNotRegisteredException {
+        synchronized (mTransportLock) {
+            return getRegisteredTransportDescriptionOrThrowLocked(transportComponent).name;
+        }
+    }
+
+    /**
      * Retrieve the configuration intent of {@code transportName}.
      * @throws TransportNotRegisteredException if the transport is not registered.
      */
@@ -340,23 +334,6 @@ public class TransportManager {
         return null;
     }
 
-    /**
-     * Returns the transport name associated with {@param transportComponent} or {@code null} if not
-     * found.
-     */
-    @Nullable
-    public String getTransportName(ComponentName transportComponent) {
-        synchronized (mTransportLock) {
-            TransportDescription description =
-                    mRegisteredTransportsDescriptionMap.get(transportComponent);
-            if (description == null) {
-                Slog.e(TAG, "Trying to find name of unregistered transport " + transportComponent);
-                return null;
-            }
-            return description.name;
-        }
-    }
-
     @GuardedBy("mTransportLock")
     @Nullable
     private ComponentName getRegisteredTransportComponentLocked(String transportName) {
@@ -552,29 +529,6 @@ public class TransportManager {
         return mCurrentTransportName;
     }
 
-    String selectTransport(String transport) {
-        synchronized (mTransportLock) {
-            String prevTransport = mCurrentTransportName;
-            mCurrentTransportName = transport;
-            return prevTransport;
-        }
-    }
-
-    void ensureTransportReady(ComponentName transportComponent,
-            TransportReadyCallback listener) {
-        synchronized (mTransportLock) {
-            TransportConnection conn = mValidTransports.get(transportComponent);
-            if (conn == null) {
-                listener.onFailure(BackupManager.ERROR_TRANSPORT_UNAVAILABLE);
-                return;
-            }
-            // Transport can be unbound if the process hosting it crashed.
-            conn.bindIfUnbound();
-            conn.addListener(listener);
-        }
-    }
-
-
     // This is for mocking, Mockito can't mock if package-protected and in the same package but
     // different class loaders. Checked with the debugger and class loaders are different
     // See https://github.com/mockito/mockito/issues/796
@@ -674,6 +628,90 @@ public class TransportManager {
                 createSystemUserHandle());
     }
 
+    String selectTransport(String transportName) {
+        synchronized (mTransportLock) {
+            String prevTransport = mCurrentTransportName;
+            mCurrentTransportName = transportName;
+            return prevTransport;
+        }
+    }
+
+    /**
+     * Tries to register the transport if not registered. If successful also selects the transport.
+     *
+     * @param transportComponent Host of the transport.
+     * @return One of {@link BackupManager#SUCCESS}, {@link BackupManager#ERROR_TRANSPORT_INVALID}
+     *     or {@link BackupManager#ERROR_TRANSPORT_UNAVAILABLE}.
+     */
+    public int registerAndSelectTransport(ComponentName transportComponent) {
+        synchronized (mTransportLock) {
+            if (!mRegisteredTransportsDescriptionMap.containsKey(transportComponent)) {
+                int result = registerTransport(transportComponent);
+                if (result != BackupManager.SUCCESS) {
+                    return result;
+                }
+            }
+
+            try {
+                selectTransport(getTransportName(transportComponent));
+                return BackupManager.SUCCESS;
+            } catch (TransportNotRegisteredException e) {
+                // Shouldn't happen because we are holding the lock
+                Slog.wtf(TAG, "Transport unexpectedly not registered");
+                return BackupManager.ERROR_TRANSPORT_UNAVAILABLE;
+            }
+        }
+    }
+
+    /**
+     * Tries to register transport represented by {@code transportComponent}.
+     *
+     * @param transportComponent Host of the transport that we want to register.
+     * @return One of {@link BackupManager#SUCCESS}, {@link BackupManager#ERROR_TRANSPORT_INVALID}
+     *     or {@link BackupManager#ERROR_TRANSPORT_UNAVAILABLE}.
+     */
+    private int registerTransport(ComponentName transportComponent) {
+        String transportString = transportComponent.flattenToShortString();
+
+        String callerLogString = "TransportManager.registerTransport()";
+        TransportClient transportClient =
+                mTransportClientManager.getTransportClient(transportComponent, callerLogString);
+
+        final IBackupTransport transport;
+        try {
+            transport = transportClient.connectOrThrow(callerLogString);
+        } catch (TransportNotAvailableException e) {
+            Slog.e(TAG, "Couldn't connect to transport " + transportString + " for registration");
+            mTransportClientManager.disposeOfTransportClient(transportClient, callerLogString);
+            return BackupManager.ERROR_TRANSPORT_UNAVAILABLE;
+        }
+
+        EventLog.writeEvent(EventLogTags.BACKUP_TRANSPORT_LIFECYCLE, transportString, 1);
+
+        int result;
+        if (isTransportValid(transport)) {
+            try {
+                registerTransport(transportComponent, transport);
+                // If registerTransport() hasn't thrown...
+                result = BackupManager.SUCCESS;
+            } catch (RemoteException e) {
+                Slog.e(TAG, "Transport " + transportString + " died while registering");
+                result = BackupManager.ERROR_TRANSPORT_UNAVAILABLE;
+            }
+        } else {
+            Slog.w(TAG, "Can't register invalid transport " + transportString);
+            result = BackupManager.ERROR_TRANSPORT_INVALID;
+        }
+
+        mTransportClientManager.disposeOfTransportClient(transportClient, callerLogString);
+        if (result == BackupManager.SUCCESS) {
+            Slog.d(TAG, "Transport " + transportString + " registered");
+        } else {
+            EventLog.writeEvent(EventLogTags.BACKUP_TRANSPORT_LIFECYCLE, transportString, 0);
+        }
+        return result;
+    }
+
     /** If {@link RemoteException} is thrown the transport is guaranteed to not be registered. */
     private void registerTransport(ComponentName transportComponent, IBackupTransport transport)
             throws RemoteException {
@@ -690,11 +728,18 @@ public class TransportManager {
         }
     }
 
+    private boolean isTransportValid(IBackupTransport transport) {
+        if (mTransportBoundListener == null) {
+            Slog.w(TAG, "setTransportBoundListener() not called, assuming transport invalid");
+            return false;
+        }
+        return mTransportBoundListener.onTransportBound(transport);
+    }
+
     private class TransportConnection implements ServiceConnection {
 
         // Hold mTransportLock to access these fields so as to provide a consistent view of them.
         private volatile IBackupTransport mBinder;
-        private final List<TransportReadyCallback> mListeners = new ArrayList<>();
         private volatile String mTransportName;
 
         private final ComponentName mTransportComponent;
@@ -716,15 +761,7 @@ public class TransportManager {
                     mTransportName = mBinder.name();
                     // BackupManager requests some fields from the transport. If they are
                     // invalid, throw away this transport.
-                    final boolean valid;
-                    if (mTransportBoundListener != null) {
-                        valid = mTransportBoundListener.onTransportBound(mBinder);
-                    } else {
-                        Slog.w(TAG, "setTransportBoundListener() not called, assuming transport "
-                                + component + " valid");
-                        valid = true;
-                    }
-                    if (valid) {
+                    if (isTransportValid(mBinder)) {
                         // We're now using the always-bound connection to do the registration but
                         // when we remove the always-bound code this will be in the first binding
                         // TODO: Move registration to first binding
@@ -742,9 +779,6 @@ public class TransportManager {
                     if (success) {
                         Slog.d(TAG, "Bound to transport: " + componentShortString);
                         mBoundTransports.put(mTransportName, component);
-                        for (TransportReadyCallback listener : mListeners) {
-                            listener.onSuccess(mTransportName);
-                        }
                         // cancel rebinding on timeout for this component as we've already connected
                         mHandler.removeMessages(REBINDING_TIMEOUT_MSG, componentShortString);
                     } else {
@@ -756,11 +790,7 @@ public class TransportManager {
                         mValidTransports.remove(component);
                         mEligibleTransports.remove(component);
                         mBinder = null;
-                        for (TransportReadyCallback listener : mListeners) {
-                            listener.onFailure(BackupManager.ERROR_TRANSPORT_INVALID);
-                        }
                     }
-                    mListeners.clear();
                 }
             }
         }
@@ -815,19 +845,6 @@ public class TransportManager {
             }
         }
 
-        private void addListener(TransportReadyCallback listener) {
-            synchronized (mTransportLock) {
-                if (mBinder == null) {
-                    // We are waiting for bind to complete. If mBinder is set to null after the bind
-                    // is complete due to transport being invalid, we won't find 'this' connection
-                    // object in mValidTransports list and this function can't be called.
-                    mListeners.add(listener);
-                } else {
-                    listener.onSuccess(mTransportName);
-                }
-            }
-        }
-
         private long getRebindTimeout() {
             final boolean isDeviceProvisioned = Settings.Global.getInt(
                     mContext.getContentResolver(),
index a002334..c232241 100644 (file)
@@ -560,16 +560,9 @@ public class PerformBackupTask implements BackupRestoreTask {
                 }
                 backupManagerService.addBackupTrace("init required; rerunning");
                 try {
-                    final String name = backupManagerService.getTransportManager()
+                    String name = backupManagerService.getTransportManager()
                             .getTransportName(mTransportClient.getTransportComponent());
-                    if (name != null) {
-                        backupManagerService.getPendingInits().add(name);
-                    } else {
-                        if (DEBUG) {
-                            Slog.w(TAG, "Couldn't find name of transport "
-                                    + mTransportClient.getTransportComponent() + " for init");
-                        }
-                    }
+                    backupManagerService.getPendingInits().add(name);
                 } catch (Exception e) {
                     Slog.w(TAG, "Failed to query transport name for init: " + e.getMessage());
                     // swallow it and proceed; we don't rely on this
index d9e8f0f..f9ebd28 100644 (file)
 
 package com.android.server.backup;
 
+import static com.android.server.backup.testing.TransportTestUtils.TRANSPORT_NAMES;
+
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.robolectric.Shadows.shadowOf;
 import static org.testng.Assert.expectThrows;
 
+import android.app.backup.BackupManager;
+import android.app.backup.ISelectBackupTransportCallback;
+import android.content.ComponentName;
+import android.content.Context;
 import android.content.ContextWrapper;
 import android.os.HandlerThread;
 import android.platform.test.annotations.Presubmit;
+import android.provider.Settings;
 
 import com.android.server.backup.testing.ShadowAppBackupUtils;
 import com.android.server.backup.testing.TransportTestUtils;
@@ -42,13 +52,15 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
-import org.robolectric.Shadows;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowContextWrapper;
 import org.robolectric.shadows.ShadowLog;
+import org.robolectric.shadows.ShadowLooper;
+import org.robolectric.shadows.ShadowSettings;
 
 import java.io.File;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 @RunWith(FrameworkRobolectricTestRunner.class)
@@ -66,10 +78,12 @@ public class BackupManagerServiceRoboTest {
 
     @Mock private TransportManager mTransportManager;
     private HandlerThread mBackupThread;
+    private ShadowLooper mShadowBackupLooper;
     private File mBaseStateDir;
     private File mDataDir;
     private RefactoredBackupManagerService mBackupManagerService;
     private ShadowContextWrapper mShadowContext;
+    private Context mContext;
 
     @Before
     public void setUp() throws Exception {
@@ -79,18 +93,20 @@ public class BackupManagerServiceRoboTest {
         mBackupThread.setUncaughtExceptionHandler(
                 (t, e) -> ShadowLog.e(TAG, "Uncaught exception in test thread " + t.getName(), e));
         mBackupThread.start();
+        mShadowBackupLooper = shadowOf(mBackupThread.getLooper());
 
         ContextWrapper context = RuntimeEnvironment.application;
-        mShadowContext = Shadows.shadowOf(context);
+        mContext = context;
+        mShadowContext = shadowOf(context);
 
-        File cacheDir = context.getCacheDir();
+        File cacheDir = mContext.getCacheDir();
         mBaseStateDir = new File(cacheDir, "base_state_dir");
         mDataDir = new File(cacheDir, "data_dir");
 
         mBackupManagerService =
                 new RefactoredBackupManagerService(
-                        context,
-                        new Trampoline(context),
+                        mContext,
+                        new Trampoline(mContext),
                         mBackupThread,
                         mBaseStateDir,
                         mDataDir,
@@ -103,6 +119,8 @@ public class BackupManagerServiceRoboTest {
         ShadowAppBackupUtils.reset();
     }
 
+    /* Tests for destination string */
+
     @Test
     public void testDestinationString() throws Exception {
         mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
@@ -136,6 +154,8 @@ public class BackupManagerServiceRoboTest {
                 () -> mBackupManagerService.getDestinationString(TRANSPORT_NAME));
     }
 
+    /* Tests for app eligibility */
+
     @Test
     public void testIsAppEligibleForBackup_whenAppEligible() throws Exception {
         mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
@@ -212,4 +232,110 @@ public class BackupManagerServiceRoboTest {
                         mBackupManagerService.filterAppsEligibleForBackup(
                                 new String[] {"package.a", "package.b"}));
     }
+
+    /* Tests for select transport */
+
+    private TransportData mNewTransport;
+    private TransportData mOldTransport;
+    private ComponentName mNewTransportComponent;
+    private ISelectBackupTransportCallback mCallback;
+
+    private void setUpForSelectTransport() throws Exception {
+        List<TransportData> transports =
+                TransportTestUtils.setUpTransports(mTransportManager, TRANSPORT_NAMES);
+        mNewTransport = transports.get(0);
+        mNewTransportComponent = mNewTransport.transportClientMock.getTransportComponent();
+        mOldTransport = transports.get(1);
+        when(mTransportManager.selectTransport(eq(mNewTransport.transportName)))
+                .thenReturn(mOldTransport.transportName);
+    }
+
+    @Test
+    public void testSelectBackupTransport() throws Exception {
+        setUpForSelectTransport();
+        mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
+
+        String oldTransport =
+                mBackupManagerService.selectBackupTransport(mNewTransport.transportName);
+
+        assertThat(getSettingsTransport()).isEqualTo(mNewTransport.transportName);
+        assertThat(oldTransport).isEqualTo(mOldTransport.transportName);
+    }
+
+    @Test
+    public void testSelectBackupTransport_withoutPermission() throws Exception {
+        setUpForSelectTransport();
+        mShadowContext.denyPermissions(android.Manifest.permission.BACKUP);
+
+        expectThrows(
+                SecurityException.class,
+                () -> mBackupManagerService.selectBackupTransport(mNewTransport.transportName));
+    }
+
+    @Test
+    public void testSelectBackupTransportAsync() throws Exception {
+        setUpForSelectTransport();
+        mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
+        when(mTransportManager.registerAndSelectTransport(eq(mNewTransportComponent)))
+                .thenReturn(BackupManager.SUCCESS);
+        ISelectBackupTransportCallback callback = mock(ISelectBackupTransportCallback.class);
+
+        mBackupManagerService.selectBackupTransportAsync(mNewTransportComponent, callback);
+
+        mShadowBackupLooper.runToEndOfTasks();
+        assertThat(getSettingsTransport()).isEqualTo(mNewTransport.transportName);
+        verify(callback).onSuccess(eq(mNewTransport.transportName));
+    }
+
+    @Test
+    public void testSelectBackupTransportAsync_whenRegistrationFails() throws Exception {
+        setUpForSelectTransport();
+        mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
+        when(mTransportManager.registerAndSelectTransport(eq(mNewTransportComponent)))
+                .thenReturn(BackupManager.ERROR_TRANSPORT_UNAVAILABLE);
+        ISelectBackupTransportCallback callback = mock(ISelectBackupTransportCallback.class);
+
+        mBackupManagerService.selectBackupTransportAsync(mNewTransportComponent, callback);
+
+        mShadowBackupLooper.runToEndOfTasks();
+        assertThat(getSettingsTransport()).isNotEqualTo(mNewTransport.transportName);
+        verify(callback).onFailure(anyInt());
+    }
+
+    @Test
+    public void testSelectBackupTransportAsync_whenTransportGetsUnregistered() throws Exception {
+        TransportTestUtils.setUpTransports(
+                mTransportManager, new TransportData(TRANSPORT_NAME, null, null));
+        ComponentName newTransportComponent =
+                TransportTestUtils.transportComponentName(TRANSPORT_NAME);
+        mShadowContext.grantPermissions(android.Manifest.permission.BACKUP);
+        when(mTransportManager.registerAndSelectTransport(eq(newTransportComponent)))
+                .thenReturn(BackupManager.SUCCESS);
+        ISelectBackupTransportCallback callback = mock(ISelectBackupTransportCallback.class);
+
+        mBackupManagerService.selectBackupTransportAsync(newTransportComponent, callback);
+
+        mShadowBackupLooper.runToEndOfTasks();
+        assertThat(getSettingsTransport()).isNotEqualTo(TRANSPORT_NAME);
+        verify(callback).onFailure(anyInt());
+    }
+
+    @Test
+    public void testSelectBackupTransportAsync_withoutPermission() throws Exception {
+        setUpForSelectTransport();
+        mShadowContext.denyPermissions(android.Manifest.permission.BACKUP);
+        ComponentName newTransportComponent =
+                mNewTransport.transportClientMock.getTransportComponent();
+
+        expectThrows(
+                SecurityException.class,
+                () ->
+                        mBackupManagerService.selectBackupTransportAsync(
+                                newTransportComponent, mock(ISelectBackupTransportCallback.class)));
+    }
+
+    private String getSettingsTransport() {
+        return ShadowSettings.ShadowSecure.getString(
+                mContext.getContentResolver(), Settings.Secure.BACKUP_TRANSPORT);
+    }
 }
index 82830fe..acd670f 100644 (file)
@@ -24,7 +24,6 @@ import static org.robolectric.shadow.api.Shadow.extract;
 import static org.testng.Assert.expectThrows;
 
 import android.annotation.Nullable;
-import android.app.backup.BackupManager;
 import android.content.ComponentName;
 import android.content.Intent;
 import android.content.pm.ApplicationInfo;
@@ -40,7 +39,6 @@ import com.android.server.backup.testing.ShadowBackupTransportStub;
 import com.android.server.backup.testing.ShadowContextImplForBackup;
 import com.android.server.backup.testing.ShadowPackageManagerForBackup;
 import com.android.server.backup.testing.TransportBoundListenerStub;
-import com.android.server.backup.testing.TransportReadyCallbackStub;
 import com.android.server.backup.transport.TransportClient;
 import com.android.server.backup.transport.TransportNotRegisteredException;
 import com.android.server.testing.FrameworkRobolectricTestRunner;
@@ -87,9 +85,6 @@ public class TransportManagerTest {
     private final TransportBoundListenerStub mTransportBoundListenerStub =
             new TransportBoundListenerStub(true);
 
-    private final TransportReadyCallbackStub mTransportReadyCallbackStub =
-            new TransportReadyCallbackStub();
-
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -466,59 +461,6 @@ public class TransportManagerTest {
     }
 
     @Test
-    public void ensureTransportReady_transportNotYetBound_callsListenerOnFailure()
-            throws Exception {
-        setUpPackageWithTransports(PACKAGE_NAME, Arrays.asList(mTransport1, mTransport2),
-                ApplicationInfo.PRIVATE_FLAG_PRIVILEGED);
-
-        TransportManager transportManager = new TransportManager(
-                RuntimeEnvironment.application.getApplicationContext(),
-                new HashSet<>(Arrays.asList(mTransport1.componentName, mTransport2.componentName)),
-                mTransport1.name,
-                mTransportBoundListenerStub,
-                ShadowLooper.getMainLooper());
-
-        transportManager.ensureTransportReady(mTransport1.componentName,
-                mTransportReadyCallbackStub);
-
-        assertThat(mTransportReadyCallbackStub.getSuccessCalls()).isEmpty();
-        assertThat(mTransportReadyCallbackStub.getFailureCalls()).containsExactlyElementsIn(
-                Collections.singleton(
-                        BackupManager.ERROR_TRANSPORT_UNAVAILABLE));
-    }
-
-    @Test
-    public void ensureTransportReady_transportCannotBeBound_callsListenerOnFailure()
-            throws Exception {
-        TransportManager transportManager =
-                createTransportManagerAndSetUpTransports(Collections.singletonList(mTransport2),
-                        Collections.singletonList(mTransport1), mTransport1.name);
-
-        transportManager.ensureTransportReady(mTransport1.componentName,
-                mTransportReadyCallbackStub);
-
-        assertThat(mTransportReadyCallbackStub.getSuccessCalls()).isEmpty();
-        assertThat(mTransportReadyCallbackStub.getFailureCalls()).containsExactlyElementsIn(
-                Collections.singleton(
-                        BackupManager.ERROR_TRANSPORT_UNAVAILABLE));
-    }
-
-    @Test
-    public void ensureTransportReady_transportsAlreadyBound_callsListenerOnSuccess()
-            throws Exception {
-        TransportManager transportManager =
-                createTransportManagerAndSetUpTransports(Collections.singletonList(mTransport2),
-                        Collections.singletonList(mTransport1), mTransport1.name);
-
-        transportManager.ensureTransportReady(mTransport2.componentName,
-                mTransportReadyCallbackStub);
-
-        assertThat(mTransportReadyCallbackStub.getSuccessCalls()).containsExactlyElementsIn(
-                Collections.singleton(mTransport2.name));
-        assertThat(mTransportReadyCallbackStub.getFailureCalls()).isEmpty();
-    }
-
-    @Test
     public void getTransportClient_forRegisteredTransport_returnCorrectly() throws Exception {
         TransportManager transportManager =
                 createTransportManagerAndSetUpTransports(
diff --git a/services/robotests/src/com/android/server/backup/testing/TransportReadyCallbackStub.java b/services/robotests/src/com/android/server/backup/testing/TransportReadyCallbackStub.java
deleted file mode 100644 (file)
index bbe7eba..0000000
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 2017 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.server.backup.testing;
-
-import com.android.server.backup.TransportManager;
-
-import java.util.HashSet;
-import java.util.Set;
-
-/**
- * Stub implementation of TransportReadyCallback, which can tell which calls were made.
- */
-public class TransportReadyCallbackStub implements
-        TransportManager.TransportReadyCallback {
-    private final Set<String> mSuccessCalls = new HashSet<>();
-    private final Set<Integer> mFailureCalls = new HashSet<>();
-
-    @Override
-    public void onSuccess(String transportName) {
-        mSuccessCalls.add(transportName);
-    }
-
-    @Override
-    public void onFailure(int reason) {
-        mFailureCalls.add(reason);
-    }
-
-    /**
-     * Returns set of transport names for which {@link #onSuccess(String)} was called.
-     */
-    public Set<String> getSuccessCalls() {
-        return mSuccessCalls;
-    }
-
-    /**
-     * Returns set of reasons for which {@link #onFailure(int)} } was called.
-     */
-    public Set<Integer> getFailureCalls() {
-        return mFailureCalls;
-    }
-}