diff options
4 files changed, 209 insertions, 59 deletions
diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index 66817fad4e5b..736aa46352bd 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -127,7 +127,7 @@ import java.util.function.Predicate; * Any function with the suffix 'Locked' also needs to lock on {@link #mJobs}. * @hide */ -public final class JobSchedulerService extends com.android.server.SystemService +public class JobSchedulerService extends com.android.server.SystemService implements StateChangedListener, JobCompletedListener { public static final String TAG = "JobScheduler"; public static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); @@ -781,6 +781,10 @@ public final class JobSchedulerService extends com.android.server.SystemService } }; + public Context getTestableContext() { + return getContext(); + } + public Object getLock() { return mLock; } diff --git a/services/core/java/com/android/server/job/controllers/ConnectivityController.java b/services/core/java/com/android/server/job/controllers/ConnectivityController.java index 8365fd2e5427..0c66c5b22d76 100644 --- a/services/core/java/com/android/server/job/controllers/ConnectivityController.java +++ b/services/core/java/com/android/server/job/controllers/ConnectivityController.java @@ -30,12 +30,12 @@ import android.net.NetworkInfo; import android.net.NetworkPolicyManager; import android.net.NetworkRequest; import android.net.TrafficStats; -import android.os.Process; import android.os.UserHandle; import android.text.format.DateUtils; import android.util.ArraySet; import android.util.Log; import android.util.Slog; +import android.util.SparseArray; import android.util.proto.ProtoOutputStream; import com.android.internal.annotations.GuardedBy; @@ -46,6 +46,7 @@ import com.android.server.job.JobSchedulerService.Constants; import com.android.server.job.JobServiceContext; import com.android.server.job.StateControllerProto; +import java.util.Objects; import java.util.function.Predicate; /** @@ -63,7 +64,6 @@ public final class ConnectivityController extends StateController implements private final ConnectivityManager mConnManager; private final NetworkPolicyManager mNetPolicyManager; - private boolean mConnected; @GuardedBy("mLock") private final ArraySet<JobStatus> mTrackedJobs = new ArraySet<>(); @@ -74,9 +74,11 @@ public final class ConnectivityController extends StateController implements mConnManager = mContext.getSystemService(ConnectivityManager.class); mNetPolicyManager = mContext.getSystemService(NetworkPolicyManager.class); - mConnected = false; + // We're interested in all network changes; internally we match these + // network changes against the active network for each UID with jobs. + final NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build(); + mConnManager.registerNetworkCallback(request, mNetworkCallback); - mConnManager.registerDefaultNetworkCallback(mNetworkCallback); mNetPolicyManager.registerListener(mNetPolicyListener); } @@ -198,14 +200,18 @@ public final class ConnectivityController extends StateController implements } private boolean updateConstraintsSatisfied(JobStatus jobStatus) { + final Network network = mConnManager.getActiveNetworkForUid(jobStatus.getSourceUid()); + final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network); + return updateConstraintsSatisfied(jobStatus, network, capabilities); + } + + private boolean updateConstraintsSatisfied(JobStatus jobStatus, Network network, + NetworkCapabilities capabilities) { // TODO: consider matching against non-active networks - final int jobUid = jobStatus.getSourceUid(); final boolean ignoreBlocked = (jobStatus.getFlags() & JobInfo.FLAG_WILL_BE_FOREGROUND) != 0; - - final Network network = mConnManager.getActiveNetworkForUid(jobUid, ignoreBlocked); - final NetworkInfo info = mConnManager.getNetworkInfoForUid(network, jobUid, ignoreBlocked); - final NetworkCapabilities capabilities = mConnManager.getNetworkCapabilities(network); + final NetworkInfo info = mConnManager.getNetworkInfoForUid(network, + jobStatus.getSourceUid(), ignoreBlocked); final boolean connected = (info != null) && info.isConnected(); final boolean satisfied = isSatisfied(jobStatus, network, capabilities, mConstants); @@ -218,12 +224,6 @@ public final class ConnectivityController extends StateController implements // using non-default routes. jobStatus.network = network; - // Track system-uid connected/validated as a general reportable proxy for the - // overall state of connectivity constraint satisfiability. - if (jobUid == Process.SYSTEM_UID) { - mConnected = connected; - } - if (DEBUG) { Slog.i(TAG, "Connectivity " + (changed ? "CHANGED" : "unchanged") + " for " + jobStatus + ": connected=" + connected @@ -233,18 +233,48 @@ public final class ConnectivityController extends StateController implements } /** - * Update all jobs tracked by this controller. + * Update any jobs tracked by this controller that match given filters. * - * @param uid only update jobs belonging to this UID, or {@code -1} to + * @param filterUid only update jobs belonging to this UID, or {@code -1} to * update all tracked jobs. + * @param filterNetwork only update jobs that would use this + * {@link Network}, or {@code null} to update all tracked jobs. */ - private void updateTrackedJobs(int uid) { + private void updateTrackedJobs(int filterUid, Network filterNetwork) { synchronized (mLock) { + // Since this is a really hot codepath, temporarily cache any + // answers that we get from ConnectivityManager. + final SparseArray<Network> uidToNetwork = new SparseArray<>(); + final SparseArray<NetworkCapabilities> networkToCapabilities = new SparseArray<>(); + boolean changed = false; - for (int i = mTrackedJobs.size()-1; i >= 0; i--) { + for (int i = mTrackedJobs.size() - 1; i >= 0; i--) { final JobStatus js = mTrackedJobs.valueAt(i); - if (uid == -1 || uid == js.getSourceUid()) { - changed |= updateConstraintsSatisfied(js); + final int uid = js.getSourceUid(); + + final boolean uidMatch = (filterUid == -1 || filterUid == uid); + if (uidMatch) { + Network network = uidToNetwork.get(uid); + if (network == null) { + network = mConnManager.getActiveNetworkForUid(uid); + uidToNetwork.put(uid, network); + } + + // Update either when we have a network match, or when the + // job hasn't yet been evaluated against the currently + // active network; typically when we just lost a network. + final boolean networkMatch = (filterNetwork == null + || Objects.equals(filterNetwork, network)); + final boolean forceUpdate = !Objects.equals(js.network, network); + if (networkMatch || forceUpdate) { + final int netId = network != null ? network.netId : -1; + NetworkCapabilities capabilities = networkToCapabilities.get(netId); + if (capabilities == null) { + capabilities = mConnManager.getNetworkCapabilities(network); + networkToCapabilities.put(netId, capabilities); + } + changed |= updateConstraintsSatisfied(js, network, capabilities); + } } } if (changed) { @@ -273,19 +303,19 @@ public final class ConnectivityController extends StateController implements private final NetworkCallback mNetworkCallback = new NetworkCallback() { @Override - public void onCapabilitiesChanged(Network network, NetworkCapabilities networkCapabilities) { + public void onCapabilitiesChanged(Network network, NetworkCapabilities capabilities) { if (DEBUG) { - Slog.v(TAG, "onCapabilitiesChanged() : " + networkCapabilities); + Slog.v(TAG, "onCapabilitiesChanged: " + network); } - updateTrackedJobs(-1); + updateTrackedJobs(-1, network); } @Override public void onLost(Network network) { if (DEBUG) { - Slog.v(TAG, "Network lost"); + Slog.v(TAG, "onLost: " + network); } - updateTrackedJobs(-1); + updateTrackedJobs(-1, network); } }; @@ -293,25 +323,9 @@ public final class ConnectivityController extends StateController implements @Override public void onUidRulesChanged(int uid, int uidRules) { if (DEBUG) { - Slog.v(TAG, "Uid rules changed for " + uid); + Slog.v(TAG, "onUidRulesChanged: " + uid); } - updateTrackedJobs(uid); - } - - @Override - public void onRestrictBackgroundChanged(boolean restrictBackground) { - if (DEBUG) { - Slog.v(TAG, "Background restriction change to " + restrictBackground); - } - updateTrackedJobs(-1); - } - - @Override - public void onUidPoliciesChanged(int uid, int uidPolicies) { - if (DEBUG) { - Slog.v(TAG, "Uid policy changed for " + uid); - } - updateTrackedJobs(uid); + updateTrackedJobs(uid, null); } }; @@ -319,9 +333,6 @@ public final class ConnectivityController extends StateController implements @Override public void dumpControllerStateLocked(IndentingPrintWriter pw, Predicate<JobStatus> predicate) { - pw.println("System connected: " + mConnected); - pw.println(); - for (int i = 0; i < mTrackedJobs.size(); i++) { final JobStatus js = mTrackedJobs.valueAt(i); if (predicate.test(js)) { @@ -343,8 +354,6 @@ public final class ConnectivityController extends StateController implements final long token = proto.start(fieldId); final long mToken = proto.start(StateControllerProto.CONNECTIVITY); - proto.write(StateControllerProto.ConnectivityController.IS_CONNECTED, mConnected); - for (int i = 0; i < mTrackedJobs.size(); i++) { final JobStatus js = mTrackedJobs.valueAt(i); if (!predicate.test(js)) { diff --git a/services/core/java/com/android/server/job/controllers/StateController.java b/services/core/java/com/android/server/job/controllers/StateController.java index 495109d88fd7..c2be28336406 100644 --- a/services/core/java/com/android/server/job/controllers/StateController.java +++ b/services/core/java/com/android/server/job/controllers/StateController.java @@ -41,7 +41,7 @@ public abstract class StateController { StateController(JobSchedulerService service) { mService = service; mStateChangedListener = service; - mContext = service.getContext(); + mContext = service.getTestableContext(); mLock = service.getLock(); mConstants = service.getConstants(); } diff --git a/services/tests/servicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java b/services/tests/servicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java index 887489423b4c..5b59e607cba7 100644 --- a/services/tests/servicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java @@ -23,19 +23,28 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; import android.app.job.JobInfo; import android.content.ComponentName; +import android.content.Context; import android.content.pm.PackageManagerInternal; +import android.net.ConnectivityManager; +import android.net.ConnectivityManager.NetworkCallback; import android.net.Network; import android.net.NetworkCapabilities; +import android.net.NetworkInfo; +import android.net.NetworkInfo.DetailedState; +import android.net.NetworkPolicyManager; import android.os.Build; -import android.os.Handler; import android.os.SystemClock; -import android.support.test.runner.AndroidJUnit4; import android.util.DataUnit; import com.android.server.LocalServices; @@ -45,14 +54,26 @@ import com.android.server.job.JobSchedulerService.Constants; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import java.time.Clock; import java.time.ZoneOffset; -@RunWith(AndroidJUnit4.class) +@RunWith(MockitoJUnitRunner.class) public class ConnectivityControllerTest { + + @Mock private Context mContext; + @Mock private ConnectivityManager mConnManager; + @Mock private NetworkPolicyManager mNetPolicyManager; + @Mock private JobSchedulerService mService; + private Constants mConstants; + private static final int UID_RED = 10001; + private static final int UID_BLUE = 10002; + @Before public void setUp() throws Exception { // Assume all packages are current SDK @@ -72,6 +93,19 @@ public class ConnectivityControllerTest { // Assume default constants for now mConstants = new Constants(); + + // Get our mocks ready + when(mContext.getSystemServiceName(ConnectivityManager.class)) + .thenReturn(Context.CONNECTIVITY_SERVICE); + when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE)) + .thenReturn(mConnManager); + when(mContext.getSystemServiceName(NetworkPolicyManager.class)) + .thenReturn(Context.NETWORK_POLICY_SERVICE); + when(mContext.getSystemService(Context.NETWORK_POLICY_SERVICE)) + .thenReturn(mNetPolicyManager); + when(mService.getTestableContext()).thenReturn(mContext); + when(mService.getLock()).thenReturn(mService); + when(mService.getConstants()).thenReturn(mConstants); } @Test @@ -155,6 +189,99 @@ public class ConnectivityControllerTest { } } + @Test + public void testUpdates() throws Exception { + final ArgumentCaptor<NetworkCallback> callback = ArgumentCaptor + .forClass(NetworkCallback.class); + doNothing().when(mConnManager).registerNetworkCallback(any(), callback.capture()); + + final ConnectivityController controller = new ConnectivityController(mService); + + final Network meteredNet = new Network(101); + final NetworkCapabilities meteredCaps = createCapabilities(); + final Network unmeteredNet = new Network(202); + final NetworkCapabilities unmeteredCaps = createCapabilities() + .addCapability(NET_CAPABILITY_NOT_METERED); + + final JobStatus red = createJobStatus(createJob() + .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED), UID_RED); + final JobStatus blue = createJobStatus(createJob() + .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), 0) + .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY), UID_BLUE); + + // Pretend we're offline when job is added + { + reset(mConnManager); + answerNetwork(UID_RED, null, null); + answerNetwork(UID_BLUE, null, null); + + controller.maybeStartTrackingJobLocked(red, null); + controller.maybeStartTrackingJobLocked(blue, null); + + assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + assertFalse(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + } + + // Metered network + { + reset(mConnManager); + answerNetwork(UID_RED, meteredNet, meteredCaps); + answerNetwork(UID_BLUE, meteredNet, meteredCaps); + + callback.getValue().onCapabilitiesChanged(meteredNet, meteredCaps); + + assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + } + + // Unmetered network background + { + reset(mConnManager); + answerNetwork(UID_RED, meteredNet, meteredCaps); + answerNetwork(UID_BLUE, meteredNet, meteredCaps); + + callback.getValue().onCapabilitiesChanged(unmeteredNet, unmeteredCaps); + + assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + } + + // Lost metered network + { + reset(mConnManager); + answerNetwork(UID_RED, unmeteredNet, unmeteredCaps); + answerNetwork(UID_BLUE, unmeteredNet, unmeteredCaps); + + callback.getValue().onLost(meteredNet); + + assertTrue(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + } + + // Specific UID was blocked + { + reset(mConnManager); + answerNetwork(UID_RED, null, null); + answerNetwork(UID_BLUE, unmeteredNet, unmeteredCaps); + + callback.getValue().onCapabilitiesChanged(unmeteredNet, unmeteredCaps); + + assertFalse(red.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + assertTrue(blue.isConstraintSatisfied(JobStatus.CONSTRAINT_CONNECTIVITY)); + } + } + + private void answerNetwork(int uid, Network net, NetworkCapabilities caps) { + when(mConnManager.getActiveNetworkForUid(eq(uid))).thenReturn(net); + when(mConnManager.getNetworkCapabilities(eq(net))).thenReturn(caps); + if (net != null) { + final NetworkInfo ni = new NetworkInfo(ConnectivityManager.TYPE_WIFI, 0, null, null); + ni.setDetailedState(DetailedState.CONNECTED, null, null); + when(mConnManager.getNetworkInfoForUid(eq(net), eq(uid), anyBoolean())).thenReturn(ni); + } + } + private static NetworkCapabilities createCapabilities() { return new NetworkCapabilities().addCapability(NET_CAPABILITY_INTERNET) .addCapability(NET_CAPABILITY_VALIDATED); @@ -165,12 +292,22 @@ public class ConnectivityControllerTest { } private static JobStatus createJobStatus(JobInfo.Builder job) { - return createJobStatus(job, 0, Long.MAX_VALUE); + return createJobStatus(job, android.os.Process.NOBODY_UID, 0, Long.MAX_VALUE); + } + + private static JobStatus createJobStatus(JobInfo.Builder job, int uid) { + return createJobStatus(job, uid, 0, Long.MAX_VALUE); + } + + private static JobStatus createJobStatus(JobInfo.Builder job, + long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) { + return createJobStatus(job, android.os.Process.NOBODY_UID, + earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis); } - private static JobStatus createJobStatus(JobInfo.Builder job, long earliestRunTimeElapsedMillis, - long latestRunTimeElapsedMillis) { - return new JobStatus(job.build(), 0, null, -1, 0, 0, null, earliestRunTimeElapsedMillis, - latestRunTimeElapsedMillis, 0, 0, null, 0); + private static JobStatus createJobStatus(JobInfo.Builder job, int uid, + long earliestRunTimeElapsedMillis, long latestRunTimeElapsedMillis) { + return new JobStatus(job.build(), uid, null, -1, 0, 0, null, + earliestRunTimeElapsedMillis, latestRunTimeElapsedMillis, 0, 0, null, 0); } } |