Tell the factory it is already serving a request.

This is a cherry-pick of ag/607226 that has been rebased on
top of four years of changes and with comments addressed.

Gives each factory a serial number and propagates it to every
NetworkAgent so when a score comes back indicating a request is
being handled the factory can account for it properly.

Without this, a new request that's already handled by a network
offered by a factory will not cause an increment of the factorys
ref count. Concretely this results in issues like the RAT icon
not being displayed in spite of the network actually being up
and usable.

This will be ported to AOSP as soon as possible, but immediately
some master-only WiFi tests need to be adjusted with this change
which would not let me submit to AOSP.

Bug: 18637384
Bug: 29030667
Test: manual
Test: atest frameworks/opt/telephony/tests/telephonytests
Test: atest frameworks-net
Test: atest CtsNetTestCases CtsHostsideNetworkTests
Change-Id: I597ac588f76dd507512ff02868fd1310b7e63f7e
Merged-In: I597ac588f76dd507512ff02868fd1310b7e63f7e
This commit is contained in:
Chalard Jean
2018-05-02 21:14:54 +09:00
parent dce527fbd6
commit 29d06db64b
7 changed files with 189 additions and 77 deletions

View File

@@ -508,7 +508,7 @@ public class ConnectivityServiceTest {
mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext,
"Mock-" + typeName, mNetworkInfo, mNetworkCapabilities,
linkProperties, mScore, new NetworkMisc()) {
linkProperties, mScore, new NetworkMisc(), NetworkFactory.SerialNumber.NONE) {
@Override
public void unwanted() { mDisconnected.open(); }
@@ -743,7 +743,7 @@ public class ConnectivityServiceTest {
/**
* A NetworkFactory that allows tests to wait until any in-flight NetworkRequest add or remove
* operations have been processed. Before ConnectivityService can add or remove any requests,
* the factory must be told to expect those operations by calling expectAddRequests or
* the factory must be told to expect those operations by calling expectAddRequestsWithScores or
* expectRemoveRequests.
*/
private static class MockNetworkFactory extends NetworkFactory {
@@ -752,14 +752,16 @@ public class ConnectivityServiceTest {
private final AtomicBoolean mNetworkStarted = new AtomicBoolean(false);
// Used to expect that requests be removed or added on a separate thread, without sleeping.
// Callers can call either expectAddRequests() or expectRemoveRequests() exactly once, then
// cause some other thread to add or remove requests, then call waitForRequests(). We can
// either expect requests to be added or removed, but not both, because CountDownLatch can
// only count in one direction.
private CountDownLatch mExpectations;
// Callers can call either expectAddRequestsWithScores() or expectRemoveRequests() exactly
// once, then cause some other thread to add or remove requests, then call
// waitForRequests().
// It is not possible to wait for both add and remove requests. When adding, the queue
// contains the expected score. When removing, the value is unused, all matters is the
// number of objects in the queue.
private final LinkedBlockingQueue<Integer> mExpectations;
// Whether we are currently expecting requests to be added or removed. Valid only if
// mExpectations is non-null.
// mExpectations is non-empty.
private boolean mExpectingAdditions;
// Used to collect the networks requests managed by this factory. This is a duplicate of
@@ -769,6 +771,7 @@ public class ConnectivityServiceTest {
public MockNetworkFactory(Looper looper, Context context, String logTag,
NetworkCapabilities filter) {
super(looper, context, logTag, filter);
mExpectations = new LinkedBlockingQueue<>();
}
public int getMyRequestCount() {
@@ -800,38 +803,44 @@ public class ConnectivityServiceTest {
}
@Override
protected void handleAddRequest(NetworkRequest request, int score) {
// If we're expecting anything, we must be expecting additions.
if (mExpectations != null && !mExpectingAdditions) {
fail("Can't add requests while expecting requests to be removed");
}
protected void handleAddRequest(NetworkRequest request, int score,
int factorySerialNumber) {
synchronized (mExpectations) {
final Integer expectedScore = mExpectations.poll(); // null if the queue is empty
// Add the request.
mNetworkRequests.put(request.requestId, request);
super.handleAddRequest(request, score);
assertNotNull("Added more requests than expected (" + request + " score : "
+ score + ")", expectedScore);
// If we're expecting anything, we must be expecting additions.
if (!mExpectingAdditions) {
fail("Can't add requests while expecting requests to be removed");
}
if (expectedScore != score) {
fail("Expected score was " + expectedScore + " but actual was " + score
+ " in added request");
}
// Reduce the number of request additions we're waiting for.
if (mExpectingAdditions) {
assertTrue("Added more requests than expected", mExpectations.getCount() > 0);
mExpectations.countDown();
// Add the request.
mNetworkRequests.put(request.requestId, request);
super.handleAddRequest(request, score, factorySerialNumber);
mExpectations.notify();
}
}
@Override
protected void handleRemoveRequest(NetworkRequest request) {
// If we're expecting anything, we must be expecting removals.
if (mExpectations != null && mExpectingAdditions) {
fail("Can't remove requests while expecting requests to be added");
}
synchronized (mExpectations) {
final Integer expectedScore = mExpectations.poll(); // null if the queue is empty
// Remove the request.
mNetworkRequests.remove(request.requestId);
super.handleRemoveRequest(request);
assertTrue("Removed more requests than expected", expectedScore != null);
// If we're expecting anything, we must be expecting removals.
if (mExpectingAdditions) {
fail("Can't remove requests while expecting requests to be added");
}
// Reduce the number of request removals we're waiting for.
if (!mExpectingAdditions) {
assertTrue("Removed more requests than expected", mExpectations.getCount() > 0);
mExpectations.countDown();
// Remove the request.
mNetworkRequests.remove(request.requestId);
super.handleRemoveRequest(request);
mExpectations.notify();
}
}
@@ -841,35 +850,42 @@ public class ConnectivityServiceTest {
}
private void assertNoExpectations() {
if (mExpectations != null) {
fail("Can't add expectation, " + mExpectations.getCount() + " already pending");
if (mExpectations.size() != 0) {
fail("Can't add expectation, " + mExpectations.size() + " already pending");
}
}
// Expects that count requests will be added.
public void expectAddRequests(final int count) {
// Expects that requests with the specified scores will be added.
public void expectAddRequestsWithScores(final int... scores) {
assertNoExpectations();
mExpectingAdditions = true;
mExpectations = new CountDownLatch(count);
for (int score : scores) {
mExpectations.add(score);
}
}
// Expects that count requests will be removed.
public void expectRemoveRequests(final int count) {
assertNoExpectations();
mExpectingAdditions = false;
mExpectations = new CountDownLatch(count);
for (int i = 0; i < count; ++i) {
mExpectations.add(0); // For removals the score is ignored so any value will do.
}
}
// Waits for the expected request additions or removals to happen within a timeout.
public void waitForRequests() throws InterruptedException {
assertNotNull("Nothing to wait for", mExpectations);
mExpectations.await(TIMEOUT_MS, TimeUnit.MILLISECONDS);
final long count = mExpectations.getCount();
final long deadline = SystemClock.elapsedRealtime() + TIMEOUT_MS;
synchronized (mExpectations) {
while (mExpectations.size() > 0 && SystemClock.elapsedRealtime() < deadline) {
mExpectations.wait(deadline - SystemClock.elapsedRealtime());
}
}
final long count = mExpectations.size();
final String msg = count + " requests still not " +
(mExpectingAdditions ? "added" : "removed") +
" after " + TIMEOUT_MS + " ms";
assertEquals(msg, 0, count);
mExpectations = null;
}
public SparseArray<NetworkRequest> waitForNetworkRequests(final int count)
@@ -2326,6 +2342,12 @@ public class ConnectivityServiceTest {
callback.expectCallback(CallbackState.LOST, mEthernetNetworkAgent);
}
private int[] makeIntArray(final int size, final int value) {
final int[] array = new int[size];
Arrays.fill(array, value);
return array;
}
private void tryNetworkFactoryRequests(int capability) throws Exception {
// Verify NOT_RESTRICTED is set appropriately
final NetworkCapabilities nc = new NetworkRequest.Builder().addCapability(capability)
@@ -2347,7 +2369,7 @@ public class ConnectivityServiceTest {
mServiceContext, "testFactory", filter);
testFactory.setScoreFilter(40);
ConditionVariable cv = testFactory.getNetworkStartedCV();
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0);
testFactory.register();
testFactory.waitForNetworkRequests(1);
int expectedRequestCount = 1;
@@ -2358,7 +2380,7 @@ public class ConnectivityServiceTest {
assertFalse(testFactory.getMyStartRequested());
NetworkRequest request = new NetworkRequest.Builder().addCapability(capability).build();
networkCallback = new NetworkCallback();
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0); // New request
mCm.requestNetwork(request, networkCallback);
expectedRequestCount++;
testFactory.waitForNetworkRequests(expectedRequestCount);
@@ -2378,7 +2400,7 @@ public class ConnectivityServiceTest {
// When testAgent connects, ConnectivityService will re-send us all current requests with
// the new score. There are expectedRequestCount such requests, and we must wait for all of
// them.
testFactory.expectAddRequests(expectedRequestCount);
testFactory.expectAddRequestsWithScores(makeIntArray(expectedRequestCount, 50));
testAgent.connect(false);
testAgent.addCapability(capability);
waitFor(cv);
@@ -2386,7 +2408,7 @@ public class ConnectivityServiceTest {
assertFalse(testFactory.getMyStartRequested());
// Bring in a bunch of requests.
testFactory.expectAddRequests(10);
testFactory.expectAddRequestsWithScores(makeIntArray(10, 50));
assertEquals(expectedRequestCount, testFactory.getMyRequestCount());
ConnectivityManager.NetworkCallback[] networkCallbacks =
new ConnectivityManager.NetworkCallback[10];
@@ -2409,8 +2431,11 @@ public class ConnectivityServiceTest {
// Drop the higher scored network.
cv = testFactory.getNetworkStartedCV();
// With the default network disconnecting, the requests are sent with score 0 to factories.
testFactory.expectAddRequestsWithScores(makeIntArray(expectedRequestCount, 0));
testAgent.disconnect();
waitFor(cv);
testFactory.waitForNetworkRequests(expectedRequestCount);
assertEquals(expectedRequestCount, testFactory.getMyRequestCount());
assertTrue(testFactory.getMyStartRequested());
@@ -3332,22 +3357,23 @@ public class ConnectivityServiceTest {
testFactory.setScoreFilter(40);
// Register the factory and expect it to start looking for a network.
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0); // Score 0 as the request is not served yet.
testFactory.register();
testFactory.waitForNetworkRequests(1);
assertTrue(testFactory.getMyStartRequested());
// Bring up wifi. The factory stops looking for a network.
mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
testFactory.expectAddRequests(2); // Because the default request changes score twice.
// Score 60 - 40 penalty for not validated yet, then 60 when it validates
testFactory.expectAddRequestsWithScores(20, 60);
mWiFiNetworkAgent.connect(true);
testFactory.waitForNetworkRequests(1);
testFactory.waitForRequests();
assertFalse(testFactory.getMyStartRequested());
ContentResolver cr = mServiceContext.getContentResolver();
// Turn on mobile data always on. The factory starts looking again.
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0); // Always on requests comes up with score 0
setAlwaysOnNetworks(true);
testFactory.waitForNetworkRequests(2);
assertTrue(testFactory.getMyStartRequested());
@@ -3355,7 +3381,7 @@ public class ConnectivityServiceTest {
// Bring up cell data and check that the factory stops looking.
assertLength(1, mCm.getAllNetworks());
mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);
testFactory.expectAddRequests(2); // Because the cell request changes score twice.
testFactory.expectAddRequestsWithScores(10, 50); // Unvalidated, then validated
mCellNetworkAgent.connect(true);
cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent);
testFactory.waitForNetworkRequests(2);
@@ -3669,7 +3695,7 @@ public class ConnectivityServiceTest {
testFactory.setScoreFilter(40);
// Register the factory and expect it to receive the default request.
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0); // default request score is 0, not served yet
testFactory.register();
SparseArray<NetworkRequest> requests = testFactory.waitForNetworkRequests(1);
@@ -3677,7 +3703,7 @@ public class ConnectivityServiceTest {
int origRequestId = requests.valueAt(0).requestId;
// Now file the test request and expect it.
testFactory.expectAddRequests(1);
testFactory.expectAddRequestsWithScores(0);
mCm.requestNetwork(nr, networkCallback);
requests = testFactory.waitForNetworkRequests(2); // have 2 requests at this point

View File

@@ -35,6 +35,7 @@ import android.net.ConnectivityManager;
import android.net.INetd;
import android.net.Network;
import android.net.NetworkCapabilities;
import android.net.NetworkFactory;
import android.net.NetworkInfo;
import android.net.NetworkMisc;
import android.os.INetworkManagementService;
@@ -352,7 +353,8 @@ public class LingerMonitorTest {
caps.addCapability(0);
caps.addTransportType(transport);
NetworkAgentInfo nai = new NetworkAgentInfo(null, null, new Network(netId), info, null,
caps, 50, mCtx, null, mMisc, mConnService, mNetd, mNMS);
caps, 50, mCtx, null, mMisc, mConnService, mNetd, mNMS,
NetworkFactory.SerialNumber.NONE);
nai.everValidated = true;
return nai;
}