Clean up content observer code.

Improved the documentation a little bit.

Fixed a bug in ContentService wherein if a ContentObserver was
passed as an argument and its deliverSelfNotifications() method
returned true, then notifyChange would tell all observers that
the change was a self-change even though it was only a self-change
from the perspective of the provided observer.

Deprecated ContentObservable.notifyChange since it is never
used and in general it shouldn't be because we want the notification
to be posted to the handler.

Change-Id: Idde49eb40777e011a068f2adae8a32f779dfb923

6 files changed, 126 insertions, 95 deletions

diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java
index cc3219b..0debb84 100644
--- a/core/java/android/content/ContentResolver.java
+++ b/core/java/android/content/ContentResolver.java
@@ -1034,8 +1034,11 @@ public abstract class ContentResolver {
      * To register, call {@link #registerContentObserver(android.net.Uri , boolean, android.database.ContentObserver) registerContentObserver()}.
      * By default, CursorAdapter objects will get this notification.
      *
-     * @param uri
-     * @param observer The observer that originated the change, may be <code>null</null>
+     * @param uri The uri of the content that was changed.
+     * @param observer The observer that originated the change, may be <code>null</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.
      */
     public void notifyChange(Uri uri, ContentObserver observer) {
         notifyChange(uri, observer, true /* sync to network */);
@@ -1046,8 +1049,11 @@ public abstract class ContentResolver {
      * To register, call {@link #registerContentObserver(android.net.Uri , boolean, android.database.ContentObserver) registerContentObserver()}.
      * By default, CursorAdapter objects will get this notification.
      *
-     * @param uri
-     * @param observer The observer that originated the change, may be <code>null</null>
+     * @param uri The uri of the content that was changed.
+     * @param observer The observer that originated the change, may be <code>null</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 syncToNetwork If true, attempt to sync the change to the network.
      */
     public void notifyChange(Uri uri, ContentObserver observer, boolean syncToNetwork) {
diff --git a/core/java/android/content/ContentService.java b/core/java/android/content/ContentService.java
index 0e83dc0..9f8bec9 100644
--- a/core/java/android/content/ContentService.java
+++ b/core/java/android/content/ContentService.java
@@ -176,7 +176,7 @@ public final class ContentService extends IContentService.Stub {
             for (int i=0; i<numCalls; i++) {
                 ObserverCall oc = calls.get(i);
                 try {
-                    oc.mObserver.onChange(oc.mSelfNotify);
+                    oc.mObserver.onChange(oc.mSelfChange);
                     if (Log.isLoggable(TAG, Log.VERBOSE)) {
                         Log.v(TAG, "Notified " + oc.mObserver + " of " + "update at " + uri);
                     }
@@ -218,13 +218,12 @@ public final class ContentService extends IContentService.Stub {
     public static final class ObserverCall {
         final ObserverNode mNode;
         final IContentObserver mObserver;
-        final boolean mSelfNotify;
+        final boolean mSelfChange;
 
-        ObserverCall(ObserverNode node, IContentObserver observer,
-                boolean selfNotify) {
+        ObserverCall(ObserverNode node, IContentObserver observer, boolean selfChange) {
             mNode = node;
             mObserver = observer;
-            mSelfNotify = selfNotify;
+            mSelfChange = selfChange;
         }
     }
 
@@ -668,7 +667,7 @@ public final class ContentService extends IContentService.Stub {
         }
 
         private void collectMyObserversLocked(boolean leaf, IContentObserver observer,
-                boolean selfNotify, ArrayList<ObserverCall> calls) {
+                boolean observerWantsSelfNotifications, ArrayList<ObserverCall> calls) {
             int N = mObservers.size();
             IBinder observerBinder = observer == null ? null : observer.asBinder();
             for (int i = 0; i < N; i++) {
@@ -676,28 +675,29 @@ public final class ContentService extends IContentService.Stub {
 
                 // Don't notify the observer if it sent the notification and isn't interesed
                 // in self notifications
-                if (entry.observer.asBinder() == observerBinder && !selfNotify) {
+                boolean selfChange = (entry.observer.asBinder() == observerBinder);
+                if (selfChange && !observerWantsSelfNotifications) {
                     continue;
                 }
 
                 // Make sure the observer is interested in the notification
                 if (leaf || (!leaf && entry.notifyForDescendents)) {
-                    calls.add(new ObserverCall(this, entry.observer, selfNotify));
+                    calls.add(new ObserverCall(this, entry.observer, selfChange));
                 }
             }
         }
 
         public void collectObserversLocked(Uri uri, int index, IContentObserver observer,
-                boolean selfNotify, ArrayList<ObserverCall> calls) {
+                boolean observerWantsSelfNotifications, ArrayList<ObserverCall> calls) {
             String segment = null;
             int segmentCount = countUriSegments(uri);
             if (index >= segmentCount) {
                 // This is the leaf node, notify all observers
-                collectMyObserversLocked(true, observer, selfNotify, calls);
+                collectMyObserversLocked(true, observer, observerWantsSelfNotifications, calls);
             } else if (index < segmentCount){
                 segment = getUriSegment(uri, index);
                 // Notify any observers at this level who are interested in descendents
-                collectMyObserversLocked(false, observer, selfNotify, calls);
+                collectMyObserversLocked(false, observer, observerWantsSelfNotifications, calls);
             }
 
             int N = mChildren.size();
@@ -705,7 +705,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, selfNotify, calls);
+                    node.collectObserversLocked(uri, index + 1,
+                            observer, observerWantsSelfNotifications, calls);
                     if (segment != null) {
                         break;
                     }
diff --git a/core/java/android/database/ContentObservable.java b/core/java/android/database/ContentObservable.java
index 8d7b7c5..aece904 100644
--- a/core/java/android/database/ContentObservable.java
+++ b/core/java/android/database/ContentObservable.java
@@ -17,20 +17,28 @@
 package android.database;
 
 /**
- * A specialization of Observable for ContentObserver that provides methods for
- * invoking the various callback methods of ContentObserver.
+ * A specialization of {@link Observable} for {@link ContentObserver}
+ * that provides methods for sending notifications to a list of
+ * {@link ContentObserver} objects.
  */
 public class ContentObservable extends Observable<ContentObserver> {
-
+    // Even though the generic method defined in Observable would be perfectly
+    // fine on its own, we can't delete this overridden method because it would
+    // potentially break binary compatibility with existing applications.
     @Override
     public void registerObserver(ContentObserver observer) {
         super.registerObserver(observer);
     }
 
     /**
-     * invokes dispatchUpdate on each observer, unless the observer doesn't want
-     * self-notifications and the update is from a self-notification
-     * @param selfChange
+     * Invokes {@link ContentObserver#dispatchChange} on each observer.
+     *
+     * If <code>selfChange</code> is true, only delivers the notification
+     * to the observer if it has indicated that it wants to receive self-change
+     * notifications by implementing {@link ContentObserver#deliverSelfNotifications}
+     * to return true.
+     *
+     * @param selfChange True if this is a self-change notification.
      */
     public void dispatchChange(boolean selfChange) {
         synchronized(mObservers) {
@@ -43,9 +51,13 @@ public class ContentObservable extends Observable<ContentObserver> {
     }
 
     /**
-     * invokes onChange on each observer
-     * @param selfChange
+     * Invokes {@link ContentObserver#onChange} on each observer.
+     *
+     * @param selfChange True if this is a self-change notification.
+     *
+     * @deprecated Use {@link #dispatchChange} instead.
      */
+    @Deprecated
     public void notifyChange(boolean selfChange) {
         synchronized(mObservers) {
             for (ContentObserver observer : mObservers) {
diff --git a/core/java/android/database/ContentObserver.java b/core/java/android/database/ContentObserver.java
index 3b829a3..4c20262 100644
--- a/core/java/android/database/ContentObserver.java
+++ b/core/java/android/database/ContentObserver.java
@@ -19,62 +19,19 @@ package android.database;
 import android.os.Handler;
 
 /**
- * Receives call backs for changes to content. Must be implemented by objects which are added
- * to a {@link ContentObservable}.
+ * Receives call backs for changes to content.
+ * Must be implemented by objects which are added to a {@link ContentObservable}.
  */
 public abstract class ContentObserver {
+    private final Object mLock = new Object();
+    private Transport mTransport; // guarded by mLock
 
-    private Transport mTransport;
-
-    // Protects mTransport
-    private Object lock = new Object();
-
-    /* package */ Handler mHandler;
-
-    private final class NotificationRunnable implements Runnable {
-
-        private boolean mSelf;
-
-        public NotificationRunnable(boolean self) {
-            mSelf = self;
-        }
-
-        public void run() {
-            ContentObserver.this.onChange(mSelf);
-        }
-    }
-
-    private static final class Transport extends IContentObserver.Stub {
-        ContentObserver mContentObserver;
-
-        public Transport(ContentObserver contentObserver) {
-            mContentObserver = contentObserver;
-        }
-
-        public boolean deliverSelfNotifications() {
-            ContentObserver contentObserver = mContentObserver;
-            if (contentObserver != null) {
-                return contentObserver.deliverSelfNotifications();
-            }
-            return false;
-        }
-
-        public void onChange(boolean selfChange) {
-            ContentObserver contentObserver = mContentObserver;
-            if (contentObserver != null) {
-                contentObserver.dispatchChange(selfChange);
-            }
-        }
-
-        public void releaseContentObserver() {
-            mContentObserver = null;
-        }
-    }
+    Handler mHandler;
 
     /**
-     * onChange() will happen on the provider Handler.
+     * Creates a content observer.
      *
-     * @param handler The handler to run {@link #onChange} on.
+     * @param handler The handler to run {@link #onChange} on, or null if none.
      */
     public ContentObserver(Handler handler) {
         mHandler = handler;
@@ -86,7 +43,7 @@ public abstract class ContentObserver {
      * {@hide}
      */
     public IContentObserver getContentObserver() {
-        synchronized(lock) {
+        synchronized (mLock) {
             if (mTransport == null) {
                 mTransport = new Transport(this);
             }
@@ -101,8 +58,8 @@ public abstract class ContentObserver {
      * {@hide}
      */
     public IContentObserver releaseContentObserver() {
-        synchronized(lock) {
-            Transport oldTransport = mTransport;
+        synchronized (mLock) {
+            final Transport oldTransport = mTransport;
             if (oldTransport != null) {
                 oldTransport.releaseContentObserver();
                 mTransport = null;
@@ -112,22 +69,36 @@ public abstract class ContentObserver {
     }
 
     /**
-     * Returns true if this observer is interested in notifications for changes
-     * made through the cursor the observer is registered with.
+     * Returns true if this observer is interested receiving self-change notifications.
+     *
+     * Subclasses should override this method to indicate whether the observer
+     * is interested in receiving notifications for changes that it made to the
+     * content itself.
+     *
+     * @return True if self-change notifications should be delivered to the observer.
      */
     public boolean deliverSelfNotifications() {
         return false;
     }
 
     /**
-     * This method is called when a change occurs to the cursor that
-     * is being observed.
-     *  
-     * @param selfChange true if the update was caused by a call to <code>commit</code> on the
-     *  cursor that is being observed.
+     * This method is called when a content change occurs.
+     *
+     * @param selfChange True if this is a self-change notification.
      */
-    public void onChange(boolean selfChange) {}
+    public void onChange(boolean selfChange) {
+        // Do nothing.  Subclass should override.
+    }
 
+    /**
+     * Dispatches a change notification to the observer.
+     *
+     * If a {@link Handler} was supplied to the {@link ContentObserver} constructor,
+     * then a call to the {@link #onChange} method is posted to the handler's message queue.
+     * Otherwise, the {@link #onChange} method is invoked immediately on this thread.
+     *
+     * @param selfChange True if this is a self-change notification.
+     */
     public final void dispatchChange(boolean selfChange) {
         if (mHandler == null) {
             onChange(selfChange);
@@ -135,4 +106,37 @@ public abstract class ContentObserver {
             mHandler.post(new NotificationRunnable(selfChange));
         }
     }
+
+    private final class NotificationRunnable implements Runnable {
+        private final boolean mSelf;
+
+        public NotificationRunnable(boolean self) {
+            mSelf = self;
+        }
+
+        @Override
+        public void run() {
+            ContentObserver.this.onChange(mSelf);
+        }
+    }
+
+    private static final class Transport extends IContentObserver.Stub {
+        private ContentObserver mContentObserver;
+
+        public Transport(ContentObserver contentObserver) {
+            mContentObserver = contentObserver;
+        }
+
+        @Override
+        public void onChange(boolean selfChange) {
+            ContentObserver contentObserver = mContentObserver;
+            if (contentObserver != null) {
+                contentObserver.dispatchChange(selfChange);
+            }
+        }
+
+        public void releaseContentObserver() {
+            mContentObserver = null;
+        }
+    }
 }
diff --git a/core/java/android/database/DataSetObservable.java b/core/java/android/database/DataSetObservable.java
index 51c72c1..ca77a13 100644
--- a/core/java/android/database/DataSetObservable.java
+++ b/core/java/android/database/DataSetObservable.java
@@ -17,13 +17,15 @@
 package android.database;
 
 /**
- * A specialization of Observable for DataSetObserver that provides methods for
- * invoking the various callback methods of DataSetObserver.
+ * A specialization of {@link Observable} for {@link DataSetObserver}
+ * that provides methods for sending notifications to a list of
+ * {@link DataSetObserver} objects.
  */
 public class DataSetObservable extends Observable<DataSetObserver> {
     /**
-     * Invokes onChanged on each observer. Called when the data set being observed has
-     * changed, and which when read contains the new state of the data.
+     * Invokes {@link DataSetObserver#onChanged} on each observer.
+     * Called when the contents of the data set have changed.  The recipient
+     * will obtain the new contents the next time it queries the data set.
      */
     public void notifyChanged() {
         synchronized(mObservers) {
@@ -38,8 +40,9 @@ public class DataSetObservable extends Observable<DataSetObserver> {
     }
 
     /**
-     * Invokes onInvalidated on each observer. Called when the data set being monitored
-     * has changed such that it is no longer valid.
+     * Invokes {@link DataSetObserver#onInvalidated} on each observer.
+     * Called when the data set is no longer valid and cannot be queried again,
+     * such as when the data set has been closed.
      */
     public void notifyInvalidated() {
         synchronized (mObservers) {
diff --git a/core/java/android/database/Observable.java b/core/java/android/database/Observable.java
index b6fecab..aff32db 100644
--- a/core/java/android/database/Observable.java
+++ b/core/java/android/database/Observable.java
@@ -19,7 +19,12 @@ package android.database;
 import java.util.ArrayList;
 
 /**
- * Provides methods for (un)registering arbitrary observers in an ArrayList.
+ * Provides methods for registering or unregistering arbitrary observers in an {@link ArrayList}.
+ *
+ * This abstract class is intended to be subclassed and specialized to maintain
+ * a registry of observers of specific types and dispatch notifications to them.
+ *
+ * @param T The observer type.
  */
 public abstract class Observable<T> {
     /**
@@ -66,13 +71,13 @@ public abstract class Observable<T> {
             mObservers.remove(index);
         }
     }
-    
+
     /**
-     * Remove all registered observer
+     * Remove all registered observers.
      */
     public void unregisterAll() {
         synchronized(mObservers) {
             mObservers.clear();
-        }        
+        }
     }
 }