Pipe requestSetProviderAllowed through
Add piping to connect requestSetProviderAllowed through
MockableLocatioProvider.
Test: presubmits
Change-Id: I323d7540a0af4d0d12b26ee1a37cb247a90d7141
diff --git a/services/core/java/com/android/server/LocationManagerService.java b/services/core/java/com/android/server/LocationManagerService.java
index 3bc93cc..e342dd7 100644
--- a/services/core/java/com/android/server/LocationManagerService.java
+++ b/services/core/java/com/android/server/LocationManagerService.java
@@ -649,7 +649,7 @@
mEnabled = new SparseArray<>(1);
// initialize last since this lets our reference escape
- mProvider = new MockableLocationProvider(mContext, mLock, this);
+ mProvider = new MockableLocationProvider(mLock, this);
// we can assume all users start with disabled location state since the initial state
// of all providers is disabled. no need to initialize mEnabled further.
@@ -2699,7 +2699,7 @@
mProviderManagers.add(manager);
}
- manager.setMockProvider(new MockProvider(mContext, properties));
+ manager.setMockProvider(new MockProvider(properties));
}
}
diff --git a/services/core/java/com/android/server/location/AbstractLocationProvider.java b/services/core/java/com/android/server/location/AbstractLocationProvider.java
index 64bca78..997f21c 100644
--- a/services/core/java/com/android/server/location/AbstractLocationProvider.java
+++ b/services/core/java/com/android/server/location/AbstractLocationProvider.java
@@ -177,7 +177,6 @@
}
}
- protected final Context mContext;
protected final Executor mExecutor;
// we use a lock-free implementation to update state to ensure atomicity between updating the
@@ -186,13 +185,11 @@
// before it was set, and should not miss any updates that occur after it was set).
private final AtomicReference<InternalState> mInternalState;
- protected AbstractLocationProvider(Context context, Executor executor) {
- this(context, executor, Collections.singleton(context.getPackageName()));
+ protected AbstractLocationProvider(Executor executor, Context context) {
+ this(executor, Collections.singleton(context.getPackageName()));
}
- protected AbstractLocationProvider(Context context, Executor executor,
- Set<String> packageNames) {
- mContext = context;
+ protected AbstractLocationProvider(Executor executor, Set<String> packageNames) {
mExecutor = executor;
mInternalState = new AtomicReference<>(
new InternalState(null, State.EMPTY_STATE.withProviderPackageNames(packageNames)));
@@ -202,7 +199,7 @@
* Sets the listener and returns the state at the moment the listener was set. The listener can
* expect to receive all state updates from after this point.
*/
- State setListener(@Nullable Listener listener) {
+ protected State setListener(@Nullable Listener listener) {
return mInternalState.updateAndGet(
internalState -> internalState.withListener(listener)).state;
}
@@ -210,14 +207,14 @@
/**
* Retrieves the state of the provider.
*/
- State getState() {
+ public State getState() {
return mInternalState.get().state;
}
/**
* Sets the state of the provider to the new state.
*/
- void setState(State newState) {
+ protected void setState(State newState) {
InternalState oldInternalState = mInternalState.getAndUpdate(
internalState -> internalState.withState(newState));
if (newState.equals(oldInternalState.state)) {
@@ -357,7 +354,7 @@
/**
* Always invoked on the provider executor.
*/
- protected void onExtraCommand(int uid, int pid, String command, Bundle extras) {}
+ protected abstract void onExtraCommand(int uid, int pid, String command, Bundle extras);
/**
* Requests a provider to enable itself for the given user id.
@@ -370,7 +367,7 @@
/**
* Always invoked on the provider executor.
*/
- protected void onRequestSetAllowed(boolean allowed) {}
+ protected abstract void onRequestSetAllowed(boolean allowed);
/**
* Dumps debug or log information. May be invoked from any thread.
diff --git a/services/core/java/com/android/server/location/GnssLocationProvider.java b/services/core/java/com/android/server/location/GnssLocationProvider.java
index 62fa5ec..d14cd4c 100644
--- a/services/core/java/com/android/server/location/GnssLocationProvider.java
+++ b/services/core/java/com/android/server/location/GnssLocationProvider.java
@@ -408,7 +408,7 @@
// Available only on GNSS HAL 2.0 implementations and later.
private GnssVisibilityControl mGnssVisibilityControl;
- // Handler for processing events
+ private final Context mContext;
private Handler mHandler;
private final GnssNetworkConnectivityHandler mNetworkConnectivityHandler;
@@ -625,10 +625,11 @@
}
public GnssLocationProvider(Context context) {
- super(context, FgThread.getExecutor());
+ super(FgThread.getExecutor(), context);
ensureInitialized();
+ mContext = context;
mLooper = FgThread.getHandler().getLooper();
// Create a wake lock
@@ -1212,6 +1213,11 @@
}
}
+ @Override
+ protected void onRequestSetAllowed(boolean allowed) {
+ // do nothing - the gnss provider is always allowed
+ }
+
private void deleteAidingData(Bundle extras) {
int flags;
diff --git a/services/core/java/com/android/server/location/LocationProviderProxy.java b/services/core/java/com/android/server/location/LocationProviderProxy.java
index 1dee7a8..19fb669 100644
--- a/services/core/java/com/android/server/location/LocationProviderProxy.java
+++ b/services/core/java/com/android/server/location/LocationProviderProxy.java
@@ -134,6 +134,7 @@
// also used to synchronized any state changes (setEnabled, setProperties, setState, etc)
private final Object mLock = new Object();
+ private final Context mContext;
private final ServiceWatcher mServiceWatcher;
@GuardedBy("mLock")
@@ -143,10 +144,11 @@
private LocationProviderProxy(Context context, String action, int enableOverlayResId,
int nonOverlayPackageResId) {
- // safe to use direct executor since none of our callbacks call back into any code above
- // this provider - they simply forward to the proxy service
- super(context, DIRECT_EXECUTOR);
+ // safe to use direct executor even though this class has internal locks - all of our
+ // callbacks go to oneway binder transactions which cannot possibly be re-entrant
+ super(DIRECT_EXECUTOR, Collections.emptySet());
+ mContext = context;
mServiceWatcher = new ServiceWatcher(context, FgThread.getHandler(), action, this::onBind,
this::onUnbind, enableOverlayResId, nonOverlayPackageResId);
diff --git a/services/core/java/com/android/server/location/MockProvider.java b/services/core/java/com/android/server/location/MockProvider.java
index bcec8b1..b45b660 100644
--- a/services/core/java/com/android/server/location/MockProvider.java
+++ b/services/core/java/com/android/server/location/MockProvider.java
@@ -16,15 +16,18 @@
package com.android.server.location;
+import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
+
import android.annotation.Nullable;
-import android.content.Context;
import android.location.Location;
+import android.os.Bundle;
import com.android.internal.location.ProviderProperties;
import com.android.internal.location.ProviderRequest;
import java.io.FileDescriptor;
import java.io.PrintWriter;
+import java.util.Collections;
/**
* A mock location provider used by LocationManagerService to implement test providers.
@@ -35,10 +38,9 @@
@Nullable private Location mLocation;
- public MockProvider(Context context, ProviderProperties properties) {
- // using a direct executor is only acceptable because this class is so simple it is trivial
- // to verify that it does not acquire any locks or re-enter LMS from callbacks
- super(context, Runnable::run);
+ public MockProvider(ProviderProperties properties) {
+ // using a direct executor is ok because this class has no locks that could deadlock
+ super(DIRECT_EXECUTOR, Collections.emptySet());
setProperties(properties);
}
@@ -59,6 +61,9 @@
public void onSetRequest(ProviderRequest request) {}
@Override
+ protected void onExtraCommand(int uid, int pid, String command, Bundle extras) {}
+
+ @Override
protected void onRequestSetAllowed(boolean allowed) {
setAllowed(allowed);
}
diff --git a/services/core/java/com/android/server/location/MockableLocationProvider.java b/services/core/java/com/android/server/location/MockableLocationProvider.java
index 5b4f008..b0e1330 100644
--- a/services/core/java/com/android/server/location/MockableLocationProvider.java
+++ b/services/core/java/com/android/server/location/MockableLocationProvider.java
@@ -16,8 +16,9 @@
package com.android.server.location;
+import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
+
import android.annotation.Nullable;
-import android.content.Context;
import android.location.Location;
import android.os.Bundle;
@@ -68,10 +69,10 @@
* The client should expect that it may being to receive callbacks as soon as this constructor
* is invoked.
*/
- public MockableLocationProvider(Context context, Object ownerLock, Listener listener) {
+ public MockableLocationProvider(Object ownerLock, Listener listener) {
// using a direct executor is acceptable because all inbound calls are delegated to the
// actual provider implementations which will use their own executors
- super(context, Runnable::run, Collections.emptySet());
+ super(DIRECT_EXECUTOR, Collections.emptySet());
mOwnerLock = ownerLock;
mRequest = ProviderRequest.EMPTY_REQUEST;
@@ -190,11 +191,6 @@
}
}
- @Override
- public State getState() {
- return super.getState();
- }
-
/**
* Returns the current location request.
*/
@@ -204,6 +200,7 @@
}
}
+ @Override
protected void onSetRequest(ProviderRequest request) {
synchronized (mOwnerLock) {
if (request == mRequest) {
@@ -218,6 +215,7 @@
}
}
+ @Override
protected void onExtraCommand(int uid, int pid, String command, Bundle extras) {
synchronized (mOwnerLock) {
if (mProvider != null) {
@@ -226,6 +224,15 @@
}
}
+ @Override
+ protected void onRequestSetAllowed(boolean allowed) {
+ synchronized (mOwnerLock) {
+ if (mProvider != null) {
+ mProvider.onRequestSetAllowed(allowed);
+ }
+ }
+ }
+
/**
* Dumps the current provider implementation.
*/
diff --git a/services/core/java/com/android/server/location/PassiveProvider.java b/services/core/java/com/android/server/location/PassiveProvider.java
index ef157a3..54dffff 100644
--- a/services/core/java/com/android/server/location/PassiveProvider.java
+++ b/services/core/java/com/android/server/location/PassiveProvider.java
@@ -16,9 +16,12 @@
package com.android.server.location;
+import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;
+
import android.content.Context;
import android.location.Criteria;
import android.location.Location;
+import android.os.Bundle;
import com.android.internal.location.ProviderProperties;
import com.android.internal.location.ProviderRequest;
@@ -49,9 +52,8 @@
private volatile boolean mReportLocation;
public PassiveProvider(Context context) {
- // using a direct executor is only acceptable because this class is so simple it is trivial
- // to verify that it does not acquire any locks or re-enter LMS from callbacks
- super(context, Runnable::run);
+ // using a direct executor is ok because this class has no locks that could deadlock
+ super(DIRECT_EXECUTOR, context);
mReportLocation = false;
@@ -59,15 +61,26 @@
setAllowed(true);
}
+ /**
+ * Pass a location into the passive provider.
+ */
+ public void updateLocation(Location location) {
+ if (mReportLocation) {
+ reportLocation(location);
+ }
+ }
+
@Override
public void onSetRequest(ProviderRequest request) {
mReportLocation = request.reportLocation;
}
- public void updateLocation(Location location) {
- if (mReportLocation) {
- reportLocation(location);
- }
+ @Override
+ protected void onExtraCommand(int uid, int pid, String command, Bundle extras) {}
+
+ @Override
+ protected void onRequestSetAllowed(boolean allowed) {
+ // do nothing - the passive provider is always allowed
}
@Override
diff --git a/services/tests/servicestests/Android.bp b/services/tests/servicestests/Android.bp
index 556f636..f990810 100644
--- a/services/tests/servicestests/Android.bp
+++ b/services/tests/servicestests/Android.bp
@@ -30,6 +30,7 @@
"services.usage",
"guava",
"androidx.test.core",
+ "androidx.test.ext.truth",
"androidx.test.runner",
"androidx.test.rules",
"mockito-target-minus-junit4",
diff --git a/services/tests/servicestests/src/com/android/server/location/MockableLocationProviderTest.java b/services/tests/servicestests/src/com/android/server/location/MockableLocationProviderTest.java
new file mode 100644
index 0000000..6fafe11
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/location/MockableLocationProviderTest.java
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) 2020 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.location;
+
+import static androidx.test.ext.truth.location.LocationSubject.assertThat;
+
+import static com.android.internal.location.ProviderRequest.EMPTY_REQUEST;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import android.location.Criteria;
+import android.location.Location;
+import android.platform.test.annotations.Presubmit;
+
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+
+import com.android.internal.location.ProviderProperties;
+import com.android.internal.location.ProviderRequest;
+import com.android.server.location.test.FakeProvider;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.LinkedList;
+import java.util.List;
+
+@Presubmit
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class MockableLocationProviderTest {
+
+ private Object mLock;
+ private ListenerCapture mListener;
+
+ private AbstractLocationProvider mRealProvider;
+ private MockProvider mMockProvider;
+
+ private MockableLocationProvider mProvider;
+
+ @Before
+ public void setUp() {
+ mLock = new Object();
+ mListener = new ListenerCapture();
+
+ mRealProvider = spy(new FakeProvider());
+ mMockProvider = spy(new MockProvider(new ProviderProperties(
+ false,
+ false,
+ false,
+ false,
+ true,
+ true,
+ true,
+ Criteria.POWER_LOW,
+ Criteria.ACCURACY_FINE)));
+
+ mProvider = new MockableLocationProvider(mLock, mListener);
+ mProvider.setRealProvider(mRealProvider);
+ }
+
+ @Test
+ public void testSetProvider() {
+ assertThat(mProvider.getProvider()).isEqualTo(mRealProvider);
+
+ mProvider.setMockProvider(mMockProvider);
+ assertThat(mProvider.getProvider()).isEqualTo(mMockProvider);
+
+ mProvider.setMockProvider(null);
+ assertThat(mProvider.getProvider()).isEqualTo(mRealProvider);
+
+ mProvider.setRealProvider(null);
+ assertThat(mProvider.getProvider()).isNull();
+ }
+
+ @Test
+ public void testSetRequest() {
+ assertThat(mProvider.getCurrentRequest()).isEqualTo(EMPTY_REQUEST);
+ verify(mRealProvider, times(1)).onSetRequest(EMPTY_REQUEST);
+
+ ProviderRequest request = new ProviderRequest.Builder().setInterval(1).build();
+ mProvider.setRequest(request);
+
+ assertThat(mProvider.getCurrentRequest()).isEqualTo(request);
+ verify(mRealProvider, times(1)).onSetRequest(request);
+
+ mProvider.setMockProvider(mMockProvider);
+ assertThat(mProvider.getCurrentRequest()).isEqualTo(request);
+ verify(mRealProvider, times(2)).onSetRequest(EMPTY_REQUEST);
+ verify(mMockProvider, times(1)).onSetRequest(request);
+
+ mProvider.setMockProvider(null);
+ assertThat(mProvider.getCurrentRequest()).isEqualTo(request);
+ verify(mMockProvider, times(1)).onSetRequest(EMPTY_REQUEST);
+ verify(mRealProvider, times(2)).onSetRequest(request);
+
+ mProvider.setRealProvider(null);
+ assertThat(mProvider.getCurrentRequest()).isEqualTo(request);
+ verify(mRealProvider, times(3)).onSetRequest(EMPTY_REQUEST);
+ }
+
+ @Test
+ public void testRequestSetAllowed() {
+ mProvider.requestSetAllowed(true);
+ verify(mRealProvider, times(1)).onRequestSetAllowed(true);
+
+ mProvider.setMockProvider(mMockProvider);
+ mProvider.requestSetAllowed(true);
+ verify(mMockProvider, times(1)).onRequestSetAllowed(true);
+ }
+
+ @Test
+ public void testSendExtraCommand() {
+ mProvider.sendExtraCommand(0, 0, "command", null);
+ verify(mRealProvider, times(1)).onExtraCommand(0, 0, "command", null);
+
+ mProvider.setMockProvider(mMockProvider);
+ mProvider.sendExtraCommand(0, 0, "command", null);
+ verify(mMockProvider, times(1)).onExtraCommand(0, 0, "command", null);
+ }
+
+ @Test
+ public void testSetState() {
+ assertThat(mProvider.getState().allowed).isFalse();
+
+ AbstractLocationProvider.State newState;
+
+ mRealProvider.setAllowed(true);
+ newState = mListener.getNextNewState();
+ assertThat(newState).isNotNull();
+ assertThat(newState.allowed).isTrue();
+
+ mProvider.setMockProvider(mMockProvider);
+ newState = mListener.getNextNewState();
+ assertThat(newState).isNotNull();
+ assertThat(newState.allowed).isFalse();
+
+ mMockProvider.setAllowed(true);
+ newState = mListener.getNextNewState();
+ assertThat(newState).isNotNull();
+ assertThat(newState.allowed).isTrue();
+
+ mRealProvider.setAllowed(false);
+ assertThat(mListener.getNextNewState()).isNull();
+
+ mProvider.setMockProvider(null);
+ newState = mListener.getNextNewState();
+ assertThat(newState).isNotNull();
+ assertThat(newState.allowed).isFalse();
+ }
+
+ @Test
+ public void testReportLocation() {
+ Location realLocation = new Location("real");
+ Location mockLocation = new Location("mock");
+
+ mRealProvider.reportLocation(realLocation);
+ assertThat(mListener.getNextLocation()).isEqualTo(realLocation);
+
+ mProvider.setMockProvider(mMockProvider);
+ mRealProvider.reportLocation(realLocation);
+ mMockProvider.reportLocation(mockLocation);
+ assertThat(mListener.getNextLocation()).isEqualTo(mockLocation);
+ }
+
+ private class ListenerCapture implements AbstractLocationProvider.Listener {
+
+ private final LinkedList<AbstractLocationProvider.State> mNewStates = new LinkedList<>();
+ private final LinkedList<Location> mLocations = new LinkedList<>();
+
+ @Override
+ public void onStateChanged(AbstractLocationProvider.State oldState,
+ AbstractLocationProvider.State newState) {
+ assertThat(Thread.holdsLock(mLock)).isTrue();
+ mNewStates.add(newState);
+ }
+
+ private AbstractLocationProvider.State getNextNewState() {
+ return mNewStates.poll();
+ }
+
+ @Override
+ public void onReportLocation(Location location) {
+ assertThat(Thread.holdsLock(mLock)).isTrue();
+ mLocations.add(location);
+ }
+
+ private Location getNextLocation() {
+ return mLocations.poll();
+ }
+
+ @Override
+ public void onReportLocation(List<Location> locations) {}
+ }
+}
diff --git a/services/tests/servicestests/src/com/android/server/location/test/FakeProvider.java b/services/tests/servicestests/src/com/android/server/location/test/FakeProvider.java
new file mode 100644
index 0000000..762080f
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/location/test/FakeProvider.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2020 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.location.test;
+
+import android.os.Bundle;
+
+import com.android.internal.location.ProviderRequest;
+import com.android.server.location.AbstractLocationProvider;
+
+import java.io.FileDescriptor;
+import java.io.PrintWriter;
+import java.util.Collections;
+
+public class FakeProvider extends AbstractLocationProvider {
+
+ public FakeProvider() {
+ super(Runnable::run, Collections.emptySet());
+ }
+
+ @Override
+ protected void onSetRequest(ProviderRequest request) {}
+
+ @Override
+ protected void onExtraCommand(int uid, int pid, String command, Bundle extras) {}
+
+ @Override
+ protected void onRequestSetAllowed(boolean allowed) {}
+
+ @Override
+ public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {}
+}