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 propogates 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
This commit is contained in:
Chalard Jean
2018-05-02 21:14:54 +09:00
parent 41c6a0ad9c
commit 08577fc579
9 changed files with 300 additions and 103 deletions

View File

@@ -2918,11 +2918,11 @@ public class ConnectivityManager {
}
}
/** {@hide} */
/** {@hide} - returns the factory serial number */
@UnsupportedAppUsage
public void registerNetworkFactory(Messenger messenger, String name) {
public int registerNetworkFactory(Messenger messenger, String name) {
try {
mService.registerNetworkFactory(messenger, name);
return mService.registerNetworkFactory(messenger, name);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
@@ -2938,6 +2938,10 @@ public class ConnectivityManager {
}
}
// TODO : remove this method. It is a stopgap measure to help sheperding a number
// of dependent changes that would conflict throughout the automerger graph. Having this
// temporarily helps with the process of going through with all these dependent changes across
// the entire tree.
/**
* @hide
* Register a NetworkAgent with ConnectivityService.
@@ -2945,8 +2949,20 @@ public class ConnectivityManager {
*/
public int registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
NetworkCapabilities nc, int score, NetworkMisc misc) {
return registerNetworkAgent(messenger, ni, lp, nc, score, misc,
NetworkFactory.SerialNumber.NONE);
}
/**
* @hide
* Register a NetworkAgent with ConnectivityService.
* @return NetID corresponding to NetworkAgent.
*/
public int registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
NetworkCapabilities nc, int score, NetworkMisc misc, int factorySerialNumber) {
try {
return mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc);
return mService.registerNetworkAgent(messenger, ni, lp, nc, score, misc,
factorySerialNumber);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}

View File

@@ -139,14 +139,14 @@ interface IConnectivityManager
void setAirplaneMode(boolean enable);
void registerNetworkFactory(in Messenger messenger, in String name);
int registerNetworkFactory(in Messenger messenger, in String name);
boolean requestBandwidthUpdate(in Network network);
void unregisterNetworkFactory(in Messenger messenger);
int registerNetworkAgent(in Messenger messenger, in NetworkInfo ni, in LinkProperties lp,
in NetworkCapabilities nc, int score, in NetworkMisc misc);
in NetworkCapabilities nc, int score, in NetworkMisc misc, in int factorySerialNumber);
NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities,
in Messenger messenger, int timeoutSec, in IBinder binder, int legacy);

View File

@@ -58,6 +58,7 @@ public abstract class NetworkAgent extends Handler {
private static final long BW_REFRESH_MIN_WIN_MS = 500;
private boolean mPollLceScheduled = false;
private AtomicBoolean mPollLcePending = new AtomicBoolean(false);
public final int mFactorySerialNumber;
private static final int BASE = Protocol.BASE_NETWORK_AGENT;
@@ -193,16 +194,31 @@ public abstract class NetworkAgent extends Handler {
*/
public static final int CMD_PREVENT_AUTOMATIC_RECONNECT = BASE + 15;
// TODO : remove these two constructors. They are a stopgap measure to help sheperding a number
// of dependent changes that would conflict throughout the automerger graph. Having these
// temporarily helps with the process of going through with all these dependent changes across
// the entire tree.
public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
NetworkCapabilities nc, LinkProperties lp, int score) {
this(looper, context, logTag, ni, nc, lp, score, null);
this(looper, context, logTag, ni, nc, lp, score, null, NetworkFactory.SerialNumber.NONE);
}
public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
NetworkCapabilities nc, LinkProperties lp, int score, NetworkMisc misc) {
this(looper, context, logTag, ni, nc, lp, score, misc, NetworkFactory.SerialNumber.NONE);
}
public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
NetworkCapabilities nc, LinkProperties lp, int score, NetworkMisc misc) {
NetworkCapabilities nc, LinkProperties lp, int score, int factorySerialNumber) {
this(looper, context, logTag, ni, nc, lp, score, null, factorySerialNumber);
}
public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
NetworkCapabilities nc, LinkProperties lp, int score, NetworkMisc misc,
int factorySerialNumber) {
super(looper);
LOG_TAG = logTag;
mContext = context;
mFactorySerialNumber = factorySerialNumber;
if (ni == null || nc == null || lp == null) {
throw new IllegalArgumentException();
}
@@ -211,7 +227,8 @@ public abstract class NetworkAgent extends Handler {
ConnectivityManager cm = (ConnectivityManager)mContext.getSystemService(
Context.CONNECTIVITY_SERVICE);
netId = cm.registerNetworkAgent(new Messenger(this), new NetworkInfo(ni),
new LinkProperties(lp), new NetworkCapabilities(nc), score, misc);
new LinkProperties(lp), new NetworkCapabilities(nc), score, misc,
factorySerialNumber);
}
@Override

View File

@@ -32,6 +32,7 @@ import com.android.internal.util.Protocol;
import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.concurrent.atomic.AtomicInteger;
/**
* A NetworkFactory is an entity that creates NetworkAgent objects.
@@ -49,6 +50,20 @@ import java.io.PrintWriter;
* @hide
**/
public class NetworkFactory extends Handler {
/** @hide */
public static class SerialNumber {
// Guard used by no network factory.
public static final int NONE = -1;
// A hardcoded serial number for NetworkAgents representing VPNs. These agents are
// not created by any factory, so they use this constant for clarity instead of NONE.
public static final int VPN = -2;
private static final AtomicInteger sNetworkFactorySerialNumber = new AtomicInteger(1);
/** Returns a unique serial number for a factory. */
public static final int nextSerialNumber() {
return sNetworkFactorySerialNumber.getAndIncrement();
}
}
private static final boolean DBG = true;
private static final boolean VDBG = false;
@@ -64,7 +79,7 @@ public class NetworkFactory extends Handler {
* disregard any that it will never be able to service, for example
* those requiring a different bearer.
* msg.obj = NetworkRequest
* msg.arg1 = score - the score of the any network currently satisfying this
* msg.arg1 = score - the score of the network currently satisfying this
* request. If this bearer knows in advance it cannot
* exceed this score it should not try to connect, holding the request
* for the future.
@@ -74,6 +89,8 @@ public class NetworkFactory extends Handler {
* with the same NetworkRequest but an updated score.
* Also, network conditions may change for this bearer
* allowing for a better score in the future.
* msg.arg2 = the serial number of the factory currently responsible for the
* NetworkAgent handling this request, or SerialNumber.NONE if none.
*/
public static final int CMD_REQUEST_NETWORK = BASE;
@@ -107,6 +124,7 @@ public class NetworkFactory extends Handler {
private int mRefCount = 0;
private Messenger mMessenger = null;
private int mSerialNumber;
@UnsupportedAppUsage
public NetworkFactory(Looper looper, Context context, String logTag,
@@ -121,7 +139,8 @@ public class NetworkFactory extends Handler {
if (DBG) log("Registering NetworkFactory");
if (mMessenger == null) {
mMessenger = new Messenger(this);
ConnectivityManager.from(mContext).registerNetworkFactory(mMessenger, LOG_TAG);
mSerialNumber = ConnectivityManager.from(mContext).registerNetworkFactory(mMessenger,
LOG_TAG);
}
}
@@ -137,7 +156,7 @@ public class NetworkFactory extends Handler {
public void handleMessage(Message msg) {
switch (msg.what) {
case CMD_REQUEST_NETWORK: {
handleAddRequest((NetworkRequest)msg.obj, msg.arg1);
handleAddRequest((NetworkRequest) msg.obj, msg.arg1, msg.arg2);
break;
}
case CMD_CANCEL_REQUEST: {
@@ -159,11 +178,13 @@ public class NetworkFactory extends Handler {
public final NetworkRequest request;
public int score;
public boolean requested; // do we have a request outstanding, limited by score
public int factorySerialNumber;
public NetworkRequestInfo(NetworkRequest request, int score) {
NetworkRequestInfo(NetworkRequest request, int score, int factorySerialNumber) {
this.request = request;
this.score = score;
this.requested = false;
this.factorySerialNumber = factorySerialNumber;
}
@Override
@@ -172,16 +193,51 @@ public class NetworkFactory extends Handler {
}
}
/**
* Add a NetworkRequest that the bearer may want to attempt to satisfy.
* @see #CMD_REQUEST_NETWORK
*
* @param request the request to handle.
* @param score the score of the NetworkAgent currently satisfying this request.
* @param servingFactorySerialNumber the serial number of the NetworkFactory that
* created the NetworkAgent currently satisfying this request.
*/
// TODO : remove this method. It is a stopgap measure to help sheperding a number
// of dependent changes that would conflict throughout the automerger graph. Having this
// temporarily helps with the process of going through with all these dependent changes across
// the entire tree.
@VisibleForTesting
protected void handleAddRequest(NetworkRequest request, int score) {
handleAddRequest(request, score, SerialNumber.NONE);
}
/**
* Add a NetworkRequest that the bearer may want to attempt to satisfy.
* @see #CMD_REQUEST_NETWORK
*
* @param request the request to handle.
* @param score the score of the NetworkAgent currently satisfying this request.
* @param servingFactorySerialNumber the serial number of the NetworkFactory that
* created the NetworkAgent currently satisfying this request.
*/
@VisibleForTesting
protected void handleAddRequest(NetworkRequest request, int score,
int servingFactorySerialNumber) {
NetworkRequestInfo n = mNetworkRequests.get(request.requestId);
if (n == null) {
if (DBG) log("got request " + request + " with score " + score);
n = new NetworkRequestInfo(request, score);
if (DBG) {
log("got request " + request + " with score " + score
+ " and serial " + servingFactorySerialNumber);
}
n = new NetworkRequestInfo(request, score, servingFactorySerialNumber);
mNetworkRequests.put(n.request.requestId, n);
} else {
if (VDBG) log("new score " + score + " for exisiting request " + request);
if (VDBG) {
log("new score " + score + " for exisiting request " + request
+ " with serial " + servingFactorySerialNumber);
}
n.score = score;
n.factorySerialNumber = servingFactorySerialNumber;
}
if (VDBG) log(" my score=" + mScore + ", my filter=" + mCapabilityFilter);
@@ -231,16 +287,19 @@ public class NetworkFactory extends Handler {
}
private void evalRequest(NetworkRequestInfo n) {
if (VDBG) log("evalRequest");
if (n.requested == false && n.score < mScore &&
n.request.networkCapabilities.satisfiedByNetworkCapabilities(
mCapabilityFilter) && acceptRequest(n.request, n.score)) {
if (VDBG) {
log("evalRequest");
log(" n.requests = " + n.requested);
log(" n.score = " + n.score);
log(" mScore = " + mScore);
log(" n.factorySerialNumber = " + n.factorySerialNumber);
log(" mSerialNumber = " + mSerialNumber);
}
if (shouldNeedNetworkFor(n)) {
if (VDBG) log(" needNetworkFor");
needNetworkFor(n.request, n.score);
n.requested = true;
} else if (n.requested == true &&
(n.score > mScore || n.request.networkCapabilities.satisfiedByNetworkCapabilities(
mCapabilityFilter) == false || acceptRequest(n.request, n.score) == false)) {
} else if (shouldReleaseNetworkFor(n)) {
if (VDBG) log(" releaseNetworkFor");
releaseNetworkFor(n.request);
n.requested = false;
@@ -249,10 +308,39 @@ public class NetworkFactory extends Handler {
}
}
private boolean shouldNeedNetworkFor(NetworkRequestInfo n) {
// If this request is already tracked, it doesn't qualify for need
return !n.requested
// If the score of this request is higher or equal to that of this factory and some
// other factory is responsible for it, then this factory should not track the request
// because it has no hope of satisfying it.
&& (n.score < mScore || n.factorySerialNumber == mSerialNumber)
// If this factory can't satisfy the capability needs of this request, then it
// should not be tracked.
&& n.request.networkCapabilities.satisfiedByNetworkCapabilities(mCapabilityFilter)
// Finally if the concrete implementation of the factory rejects the request, then
// don't track it.
&& acceptRequest(n.request, n.score);
}
private boolean shouldReleaseNetworkFor(NetworkRequestInfo n) {
// Don't release a request that's not tracked.
return n.requested
// The request should be released if it can't be satisfied by this factory. That
// means either of the following conditions are met :
// - Its score is too high to be satisfied by this factory and it's not already
// assigned to the factory
// - This factory can't satisfy the capability needs of the request
// - The concrete implementation of the factory rejects the request
&& ((n.score > mScore && n.factorySerialNumber != mSerialNumber)
|| !n.request.networkCapabilities.satisfiedByNetworkCapabilities(
mCapabilityFilter)
|| !acceptRequest(n.request, n.score));
}
private void evalRequests() {
for (int i = 0; i < mNetworkRequests.size(); i++) {
NetworkRequestInfo n = mNetworkRequests.valueAt(i);
evalRequest(n);
}
}
@@ -280,16 +368,6 @@ public class NetworkFactory extends Handler {
if (--mRefCount == 0) stopNetwork();
}
public void addNetworkRequest(NetworkRequest networkRequest, int score) {
sendMessage(obtainMessage(CMD_REQUEST_NETWORK,
new NetworkRequestInfo(networkRequest, score)));
}
public void removeNetworkRequest(NetworkRequest networkRequest) {
sendMessage(obtainMessage(CMD_CANCEL_REQUEST, networkRequest));
}
@UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
public void setScoreFilter(int score) {
sendMessage(obtainMessage(CMD_SET_SCORE, score, 0));
@@ -304,6 +382,10 @@ public class NetworkFactory extends Handler {
return mNetworkRequests.size();
}
public int getSerialNumber() {
return mSerialNumber;
}
protected void log(String s) {
Log.d(LOG_TAG, s);
}
@@ -321,10 +403,11 @@ public class NetworkFactory extends Handler {
@Override
public String toString() {
StringBuilder sb = new StringBuilder("{").append(LOG_TAG).append(" - ScoreFilter=").
append(mScore).append(", Filter=").append(mCapabilityFilter).append(", requests=").
append(mNetworkRequests.size()).append(", refCount=").append(mRefCount).
append("}");
StringBuilder sb = new StringBuilder("{").append(LOG_TAG).append(" - mSerialNumber=")
.append(mSerialNumber).append(", ScoreFilter=")
.append(mScore).append(", Filter=").append(mCapabilityFilter).append(", requests=")
.append(mNetworkRequests.size()).append(", refCount=").append(mRefCount)
.append("}");
return sb.toString();
}
}