From 141f11c82a2dbf042833f75aeae6f028e8ae2084 Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Tue, 5 Apr 2016 15:46:12 -0700 Subject: [PATCH] More work on issue #26390151: Add new JobScheduler API... ...for monitoring content providers - Improve media provider change reporting so that observers can avoid spurious reports of the top-level content directory changing. - Fix a bug where collected content changes while a job was running were not being properly propagated to the next job. Change-Id: I29e3c2960e6fec75b16ee3ee6588d47342bf8c75 --- api/current.txt | 3 + api/system-current.txt | 3 + api/test-current.txt | 3 + .../java/android/content/ContentResolver.java | 75 ++++++++++++++++++- .../java/android/content/IContentService.aidl | 2 +- .../server/content/ContentService.java | 70 +++++++++++------ .../server/job/JobSchedulerService.java | 23 +++--- .../job/controllers/AppIdleController.java | 2 +- .../job/controllers/BatteryController.java | 2 +- .../controllers/ConnectivityController.java | 2 +- .../ContentObserverController.java | 46 +++++++----- .../controllers/DeviceIdleJobsController.java | 2 +- .../job/controllers/IdleController.java | 2 +- .../job/controllers/StateController.java | 3 +- .../job/controllers/TimeController.java | 4 +- .../server/content/ObserverNodeTest.java | 4 +- 16 files changed, 180 insertions(+), 66 deletions(-) diff --git a/api/current.txt b/api/current.txt index b5fb23a33b36b..a7aba250733a3 100644 --- a/api/current.txt +++ b/api/current.txt @@ -7885,6 +7885,7 @@ package android.content { method public static boolean isSyncPending(android.accounts.Account, java.lang.String); method public void notifyChange(android.net.Uri, android.database.ContentObserver); method public void notifyChange(android.net.Uri, android.database.ContentObserver, boolean); + method public void notifyChange(android.net.Uri, android.database.ContentObserver, int); method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String, android.os.CancellationSignal) throws java.io.FileNotFoundException; method public final android.os.ParcelFileDescriptor openFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; @@ -7915,6 +7916,8 @@ package android.content { field public static final java.lang.String CURSOR_DIR_BASE_TYPE = "vnd.android.cursor.dir"; field public static final java.lang.String CURSOR_ITEM_BASE_TYPE = "vnd.android.cursor.item"; field public static final java.lang.String EXTRA_SIZE = "android.content.extra.SIZE"; + field public static final int NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS = 2; // 0x2 + field public static final int NOTIFY_SYNC_TO_NETWORK = 1; // 0x1 field public static final java.lang.String SCHEME_ANDROID_RESOURCE = "android.resource"; field public static final java.lang.String SCHEME_CONTENT = "content"; field public static final java.lang.String SCHEME_FILE = "file"; diff --git a/api/system-current.txt b/api/system-current.txt index b939a66beedef..f3f83644b9216 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -8183,6 +8183,7 @@ package android.content { method public static boolean isSyncPending(android.accounts.Account, java.lang.String); method public void notifyChange(android.net.Uri, android.database.ContentObserver); method public void notifyChange(android.net.Uri, android.database.ContentObserver, boolean); + method public void notifyChange(android.net.Uri, android.database.ContentObserver, int); method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String, android.os.CancellationSignal) throws java.io.FileNotFoundException; method public final android.os.ParcelFileDescriptor openFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; @@ -8213,6 +8214,8 @@ package android.content { field public static final java.lang.String CURSOR_DIR_BASE_TYPE = "vnd.android.cursor.dir"; field public static final java.lang.String CURSOR_ITEM_BASE_TYPE = "vnd.android.cursor.item"; field public static final java.lang.String EXTRA_SIZE = "android.content.extra.SIZE"; + field public static final int NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS = 2; // 0x2 + field public static final int NOTIFY_SYNC_TO_NETWORK = 1; // 0x1 field public static final java.lang.String SCHEME_ANDROID_RESOURCE = "android.resource"; field public static final java.lang.String SCHEME_CONTENT = "content"; field public static final java.lang.String SCHEME_FILE = "file"; diff --git a/api/test-current.txt b/api/test-current.txt index ddf787bfa2503..51110e023b3be 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -7890,6 +7890,7 @@ package android.content { method public static boolean isSyncPending(android.accounts.Account, java.lang.String); method public void notifyChange(android.net.Uri, android.database.ContentObserver); method public void notifyChange(android.net.Uri, android.database.ContentObserver, boolean); + method public void notifyChange(android.net.Uri, android.database.ContentObserver, int); method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; method public final android.content.res.AssetFileDescriptor openAssetFileDescriptor(android.net.Uri, java.lang.String, android.os.CancellationSignal) throws java.io.FileNotFoundException; method public final android.os.ParcelFileDescriptor openFileDescriptor(android.net.Uri, java.lang.String) throws java.io.FileNotFoundException; @@ -7920,6 +7921,8 @@ package android.content { field public static final java.lang.String CURSOR_DIR_BASE_TYPE = "vnd.android.cursor.dir"; field public static final java.lang.String CURSOR_ITEM_BASE_TYPE = "vnd.android.cursor.item"; field public static final java.lang.String EXTRA_SIZE = "android.content.extra.SIZE"; + field public static final int NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS = 2; // 0x2 + field public static final int NOTIFY_SYNC_TO_NETWORK = 1; // 0x1 field public static final java.lang.String SCHEME_ANDROID_RESOURCE = "android.resource"; field public static final java.lang.String SCHEME_CONTENT = "content"; field public static final java.lang.String SCHEME_FILE = "file"; diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java index cd67b3ee8a3ef..4db45670dd85b 100644 --- a/core/java/android/content/ContentResolver.java +++ b/core/java/android/content/ContentResolver.java @@ -17,6 +17,7 @@ package android.content; import android.accounts.Account; +import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; @@ -60,6 +61,8 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.List; import java.util.Random; @@ -289,6 +292,31 @@ public abstract class ContentResolver { /** @hide */ public static final int SYNC_OBSERVER_TYPE_ALL = 0x7fffffff; + /** @hide */ + @IntDef(flag = true, + value = { + NOTIFY_SYNC_TO_NETWORK, + NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS + }) + @Retention(RetentionPolicy.SOURCE) + public @interface NotifyFlags {} + + /** + * Flag for {@link #notifyChange(Uri, ContentObserver, int)}: attempt to sync the change + * to the network. + */ + public static final int NOTIFY_SYNC_TO_NETWORK = 1<<0; + + /** + * Flag for {@link #notifyChange(Uri, ContentObserver, int)}: if set, this notification + * will be skipped if it is being delivered to the root URI of a ContentObserver that is + * using "notify for descendants." The purpose of this is to allow the provide to send + * a general notification of "something under X" changed that observers of that specific + * URI can receive, while also sending a specific URI under X. It would use this flag + * when sending the former, so that observers of "X and descendants" only see the latter. + */ + public static final int NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS = 1<<1; + // Always log queries which take 500ms+; shorter queries are // sampled accordingly. private static final boolean ENABLE_CONTENT_SAMPLE = false; @@ -1676,7 +1704,7 @@ public abstract class ContentResolver { * The observer that originated the change will only receive the notification if it * has requested to receive self-change notifications by implementing * {@link ContentObserver#deliverSelfNotifications()} to return true. - * @param syncToNetwork If true, attempt to sync the change to the network. + * @param syncToNetwork If true, same as {@link #NOTIFY_SYNC_TO_NETWORK}. * @see #requestSync(android.accounts.Account, String, android.os.Bundle) */ public void notifyChange(@NonNull Uri uri, @Nullable ContentObserver observer, @@ -1689,6 +1717,32 @@ public abstract class ContentResolver { ContentProvider.getUserIdFromUri(uri, UserHandle.myUserId())); } + /** + * Notify registered observers that a row was updated. + * To register, call {@link #registerContentObserver(android.net.Uri, boolean, android.database.ContentObserver) registerContentObserver()}. + * By default, CursorAdapter objects will get this notification. + * If syncToNetwork is true, this will attempt to schedule a local sync using the sync + * adapter that's registered for the authority of the provided uri. No account will be + * passed to the sync adapter, so all matching accounts will be synchronized. + * + * @param uri The uri of the content that was changed. + * @param observer The observer that originated the change, may be null. + * The observer that originated the change will only receive the notification if it + * has requested to receive self-change notifications by implementing + * {@link ContentObserver#deliverSelfNotifications()} to return true. + * @param flags Additional flags: {@link #NOTIFY_SYNC_TO_NETWORK}. + * @see #requestSync(android.accounts.Account, String, android.os.Bundle) + */ + public void notifyChange(@NonNull Uri uri, @Nullable ContentObserver observer, + @NotifyFlags int flags) { + Preconditions.checkNotNull(uri, "uri"); + notifyChange( + ContentProvider.getUriWithoutUserId(uri), + observer, + flags, + ContentProvider.getUserIdFromUri(uri, UserHandle.myUserId())); + } + /** * Notify registered observers within the designated user(s) that a row was updated. * @@ -1699,7 +1753,24 @@ public abstract class ContentResolver { try { getContentService().notifyChange( uri, observer == null ? null : observer.getContentObserver(), - observer != null && observer.deliverSelfNotifications(), syncToNetwork, + observer != null && observer.deliverSelfNotifications(), + syncToNetwork ? NOTIFY_SYNC_TO_NETWORK : 0, + userHandle); + } catch (RemoteException e) { + } + } + + /** + * Notify registered observers within the designated user(s) that a row was updated. + * + * @hide + */ + public void notifyChange(Uri uri, ContentObserver observer, @NotifyFlags int flags, + @UserIdInt int userHandle) { + try { + getContentService().notifyChange( + uri, observer == null ? null : observer.getContentObserver(), + observer != null && observer.deliverSelfNotifications(), flags, userHandle); } catch (RemoteException e) { } diff --git a/core/java/android/content/IContentService.aidl b/core/java/android/content/IContentService.aidl index d47e780fa4d23..3446e0315a1d0 100644 --- a/core/java/android/content/IContentService.aidl +++ b/core/java/android/content/IContentService.aidl @@ -52,7 +52,7 @@ interface IContentService { * USER_CURRENT are properly interpreted. */ void notifyChange(in Uri uri, IContentObserver observer, - boolean observerWantsSelfNotifications, boolean syncToNetwork, + boolean observerWantsSelfNotifications, int flags, int userHandle); void requestSync(in Account account, String authority, in Bundle extras); diff --git a/services/core/java/com/android/server/content/ContentService.java b/services/core/java/com/android/server/content/ContentService.java index e9d962827632e..6e7ea9925f91b 100644 --- a/services/core/java/com/android/server/content/ContentService.java +++ b/services/core/java/com/android/server/content/ContentService.java @@ -73,7 +73,8 @@ import java.util.List; * {@hide} */ public final class ContentService extends IContentService.Stub { - private static final String TAG = "ContentService"; + static final String TAG = "ContentService"; + static final boolean DEBUG = false; public static class Lifecycle extends SystemService { private ContentService mContentService; @@ -339,12 +340,10 @@ public final class ContentService extends IContentService.Stub { */ @Override public void notifyChange(Uri uri, IContentObserver observer, - boolean observerWantsSelfNotifications, boolean syncToNetwork, + boolean observerWantsSelfNotifications, int flags, int userHandle) { - if (Log.isLoggable(TAG, Log.VERBOSE)) { - Log.v(TAG, "Notifying update of " + uri + " for user " + userHandle - + " from observer " + observer + ", syncToNetwork " + syncToNetwork); - } + if (DEBUG) Slog.d(TAG, "Notifying update of " + uri + " for user " + userHandle + + " from observer " + observer + ", flags " + Integer.toHexString(flags)); final int uid = Binder.getCallingUid(); final int pid = Binder.getCallingPid(); @@ -373,16 +372,15 @@ public final class ContentService extends IContentService.Stub { ArrayList calls = new ArrayList(); synchronized (mRootNode) { mRootNode.collectObserversLocked(uri, 0, observer, observerWantsSelfNotifications, - userHandle, calls); + flags, userHandle, calls); } final int numCalls = calls.size(); for (int i=0; i key = packageCache.keyAt(i); if (key.second != null && key.second.toString().startsWith(uri.toString())) { - Slog.d(TAG, "Invalidating cache for key " + key); + if (DEBUG) Slog.d(TAG, "Invalidating cache for key " + key); packageCache.removeAt(i); } else { i++; } } } else { - Slog.d(TAG, "Invalidating cache for package " + providerPackageName); + if (DEBUG) Slog.d(TAG, "Invalidating cache for package " + providerPackageName); packageCache.clear(); } } @@ -1310,8 +1309,8 @@ public final class ContentService extends IContentService.Stub { } private void collectMyObserversLocked(boolean leaf, IContentObserver observer, - boolean observerWantsSelfNotifications, int targetUserHandle, - ArrayList calls) { + boolean observerWantsSelfNotifications, int flags, + int targetUserHandle, ArrayList calls) { int N = mObservers.size(); IBinder observerBinder = observer == null ? null : observer.asBinder(); for (int i = 0; i < N; i++) { @@ -1329,9 +1328,29 @@ public final class ContentService extends IContentService.Stub { || entry.userHandle == UserHandle.USER_ALL || targetUserHandle == entry.userHandle) { // Make sure the observer is interested in the notification - if (leaf || (!leaf && entry.notifyForDescendants)) { - calls.add(new ObserverCall(this, entry.observer, selfChange)); + if (leaf) { + // If we are at the leaf: we always report, unless the sender has asked + // to skip observers that are notifying for descendants (since they will + // be sending another more specific URI for them). + if ((flags&ContentResolver.NOTIFY_SKIP_NOTIFY_FOR_DESCENDANTS) != 0 + && entry.notifyForDescendants) { + if (DEBUG) Slog.d(TAG, "Skipping " + entry.observer + + ": skip notify for descendants"); + continue; + } + } else { + // If we are not at the leaf: we report if the observer says it wants + // to be notified for all descendants. + if (!entry.notifyForDescendants) { + if (DEBUG) Slog.d(TAG, "Skipping " + entry.observer + + ": not monitor descendants"); + continue; + } } + if (DEBUG) Slog.d(TAG, "Reporting to " + entry.observer + ": leaf=" + leaf + + " flags=" + Integer.toHexString(flags) + + " desc=" + entry.notifyForDescendants); + calls.add(new ObserverCall(this, entry.observer, selfChange)); } } } @@ -1340,19 +1359,22 @@ public final class ContentService extends IContentService.Stub { * targetUserHandle is either a hard user handle or is USER_ALL */ public void collectObserversLocked(Uri uri, int index, IContentObserver observer, - boolean observerWantsSelfNotifications, int targetUserHandle, - ArrayList calls) { + boolean observerWantsSelfNotifications, int flags, + int targetUserHandle, ArrayList calls) { String segment = null; int segmentCount = countUriSegments(uri); if (index >= segmentCount) { // This is the leaf node, notify all observers + if (DEBUG) Slog.d(TAG, "Collecting leaf observers @ #" + index + ", node " + mName); collectMyObserversLocked(true, observer, observerWantsSelfNotifications, - targetUserHandle, calls); + flags, targetUserHandle, calls); } else if (index < segmentCount){ segment = getUriSegment(uri, index); + if (DEBUG) Slog.d(TAG, "Collecting non-leaf observers @ #" + index + " / " + + segment); // Notify any observers at this level who are interested in descendants collectMyObserversLocked(false, observer, observerWantsSelfNotifications, - targetUserHandle, calls); + flags, targetUserHandle, calls); } int N = mChildren.size(); @@ -1360,8 +1382,8 @@ public final class ContentService extends IContentService.Stub { ObserverNode node = mChildren.get(i); if (segment == null || node.mName.equals(segment)) { // We found the child, - node.collectObserversLocked(uri, index + 1, - observer, observerWantsSelfNotifications, targetUserHandle, calls); + node.collectObserversLocked(uri, index + 1, observer, + observerWantsSelfNotifications, flags, targetUserHandle, calls); if (segment != null) { break; } diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index fa8620f20a8f3..b2350025152fa 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -304,7 +304,7 @@ public final class JobSchedulerService extends com.android.server.SystemService toCancel = mJobs.getJobByUidAndJobId(uId, job.getId()); if (toCancel != null) { - cancelJobImpl(toCancel); + cancelJobImpl(toCancel, jobStatus); } startTrackingJob(jobStatus, toCancel); } @@ -331,7 +331,7 @@ public final class JobSchedulerService extends com.android.server.SystemService } for (int i=0; i it = mTrackedJobs.listIterator(mTrackedJobs.size()); while (it.hasPrevious()) { @@ -101,7 +101,7 @@ public class TimeController extends StateController { * Really an == comparison should be enough, but why play with fate? We'll do <=. */ @Override - public void maybeStopTrackingJobLocked(JobStatus job, boolean forUpdate) { + public void maybeStopTrackingJobLocked(JobStatus job, JobStatus incomingJob, boolean forUpdate) { if (mTrackedJobs.remove(job)) { checkExpiredDelaysAndResetAlarm(); checkExpiredDeadlinesAndResetAlarm(); diff --git a/services/tests/servicestests/src/com/android/server/content/ObserverNodeTest.java b/services/tests/servicestests/src/com/android/server/content/ObserverNodeTest.java index 5b70c17ae28d1..07280bc881d81 100644 --- a/services/tests/servicestests/src/com/android/server/content/ObserverNodeTest.java +++ b/services/tests/servicestests/src/com/android/server/content/ObserverNodeTest.java @@ -62,7 +62,7 @@ public class ObserverNodeTest extends AndroidTestCase { ArrayList calls = new ArrayList(); for (int i = nums.length - 1; i >=0; --i) { - root.collectObserversLocked(uris[i], 0, null, false, myUserHandle, calls); + root.collectObserversLocked(uris[i], 0, null, false, 0, myUserHandle, calls); assertEquals(nums[i], calls.size()); calls.clear(); } @@ -92,7 +92,7 @@ public class ObserverNodeTest extends AndroidTestCase { ArrayList calls = new ArrayList(); for (int i = uris.length - 1; i >=0; --i) { - root.collectObserversLocked(uris[i], 0, null, false, myUserHandle, calls); + root.collectObserversLocked(uris[i], 0, null, false, 0, myUserHandle, calls); assertEquals(nums[i], calls.size()); calls.clear(); }