Fix issue #3301572: onCreateLoader must not be a member class: VolumeMetadataLoader

Also some various cleanup.

Change-Id: I8e8616a86c50c86817f7ec9bb02a5954c1ccd84f

5 files changed, 75 insertions, 48 deletions

diff --git a/core/java/android/app/LoaderManager.java b/core/java/android/app/LoaderManager.java
index 0e97abb..5f8c098 100644
--- a/core/java/android/app/LoaderManager.java
+++ b/core/java/android/app/LoaderManager.java
@@ -24,6 +24,7 @@ import android.util.SparseArray;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.lang.reflect.Modifier;
 
 /**
  * Interface associated with an {@link Activity} or {@link Fragment} for managing
@@ -217,8 +218,8 @@ class LoaderManagerImpl extends LoaderManager {
         LoaderManager.LoaderCallbacks<Object> mCallbacks;
         Loader<Object> mLoader;
         Object mData;
+        Object mDeliveredData;
         boolean mStarted;
-        boolean mNeedReset;
         boolean mRetaining;
         boolean mRetainingStarted;
         boolean mDestroyed;
@@ -244,14 +245,17 @@ class LoaderManagerImpl extends LoaderManager {
                 return;
             }
 
+            mStarted = true;
+            
             if (DEBUG) Log.v(TAG, "  Starting: " + this);
             if (mLoader == null && mCallbacks != null) {
                mLoader = mCallbacks.onCreateLoader(mId, mArgs);
             }
             if (mLoader != null) {
-                if (mLoader.getClass().isMemberClass()) {
+                if (mLoader.getClass().isMemberClass()
+                        && !Modifier.isStatic(mLoader.getClass().getModifiers())) {
                     throw new IllegalArgumentException(
-                            "Object returned from onCreateLoader must not be a member class: "
+                            "Object returned from onCreateLoader must not be a non-static inner member class: "
                             + mLoader);
                 }
                 if (!mListenerRegistered) {
@@ -259,7 +263,6 @@ class LoaderManagerImpl extends LoaderManager {
                     mListenerRegistered = true;
                 }
                 mLoader.startLoading();
-                mStarted = true;
             }
         }
         
@@ -312,8 +315,8 @@ class LoaderManagerImpl extends LoaderManager {
         void destroy() {
             if (DEBUG) Log.v(TAG, "  Destroying: " + this);
             mDestroyed = true;
-            boolean needReset = mNeedReset;
-            mNeedReset = false;
+            boolean needReset = mDeliveredData != null;
+            mDeliveredData = null;
             if (mCallbacks != null && mLoader != null && mData != null && needReset) {
                 if (DEBUG) Log.v(TAG, "  Reseting: " + this);
                 String lastBecause = null;
@@ -350,9 +353,11 @@ class LoaderManagerImpl extends LoaderManager {
             
             // Notify of the new data so the app can switch out the old data before
             // we try to destroy it.
-            mData = data;
-            if (mStarted) {
-                callOnLoadFinished(loader, data);
+            if (mData != data) {
+                mData = data;
+                if (mStarted) {
+                    callOnLoadFinished(loader, data);
+                }
             }
 
             //if (DEBUG) Log.v(TAG, "  onLoadFinished returned: " + this);
@@ -363,7 +368,7 @@ class LoaderManagerImpl extends LoaderManager {
             // clean it up.
             LoaderInfo info = mInactiveLoaders.get(mId);
             if (info != null && info != this) {
-                info.mNeedReset = false;
+                info.mDeliveredData = null;
                 info.destroy();
                 mInactiveLoaders.remove(mId);
             }
@@ -385,7 +390,7 @@ class LoaderManagerImpl extends LoaderManager {
                         mActivity.mFragments.mNoTransactionsBecause = lastBecause;
                     }
                 }
-                mNeedReset = true;
+                mDeliveredData = data;
             }
         }
         
@@ -411,11 +416,12 @@ class LoaderManagerImpl extends LoaderManager {
                 mLoader.dump(prefix + "  ", fd, writer, args);
             }
             writer.print(prefix); writer.print("mData="); writer.println(mData);
+            writer.print(prefix); writer.print("mDeliveredData="); writer.println(mDeliveredData);
             writer.print(prefix); writer.print("mStarted="); writer.print(mStarted);
                     writer.print(" mRetaining="); writer.print(mRetaining);
                     writer.print(" mDestroyed="); writer.println(mDestroyed);
-            writer.print(prefix); writer.print("mNeedReset="); writer.print(mNeedReset);
-                    writer.print(" mListenerRegistered="); writer.println(mListenerRegistered);
+            writer.print(prefix); writer.print("mListenerRegistered=");
+                    writer.println(mListenerRegistered);
         }
     }
     
@@ -479,7 +485,7 @@ class LoaderManagerImpl extends LoaderManager {
                     // yet destroyed the last inactive loader.  So just do
                     // that now.
                     if (DEBUG) Log.v(TAG, "  Removing last inactive loader: " + info);
-                    inactive.mNeedReset = false;
+                    inactive.mDeliveredData = null;
                     inactive.destroy();
                     mInactiveLoaders.put(id, info);
                 } else {
diff --git a/core/java/android/content/AsyncTaskLoader.java b/core/java/android/content/AsyncTaskLoader.java
index 30f8ffc..d6bc959 100644
--- a/core/java/android/content/AsyncTaskLoader.java
+++ b/core/java/android/content/AsyncTaskLoader.java
@@ -38,7 +38,7 @@ public abstract class AsyncTaskLoader<D> extends Loader<D> {
         /* Runs on the UI thread */
         @Override
         protected void onPostExecute(D data) {
-            AsyncTaskLoader.this.dispatchOnLoadComplete(data);
+            AsyncTaskLoader.this.dispatchOnLoadComplete(this, data);
         }
 
         @Override
@@ -55,6 +55,7 @@ public abstract class AsyncTaskLoader<D> extends Loader<D> {
 
     @Override
     protected void onForceLoad() {
+        super.onForceLoad();
         cancelLoad();
         mTask = new LoadTask();
         mTask.execute((Void[]) null);
@@ -85,9 +86,13 @@ public abstract class AsyncTaskLoader<D> extends Loader<D> {
     public void onCancelled(D data) {
     }
 
-    void dispatchOnLoadComplete(D data) {
-        mTask = null;
-        deliverResult(data);
+    void dispatchOnLoadComplete(LoadTask task, D data) {
+        if (mTask != task) {
+            onCancelled(data);
+        } else {
+            mTask = null;
+            deliverResult(data);
+        }
     }
 
     /**
diff --git a/core/java/android/content/CursorLoader.java b/core/java/android/content/CursorLoader.java
index 91dd23b..8ab0973 100644
--- a/core/java/android/content/CursorLoader.java
+++ b/core/java/android/content/CursorLoader.java
@@ -37,7 +37,6 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> {
     String mSortOrder;
 
     Cursor mCursor;
-    boolean mContentChanged;
 
     /* Runs on a worker thread */
     @Override
@@ -63,7 +62,7 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> {
     /* Runs on the UI thread */
     @Override
     public void deliverResult(Cursor cursor) {
-        if (mReset) {
+        if (isReset()) {
             // An async query came in while the loader is stopped
             if (cursor != null) {
                 cursor.close();
@@ -105,8 +104,7 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> {
         if (mCursor != null) {
             deliverResult(mCursor);
         }
-        if (mCursor == null || mContentChanged) {
-            mContentChanged = false;
+        if (takeContentChanged() || mCursor == null) {
             forceLoad();
         }
     }
@@ -121,18 +119,6 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> {
     }
 
     @Override
-    public void onContentChanged() {
-        if (!isStarted()) {
-            // This loader has been stopped, so we don't want to load
-            // new data right now...  but keep track of it changing to
-            // refresh later if we start again.
-            mContentChanged = true;
-            return;
-        }
-        super.onContentChanged();
-    }
-    
-    @Override
     public void onCancelled(Cursor cursor) {
         if (cursor != null && !cursor.isClosed()) {
             cursor.close();
@@ -141,10 +127,10 @@ public class CursorLoader extends AsyncTaskLoader<Cursor> {
 
     @Override
     protected void onReset() {
-        mReset = true;
-
+        super.onReset();
+        
         // Ensure the loader is stopped
-        stopLoading();
+        onStopLoading();
 
         if (mCursor != null && !mCursor.isClosed()) {
             mCursor.close();
diff --git a/core/java/android/content/Loader.java b/core/java/android/content/Loader.java
index f425b29..ef81fe4 100644
--- a/core/java/android/content/Loader.java
+++ b/core/java/android/content/Loader.java
@@ -39,6 +39,7 @@ public class Loader<D> {
     Context mContext;
     boolean mStarted = false;
     boolean mReset = true;
+    boolean mContentChanged = false;
 
     public final class ForceLoadContentObserver extends ContentObserver {
         public ForceLoadContentObserver() {
@@ -161,7 +162,7 @@ public class Loader<D> {
      *
      * <p>Must be called from the UI thread.
      */
-    public void startLoading() {
+    public final void startLoading() {
         mStarted = true;
         mReset = false;
         onStartLoading();
@@ -178,11 +179,12 @@ public class Loader<D> {
     /**
      * Force an asynchronous load. Unlike {@link #startLoading()} this will ignore a previously
      * loaded data set and load a new one.  This simply calls through to the
-     * implementation's {@link #onForceLoad()}.
+     * implementation's {@link #onForceLoad()}.  You generally should only call this
+     * when the loader is started -- that is, {@link #isStarted()} returns true.
      *
      * <p>Must be called from the UI thread.
      */
-    public void forceLoad() {
+    public final void forceLoad() {
         onForceLoad();
     }
 
@@ -194,6 +196,11 @@ public class Loader<D> {
 
     /**
      * Stops delivery of updates until the next time {@link #startLoading()} is called.
+     * Implementations should <em>not</em> invalidate their data at this point --
+     * clients are still free to use the last data the loader reported.  They will,
+     * however, typically stop reporting new data if the data changes; they can
+     * still monitor for changes, but must not report them to the client until and
+     * if {@link #startLoading()} is later called.
      *
      * <p>This updates the Loader's internal state so that
      * {@link #isStarted()} will return the correct
@@ -201,7 +208,7 @@ public class Loader<D> {
      *
      * <p>Must be called from the UI thread.
      */
-    public void stopLoading() {
+    public final void stopLoading() {
         mStarted = false;
         onStopLoading();
     }
@@ -226,10 +233,11 @@ public class Loader<D> {
      *
      * <p>Must be called from the UI thread.
      */
-    public void reset() {
+    public final void reset() {
         onReset();
         mReset = true;
         mStarted = false;
+        mContentChanged = false;
     }
 
     /**
@@ -241,13 +249,33 @@ public class Loader<D> {
     }
 
     /**
-     * Called when {@link ForceLoadContentObserver} detects a change.  Calls {@link #forceLoad()}
-     * by default.
+     * Take the current flag indicating whether the loader's content had
+     * changed while it was stopped.  If it had, true is returned and the
+     * flag is cleared.
+     */
+    public boolean takeContentChanged() {
+        boolean res = mContentChanged;
+        mContentChanged = false;
+        return res;
+    }
+    
+    /**
+     * Called when {@link ForceLoadContentObserver} detects a change.  The
+     * default implementation checks to see if the loader is currently started;
+     * if so, it simply calls {@link #forceLoad()}; otherwise, it sets a flag
+     * so that {@link #takeContentChanged()} returns true.
      *
-     * <p>Must be called from the UI thread
+     * <p>Must be called from the UI thread.
      */
     public void onContentChanged() {
-        forceLoad();
+        if (mStarted) {
+            forceLoad();
+        } else {
+            // This loader has been stopped, so we don't want to load
+            // new data right now...  but keep track of it changing to
+            // refresh later if we start again.
+            mContentChanged = true;
+        }
     }
 
     /**
@@ -283,6 +311,7 @@ public class Loader<D> {
         writer.print(prefix); writer.print("mId="); writer.print(mId);
                 writer.print(" mListener="); writer.println(mListener);
         writer.print(prefix); writer.print("mStarted="); writer.print(mStarted);
+                writer.print(" mContentChanged="); writer.print(mContentChanged);
                 writer.print(" mReset="); writer.println(mReset);
     }
 }
\ No newline at end of file
diff --git a/core/java/android/widget/SimpleCursorAdapter.java b/core/java/android/widget/SimpleCursorAdapter.java
index 497610c..3d2a252 100644
--- a/core/java/android/widget/SimpleCursorAdapter.java
+++ b/core/java/android/widget/SimpleCursorAdapter.java
@@ -337,10 +337,11 @@ public class SimpleCursorAdapter extends ResourceCursorAdapter {
     }
 
     @Override
-    public void changeCursor(Cursor c) {
-        super.changeCursor(c);
+    public Cursor swapCursor(Cursor c) {
+        Cursor res = super.swapCursor(c);
         // rescan columns in case cursor layout is different
         findColumns(mOriginalFrom);
+        return res;
     }
     
     /**