From 316b14147da1eaa02c92e142e222af8ea5680355 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Fri, 14 Jun 2019 11:34:37 -0700 Subject: [PATCH] Remove deprecated & removed JobInfo methods. These methods were marked deprecated and tagged with @removed before Pie shipped but ConnectivityController wasn't updated to perform the proper computations. This removes the deprecated methods altogether and updates ConnectivityController accordingly. Bug: 135214188 Test: atest com.android.server.cts.JobSchedulerIncidentTest Test: atest com.android.server.job.controllers.ConnectivityControllerTest Test: atest com.android.providers.downloads.DownloadProviderFunctionalTest Change-Id: I7c364a4326c31e8aae54a4fc69703434d8c7c915 --- api/removed.txt | 18 ----- core/java/android/app/job/JobInfo.java | 38 ----------- core/java/android/app/job/JobWorkItem.java | 28 -------- core/proto/android/server/jobscheduler.proto | 12 +++- .../controllers/ConnectivityController.java | 55 ++++++++------- .../server/job/controllers/JobStatus.java | 68 +++++++++++-------- .../ConnectivityControllerTest.java | 19 ++++-- 7 files changed, 99 insertions(+), 139 deletions(-) diff --git a/api/removed.txt b/api/removed.txt index 6128e49aa443d..8ebded13b5a48 100644 --- a/api/removed.txt +++ b/api/removed.txt @@ -32,24 +32,6 @@ package android.app.admin { } -package android.app.job { - - public class JobInfo implements android.os.Parcelable { - method @Deprecated public long getEstimatedNetworkBytes(); - } - - public static final class JobInfo.Builder { - method @Deprecated public android.app.job.JobInfo.Builder setEstimatedNetworkBytes(long); - method @Deprecated public android.app.job.JobInfo.Builder setIsPrefetch(boolean); - } - - public final class JobWorkItem implements android.os.Parcelable { - ctor @Deprecated public JobWorkItem(android.content.Intent, long); - method @Deprecated public long getEstimatedNetworkBytes(); - } - -} - package android.app.slice { public final class Slice implements android.os.Parcelable { diff --git a/core/java/android/app/job/JobInfo.java b/core/java/android/app/job/JobInfo.java index a8f89df8058c2..8b3b3a28f2bc9 100644 --- a/core/java/android/app/job/JobInfo.java +++ b/core/java/android/app/job/JobInfo.java @@ -480,25 +480,6 @@ public class JobInfo implements Parcelable { return networkRequest; } - /** - * @deprecated replaced by {@link #getEstimatedNetworkDownloadBytes()} and - * {@link #getEstimatedNetworkUploadBytes()}. - * @removed - */ - @Deprecated - public @BytesLong long getEstimatedNetworkBytes() { - if (networkDownloadBytes == NETWORK_BYTES_UNKNOWN - && networkUploadBytes == NETWORK_BYTES_UNKNOWN) { - return NETWORK_BYTES_UNKNOWN; - } else if (networkDownloadBytes == NETWORK_BYTES_UNKNOWN) { - return networkUploadBytes; - } else if (networkUploadBytes == NETWORK_BYTES_UNKNOWN) { - return networkDownloadBytes; - } else { - return networkDownloadBytes + networkUploadBytes; - } - } - /** * Return the estimated size of download traffic that will be performed by * this job, in bytes. @@ -1177,16 +1158,6 @@ public class JobInfo implements Parcelable { return this; } - /** - * @deprecated replaced by - * {@link #setEstimatedNetworkBytes(long, long)}. - * @removed - */ - @Deprecated - public Builder setEstimatedNetworkBytes(@BytesLong long networkBytes) { - return setEstimatedNetworkBytes(networkBytes, NETWORK_BYTES_UNKNOWN); - } - /** * Set the estimated size of network traffic that will be performed by * this job, in bytes. @@ -1496,15 +1467,6 @@ public class JobInfo implements Parcelable { return this; } - /** - * @removed - * @deprecated replaced with {@link #setPrefetch(boolean)} - */ - @Deprecated - public Builder setIsPrefetch(boolean isPrefetch) { - return setPrefetch(isPrefetch); - } - /** * Setting this to true indicates that this job is designed to prefetch * content that will make a material improvement to the experience of diff --git a/core/java/android/app/job/JobWorkItem.java b/core/java/android/app/job/JobWorkItem.java index a055ab48c7662..c6631fa764949 100644 --- a/core/java/android/app/job/JobWorkItem.java +++ b/core/java/android/app/job/JobWorkItem.java @@ -54,15 +54,6 @@ final public class JobWorkItem implements Parcelable { mNetworkUploadBytes = NETWORK_BYTES_UNKNOWN; } - /** - * @deprecated replaced by {@link #JobWorkItem(Intent, long, long)} - * @removed - */ - @Deprecated - public JobWorkItem(Intent intent, @BytesLong long networkBytes) { - this(intent, networkBytes, NETWORK_BYTES_UNKNOWN); - } - /** * Create a new piece of work, which can be submitted to * {@link JobScheduler#enqueue JobScheduler.enqueue}. @@ -89,25 +80,6 @@ final public class JobWorkItem implements Parcelable { return mIntent; } - /** - * @deprecated replaced by {@link #getEstimatedNetworkDownloadBytes()} and - * {@link #getEstimatedNetworkUploadBytes()}. - * @removed - */ - @Deprecated - public @BytesLong long getEstimatedNetworkBytes() { - if (mNetworkDownloadBytes == NETWORK_BYTES_UNKNOWN - && mNetworkUploadBytes == NETWORK_BYTES_UNKNOWN) { - return NETWORK_BYTES_UNKNOWN; - } else if (mNetworkDownloadBytes == NETWORK_BYTES_UNKNOWN) { - return mNetworkUploadBytes; - } else if (mNetworkUploadBytes == NETWORK_BYTES_UNKNOWN) { - return mNetworkDownloadBytes; - } else { - return mNetworkDownloadBytes + mNetworkUploadBytes; - } - } - /** * Return the estimated size of download traffic that will be performed by * this job, in bytes. diff --git a/core/proto/android/server/jobscheduler.proto b/core/proto/android/server/jobscheduler.proto index 28733798b9ad5..5a57163d0e4b5 100644 --- a/core/proto/android/server/jobscheduler.proto +++ b/core/proto/android/server/jobscheduler.proto @@ -833,7 +833,15 @@ message JobStatusDumpProto { optional .android.net.NetworkRequestProto required_network = 18; - optional int64 total_network_bytes = 19; + reserved 19; // total_network_bytes + + // The estimated download bytes of the job. Will have a value of + // {@link JobInfo.NETWORK_BYTES_UNKNOWN} if any part of the job download is unknown. + optional int64 total_network_download_bytes = 25; + + // The estimated upload bytes of the job. Will have a value of + // {@link JobInfo.NETWORK_BYTES_UNKNOWN} if any part of the job upload is unknown. + optional int64 total_network_upload_bytes = 26; optional int64 min_latency_ms = 20; optional int64 max_execution_delay_ms = 21; @@ -852,6 +860,8 @@ message JobStatusDumpProto { optional bool has_early_constraint = 23; optional bool has_late_constraint = 24; + + // Next tag: 27 } optional JobInfo job_info = 6; 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 c820841663987..490e87f9d1031 100644 --- a/services/core/java/com/android/server/job/controllers/ConnectivityController.java +++ b/services/core/java/com/android/server/job/controllers/ConnectivityController.java @@ -29,13 +29,13 @@ import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkPolicyManager; import android.net.NetworkRequest; -import android.net.TrafficStats; import android.os.Handler; import android.os.Looper; import android.os.Message; import android.os.UserHandle; import android.text.format.DateUtils; import android.util.ArraySet; +import android.util.DataUnit; import android.util.Log; import android.util.Slog; import android.util.SparseArray; @@ -321,32 +321,41 @@ public final class ConnectivityController extends StateController implements @SuppressWarnings("unused") private static boolean isInsane(JobStatus jobStatus, Network network, NetworkCapabilities capabilities, Constants constants) { - final long estimatedBytes = jobStatus.getEstimatedNetworkBytes(); - if (estimatedBytes == JobInfo.NETWORK_BYTES_UNKNOWN) { - // We don't know how large the job is; cross our fingers! - return false; + final long downloadBytes = jobStatus.getEstimatedNetworkDownloadBytes(); + if (downloadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + final long bandwidth = capabilities.getLinkDownstreamBandwidthKbps(); + // If we don't know the bandwidth, all we can do is hope the job finishes in time. + if (bandwidth != LINK_BANDWIDTH_UNSPECIFIED) { + // Divide by 8 to convert bits to bytes. + final long estimatedMillis = ((downloadBytes * DateUtils.SECOND_IN_MILLIS) + / (DataUnit.KIBIBYTES.toBytes(bandwidth) / 8)); + if (estimatedMillis > JobServiceContext.EXECUTING_TIMESLICE_MILLIS) { + // If we'd never finish before the timeout, we'd be insane! + Slog.w(TAG, "Estimated " + downloadBytes + " download bytes over " + bandwidth + + " kbps network would take " + estimatedMillis + "ms; that's insane!"); + return true; + } + } } - // We don't ask developers to differentiate between upstream/downstream - // in their size estimates, so test against the slowest link direction. - final long slowest = NetworkCapabilities.minBandwidth( - capabilities.getLinkDownstreamBandwidthKbps(), - capabilities.getLinkUpstreamBandwidthKbps()); - if (slowest == LINK_BANDWIDTH_UNSPECIFIED) { - // We don't know what the network is like; cross our fingers! - return false; + final long uploadBytes = jobStatus.getEstimatedNetworkUploadBytes(); + if (uploadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + final long bandwidth = capabilities.getLinkUpstreamBandwidthKbps(); + // If we don't know the bandwidth, all we can do is hope the job finishes in time. + if (bandwidth != LINK_BANDWIDTH_UNSPECIFIED) { + // Divide by 8 to convert bits to bytes. + final long estimatedMillis = ((uploadBytes * DateUtils.SECOND_IN_MILLIS) + / (DataUnit.KIBIBYTES.toBytes(bandwidth) / 8)); + if (estimatedMillis > JobServiceContext.EXECUTING_TIMESLICE_MILLIS) { + // If we'd never finish before the timeout, we'd be insane! + Slog.w(TAG, "Estimated " + uploadBytes + " upload bytes over " + bandwidth + + " kbps network would take " + estimatedMillis + "ms; that's insane!"); + return true; + } + } } - final long estimatedMillis = ((estimatedBytes * DateUtils.SECOND_IN_MILLIS) - / (slowest * TrafficStats.KB_IN_BYTES / 8)); - if (estimatedMillis > JobServiceContext.EXECUTING_TIMESLICE_MILLIS) { - // If we'd never finish before the timeout, we'd be insane! - Slog.w(TAG, "Estimated " + estimatedBytes + " bytes over " + slowest - + " kbps network would take " + estimatedMillis + "ms; that's insane!"); - return true; - } else { - return false; - } + return false; } @SuppressWarnings("unused") diff --git a/services/core/java/com/android/server/job/controllers/JobStatus.java b/services/core/java/com/android/server/job/controllers/JobStatus.java index eb5d472f7fbc3..a628b17a3289e 100644 --- a/services/core/java/com/android/server/job/controllers/JobStatus.java +++ b/services/core/java/com/android/server/job/controllers/JobStatus.java @@ -305,7 +305,8 @@ public final class JobStatus { */ ContentObserverController.JobInstance contentObserverJobInstance; - private long totalNetworkBytes = JobInfo.NETWORK_BYTES_UNKNOWN; + private long mTotalNetworkDownloadBytes = JobInfo.NETWORK_BYTES_UNKNOWN; + private long mTotalNetworkUploadBytes = JobInfo.NETWORK_BYTES_UNKNOWN; /////// Booleans that track if a job is ready to run. They should be updated whenever dependent /////// states change. @@ -787,35 +788,39 @@ public final class JobStatus { } private void updateEstimatedNetworkBytesLocked() { - totalNetworkBytes = computeEstimatedNetworkBytesLocked(); - } + mTotalNetworkDownloadBytes = job.getEstimatedNetworkDownloadBytes(); + mTotalNetworkUploadBytes = job.getEstimatedNetworkUploadBytes(); - private long computeEstimatedNetworkBytesLocked() { - // If any component of the job has unknown usage, we don't have a - // complete picture of what data will be used, and we have to treat the - // entire job as unknown. - long totalNetworkBytes = 0; - long networkBytes = job.getEstimatedNetworkBytes(); - if (networkBytes == JobInfo.NETWORK_BYTES_UNKNOWN) { - return JobInfo.NETWORK_BYTES_UNKNOWN; - } else { - totalNetworkBytes += networkBytes; - } if (pendingWork != null) { for (int i = 0; i < pendingWork.size(); i++) { - networkBytes = pendingWork.get(i).getEstimatedNetworkBytes(); - if (networkBytes == JobInfo.NETWORK_BYTES_UNKNOWN) { - return JobInfo.NETWORK_BYTES_UNKNOWN; - } else { - totalNetworkBytes += networkBytes; + if (mTotalNetworkDownloadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + // If any component of the job has unknown usage, we don't have a + // complete picture of what data will be used, and we have to treat the + // entire up/download as unknown. + long downloadBytes = pendingWork.get(i).getEstimatedNetworkDownloadBytes(); + if (downloadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + mTotalNetworkDownloadBytes += downloadBytes; + } + } + if (mTotalNetworkUploadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + // If any component of the job has unknown usage, we don't have a + // complete picture of what data will be used, and we have to treat the + // entire up/download as unknown. + long uploadBytes = pendingWork.get(i).getEstimatedNetworkUploadBytes(); + if (uploadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + mTotalNetworkUploadBytes += uploadBytes; + } } } } - return totalNetworkBytes; } - public long getEstimatedNetworkBytes() { - return totalNetworkBytes; + public long getEstimatedNetworkDownloadBytes() { + return mTotalNetworkDownloadBytes; + } + + public long getEstimatedNetworkUploadBytes() { + return mTotalNetworkUploadBytes; } /** Does this job have any sort of networking constraint? */ @@ -1524,9 +1529,13 @@ public final class JobStatus { pw.print(prefix); pw.print(" Network type: "); pw.println(job.getRequiredNetwork()); } - if (totalNetworkBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { - pw.print(prefix); pw.print(" Network bytes: "); - pw.println(totalNetworkBytes); + if (mTotalNetworkDownloadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + pw.print(prefix); pw.print(" Network download bytes: "); + pw.println(mTotalNetworkDownloadBytes); + } + if (mTotalNetworkUploadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + pw.print(prefix); pw.print(" Network upload bytes: "); + pw.println(mTotalNetworkUploadBytes); } if (job.getMinLatencyMillis() != 0) { pw.print(prefix); pw.print(" Minimum latency: "); @@ -1721,8 +1730,13 @@ public final class JobStatus { if (job.getRequiredNetwork() != null) { job.getRequiredNetwork().writeToProto(proto, JobStatusDumpProto.JobInfo.REQUIRED_NETWORK); } - if (totalNetworkBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { - proto.write(JobStatusDumpProto.JobInfo.TOTAL_NETWORK_BYTES, totalNetworkBytes); + if (mTotalNetworkDownloadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + proto.write(JobStatusDumpProto.JobInfo.TOTAL_NETWORK_DOWNLOAD_BYTES, + mTotalNetworkDownloadBytes); + } + if (mTotalNetworkUploadBytes != JobInfo.NETWORK_BYTES_UNKNOWN) { + proto.write(JobStatusDumpProto.JobInfo.TOTAL_NETWORK_UPLOAD_BYTES, + mTotalNetworkUploadBytes); } proto.write(JobStatusDumpProto.JobInfo.MIN_LATENCY_MS, job.getMinLatencyMillis()); proto.write(JobStatusDumpProto.JobInfo.MAX_EXECUTION_DELAY_MS, job.getMaxExecutionDelayMillis()); diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java index 9fe7b7c77530f..823cc66975ddd 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/ConnectivityControllerTest.java @@ -134,13 +134,22 @@ public class ConnectivityControllerTest { public void testInsane() throws Exception { final Network net = new Network(101); final JobInfo.Builder job = createJob() - .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1)) + .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), + DataUnit.MEBIBYTES.toBytes(1)) .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY); // Slow network is too slow assertFalse(ConnectivityController.isSatisfied(createJobStatus(job), net, createCapabilities().setLinkUpstreamBandwidthKbps(1) .setLinkDownstreamBandwidthKbps(1), mConstants)); + // Slow downstream + assertFalse(ConnectivityController.isSatisfied(createJobStatus(job), net, + createCapabilities().setLinkUpstreamBandwidthKbps(1024) + .setLinkDownstreamBandwidthKbps(1), mConstants)); + // Slow upstream + assertFalse(ConnectivityController.isSatisfied(createJobStatus(job), net, + createCapabilities().setLinkUpstreamBandwidthKbps(1) + .setLinkDownstreamBandwidthKbps(1024), mConstants)); // Fast network looks great assertTrue(ConnectivityController.isSatisfied(createJobStatus(job), net, createCapabilities().setLinkUpstreamBandwidthKbps(1024) @@ -151,7 +160,8 @@ public class ConnectivityControllerTest { public void testCongestion() throws Exception { final long now = JobSchedulerService.sElapsedRealtimeClock.millis(); final JobInfo.Builder job = createJob() - .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1)) + .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), + DataUnit.MEBIBYTES.toBytes(1)) .setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY); final JobStatus early = createJobStatus(job, now - 1000, now + 2000); final JobStatus late = createJobStatus(job, now - 2000, now + 1000); @@ -178,12 +188,13 @@ public class ConnectivityControllerTest { public void testRelaxed() throws Exception { final long now = JobSchedulerService.sElapsedRealtimeClock.millis(); final JobInfo.Builder job = createJob() - .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1)) + .setEstimatedNetworkBytes(DataUnit.MEBIBYTES.toBytes(1), + DataUnit.MEBIBYTES.toBytes(1)) .setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED); final JobStatus early = createJobStatus(job, now - 1000, now + 2000); final JobStatus late = createJobStatus(job, now - 2000, now + 1000); - job.setIsPrefetch(true); + job.setPrefetch(true); final JobStatus earlyPrefetch = createJobStatus(job, now - 1000, now + 2000); final JobStatus latePrefetch = createJobStatus(job, now - 2000, now + 1000);