Clean up CursorWindow lifetime.
Bug: 5332296

Removed dead code in SQLiteCursor related to the use of a background
query thread.  This code could result in CursorWindows being modified
concurrently or used after free.  This code is broken, unused and
is just in the way.

Added comments to explain how CursorWindow ownership is
supposed to work for AbstractWindowedCursors.  (There are still cases
where cursor windows get dropped on the floor without being closed.
Those will be taken care of in a subsequent patch.)

Cleaned up SQLiteQuery.fillWindow to eliminate duplicate code and
remove bits that were only needed for background loading, like
returning -1.

Change-Id: I03e8e2e73ff0c11df76d63f57df4c5ada06ae1cb
diff --git a/core/java/android/database/AbstractCursor.java b/core/java/android/database/AbstractCursor.java
index 5fe42db..3f23b89 100644
--- a/core/java/android/database/AbstractCursor.java
+++ b/core/java/android/database/AbstractCursor.java
@@ -64,7 +64,10 @@
     /* Methods that may optionally be implemented by subclasses */
 
     /**
-     * returns a pre-filled window, return NULL if no such window
+     * If the cursor is backed by a {@link CursorWindow}, returns a pre-filled
+     * window with the contents of the cursor, otherwise null.
+     *
+     * @return The pre-filled window that backs this cursor, or null if none.
      */
     public CursorWindow getWindow() {
         return null;
diff --git a/core/java/android/database/AbstractWindowedCursor.java b/core/java/android/database/AbstractWindowedCursor.java
index 3d95769..bfc8123 100644
--- a/core/java/android/database/AbstractWindowedCursor.java
+++ b/core/java/android/database/AbstractWindowedCursor.java
@@ -18,8 +18,22 @@
 
 /**
  * A base class for Cursors that store their data in {@link CursorWindow}s.
+ * <p>
+ * Subclasses are responsible for filling the cursor window with data during
+ * {@link #onMove(int, int)}, allocating a new cursor window if necessary.
+ * During {@link #requery()}, the existing cursor window should be cleared and
+ * filled with new data.
+ * </p><p>
+ * If the contents of the cursor change or become invalid, the old window must be closed
+ * (because it is owned by the cursor) and set to null.
+ * </p>
  */
 public abstract class AbstractWindowedCursor extends AbstractCursor {
+    /**
+     * The cursor window owned by this cursor.
+     */
+    protected CursorWindow mWindow;
+
     @Override
     public byte[] getBlob(int columnIndex) {
         checkPosition();
@@ -126,25 +140,44 @@
     public CursorWindow getWindow() {
         return mWindow;
     }
-    
+
     /**
-     * Set a new cursor window to cursor, usually set a remote cursor window
-     * @param window cursor window
+     * Sets a new cursor window for the cursor to use.
+     * <p>
+     * The cursor takes ownership of the provided cursor window; the cursor window
+     * will be closed when the cursor is closed or when the cursor adopts a new
+     * cursor window.
+     * </p><p>
+     * If the cursor previously had a cursor window, then it is closed when the
+     * new cursor window is assigned.
+     * </p>
+     *
+     * @param window The new cursor window, typically a remote cursor window.
      */
     public void setWindow(CursorWindow window) {
-        if (mWindow != null) {
-            mWindow.close();
+        if (window != mWindow) {
+            closeWindow();
+            mWindow = window;
         }
-        mWindow = window;
     }
-    
+
+    /**
+     * Returns true if the cursor has an associated cursor window.
+     *
+     * @return True if the cursor has an associated cursor window.
+     */
     public boolean hasWindow() {
         return mWindow != null;
     }
 
     /**
-     * This needs be updated in {@link #onMove} by subclasses, and
-     * needs to be set to NULL when the contents of the cursor change.
+     * Closes the cursor window and sets {@link #mWindow} to null.
+     * @hide
      */
-    protected CursorWindow mWindow;
+    protected void closeWindow() {
+        if (mWindow != null) {
+            mWindow.close();
+            mWindow = null;
+        }
+    }
 }
diff --git a/core/java/android/database/BulkCursorToCursorAdaptor.java b/core/java/android/database/BulkCursorToCursorAdaptor.java
index 16becf5..9c1b26d 100644
--- a/core/java/android/database/BulkCursorToCursorAdaptor.java
+++ b/core/java/android/database/BulkCursorToCursorAdaptor.java
@@ -154,10 +154,7 @@
                     false /* the window will be accessed across processes */));
             if (mCount != -1) {
                 mPos = -1;
-                if (mWindow != null) {
-                    mWindow.close();
-                    mWindow = null;
-                }
+                closeWindow();
 
                 // super.requery() will call onChanged. Do it here instead of relying on the
                 // observer from the far side so that observers can see a correct value for mCount
diff --git a/core/java/android/database/CursorWindow.java b/core/java/android/database/CursorWindow.java
index 183662f..2e3ef28 100644
--- a/core/java/android/database/CursorWindow.java
+++ b/core/java/android/database/CursorWindow.java
@@ -105,8 +105,12 @@
     }
 
     @Override
-    protected void finalize() {
-        dispose();
+    protected void finalize() throws Throwable {
+        try {
+            dispose();
+        } finally {
+            super.finalize();
+        }
     }
 
     private void dispose() {
@@ -145,10 +149,12 @@
 
     /**
      * Gets the start position of this cursor window.
-     * The start position is the index of the first row that this window contains
+     * <p>
+     * The start position is the zero-based index of the first row that this window contains
      * relative to the entire result set of the {@link Cursor}.
+     * </p>
      *
-     * @return The start position.
+     * @return The zero-based start position.
      */
     public int getStartPosition() {
         return mStartPos;
@@ -156,10 +162,12 @@
 
     /**
      * Sets the start position of this cursor window.
-     * The start position is the index of the first row that this window contains
+     * <p>
+     * The start position is the zero-based index of the first row that this window contains
      * relative to the entire result set of the {@link Cursor}.
+     * </p>
      *
-     * @param pos The new start position.
+     * @param pos The new zero-based start position.
      */
     public void setStartPosition(int pos) {
         mStartPos = pos;
diff --git a/core/java/android/database/CursorWrapper.java b/core/java/android/database/CursorWrapper.java
index 3c3bd43..320733e 100644
--- a/core/java/android/database/CursorWrapper.java
+++ b/core/java/android/database/CursorWrapper.java
@@ -33,7 +33,9 @@
     }
 
     /**
-     * @return the wrapped cursor
+     * Gets the underlying cursor that is wrapped by this instance.
+     *
+     * @return The wrapped cursor.
      */
     public Cursor getWrappedCursor() {
         return mCursor;
diff --git a/core/java/android/database/sqlite/SQLiteCursor.java b/core/java/android/database/sqlite/SQLiteCursor.java
index ea9346d..81fe824 100644
--- a/core/java/android/database/sqlite/SQLiteCursor.java
+++ b/core/java/android/database/sqlite/SQLiteCursor.java
@@ -18,16 +18,11 @@
 
 import android.database.AbstractWindowedCursor;
 import android.database.CursorWindow;
-import android.database.DataSetObserver;
-import android.os.Handler;
-import android.os.Message;
-import android.os.Process;
 import android.os.StrictMode;
 import android.util.Log;
 
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * A Cursor implementation that exposes results from a query on a
@@ -60,140 +55,8 @@
 
     /** Used to find out where a cursor was allocated in case it never got released. */
     private final Throwable mStackTrace;
-    
-    /** 
-     *  mMaxRead is the max items that each cursor window reads 
-     *  default to a very high value
-     */
-    private int mMaxRead = Integer.MAX_VALUE;
-    private int mInitialRead = Integer.MAX_VALUE;
-    private int mCursorState = 0;
-    private ReentrantLock mLock = null;
-    private boolean mPendingData = false;
 
     /**
-     *  support for a cursor variant that doesn't always read all results
-     *  initialRead is the initial number of items that cursor window reads 
-     *  if query contains more than this number of items, a thread will be
-     *  created and handle the left over items so that caller can show 
-     *  results as soon as possible 
-     * @param initialRead initial number of items that cursor read
-     * @param maxRead leftover items read at maxRead items per time
-     * @hide
-     */
-    public void setLoadStyle(int initialRead, int maxRead) {
-        mMaxRead = maxRead;
-        mInitialRead = initialRead;
-        mLock = new ReentrantLock(true);
-    }
-    
-    private void queryThreadLock() {
-        if (mLock != null) {
-            mLock.lock();            
-        }
-    }
-    
-    private void queryThreadUnlock() {
-        if (mLock != null) {
-            mLock.unlock();            
-        }
-    }
-    
-    
-    /**
-     * @hide
-     */
-    final private class QueryThread implements Runnable {
-        private final int mThreadState;
-        QueryThread(int version) {
-            mThreadState = version;
-        }
-        private void sendMessage() {
-            if (mNotificationHandler != null) {
-                mNotificationHandler.sendEmptyMessage(1);
-                mPendingData = false;
-            } else {
-                mPendingData = true;
-            }
-            
-        }
-        public void run() {
-             // use cached mWindow, to avoid get null mWindow
-            CursorWindow cw = mWindow;
-            Process.setThreadPriority(Process.myTid(), Process.THREAD_PRIORITY_BACKGROUND);
-            // the cursor's state doesn't change
-            while (true) {
-                mLock.lock();
-                try {
-                    if (mCursorState != mThreadState) {
-                        break;
-                    }
-
-                    int count = getQuery().fillWindow(cw, mMaxRead, mCount);
-                    // return -1 means there is still more data to be retrieved from the resultset
-                    if (count != 0) {
-                        if (count == NO_COUNT){
-                            mCount += mMaxRead;
-                            if (Log.isLoggable(TAG, Log.DEBUG)) {
-                                Log.d(TAG, "received -1 from native_fill_window. read " +
-                                        mCount + " rows so far");
-                            }
-                            sendMessage();
-                        } else {                                
-                            mCount += count;
-                            if (Log.isLoggable(TAG, Log.DEBUG)) {
-                                Log.d(TAG, "received all data from native_fill_window. read " +
-                                        mCount + " rows.");
-                            }
-                            sendMessage();
-                            break;
-                        }
-                    } else {
-                        break;
-                    }
-                } catch (Exception e) {
-                    // end the tread when the cursor is close
-                    break;
-                } finally {
-                    mLock.unlock();
-                }
-            }
-        }        
-    }
-    
-    /**
-     * @hide
-     */   
-    protected class MainThreadNotificationHandler extends Handler {
-        public void handleMessage(Message msg) {
-            notifyDataSetChange();
-        }
-    }
-    
-    /**
-     * @hide
-     */
-    protected MainThreadNotificationHandler mNotificationHandler;    
-    
-    public void registerDataSetObserver(DataSetObserver observer) {
-        super.registerDataSetObserver(observer);
-        if ((Integer.MAX_VALUE != mMaxRead || Integer.MAX_VALUE != mInitialRead) && 
-                mNotificationHandler == null) {
-            queryThreadLock();
-            try {
-                mNotificationHandler = new MainThreadNotificationHandler();
-                if (mPendingData) {
-                    notifyDataSetChange();
-                    mPendingData = false;
-                }
-            } finally {
-                queryThreadUnlock();
-            }
-        }
-        
-    }
-    
-    /**
      * Execute a query and provide access to its result set through a Cursor
      * interface. For a query such as: {@code SELECT name, birth, phone FROM
      * myTable WHERE ... LIMIT 1,20 ORDER BY...} the column names (name, birth,
@@ -293,36 +156,23 @@
         return mCount;
     }
 
-    private void fillWindow (int startPos) {
+    private void fillWindow(int startPos) {
         if (mWindow == null) {
             // If there isn't a window set already it will only be accessed locally
             mWindow = new CursorWindow(true /* the window is local only */);
         } else {
-            mCursorState++;
-                queryThreadLock();
-                try {
-                    mWindow.clear();
-                } finally {
-                    queryThreadUnlock();
-                }
+            mWindow.clear();
         }
         mWindow.setStartPosition(startPos);
-        int count = getQuery().fillWindow(mWindow, mInitialRead, 0);
-        // return -1 means there is still more data to be retrieved from the resultset
-        if (count == NO_COUNT){
-            mCount = startPos + mInitialRead;
-            if (Log.isLoggable(TAG, Log.DEBUG)) {
-                Log.d(TAG, "received -1 from native_fill_window. read " + mCount + " rows so far");
-            }
-            Thread t = new Thread(new QueryThread(mCursorState), "query thread");
-            t.start();
-        } else if (startPos == 0) { // native_fill_window returns count(*) only for startPos = 0
+        int count = getQuery().fillWindow(mWindow);
+        if (startPos == 0) { // fillWindow returns count(*) only for startPos = 0
             if (Log.isLoggable(TAG, Log.DEBUG)) {
                 Log.d(TAG, "received count(*) from native_fill_window: " + count);
             }
             mCount = count;
         } else if (mCount <= 0) {
-            throw new IllegalStateException("count should never be non-zero negative number");
+            throw new IllegalStateException("Row count should never be zero or negative "
+                    + "when the start position is non-zero");
         }
     }
 
@@ -366,11 +216,7 @@
 
     private void deactivateCommon() {
         if (false) Log.v(TAG, "<<< Releasing cursor " + this);
-        mCursorState = 0;
-        if (mWindow != null) {
-            mWindow.close();
-            mWindow = null;
-        }
+        closeWindow();
         if (false) Log.v("DatabaseWindow", "closing window in release()");
     }
 
@@ -439,16 +285,12 @@
             // This one will recreate the temp table, and get its count
             mDriver.cursorRequeried(this);
             mCount = NO_COUNT;
-            mCursorState++;
-            queryThreadLock();
             try {
                 mQuery.requery();
             } catch (IllegalStateException e) {
                 // for backwards compatibility, just return false
                 Log.w(TAG, "requery() failed " + e.getMessage(), e);
                 return false;
-            } finally {
-                queryThreadUnlock();
             }
         }
 
@@ -472,18 +314,9 @@
     }
 
     @Override
-    public void setWindow(CursorWindow window) {        
-        if (mWindow != null) {
-            mCursorState++;
-            queryThreadLock();
-            try {
-                mWindow.close();
-            } finally {
-                queryThreadUnlock();
-            }
-            mCount = NO_COUNT;
-        }
-        mWindow = window;
+    public void setWindow(CursorWindow window) {
+        super.setWindow(window);
+        mCount = NO_COUNT;
     }
 
     /**
@@ -521,11 +354,4 @@
             super.finalize();
         }
     }
-
-    /**
-     * this is only for testing purposes.
-     */
-    /* package */ int getMCount() {
-        return mCount;
-    }
 }
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index 93a6ad3..896a750 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -1593,32 +1593,6 @@
     }
 
     /**
-     * Runs the provided SQL and returns a cursor over the result set.
-     * The cursor will read an initial set of rows and the return to the caller.
-     * It will continue to read in batches and send data changed notifications
-     * when the later batches are ready.
-     * @param sql the SQL query. The SQL string must not be ; terminated
-     * @param selectionArgs You may include ?s in where clause in the query,
-     *     which will be replaced by the values from selectionArgs. The
-     *     values will be bound as Strings.
-     * @param initialRead set the initial count of items to read from the cursor
-     * @param maxRead set the count of items to read on each iteration after the first
-     * @return A {@link Cursor} object, which is positioned before the first entry. Note that
-     * {@link Cursor}s are not synchronized, see the documentation for more details.
-     *
-     * This work is incomplete and not fully tested or reviewed, so currently
-     * hidden.
-     * @hide
-     */
-    public Cursor rawQuery(String sql, String[] selectionArgs,
-            int initialRead, int maxRead) {
-        SQLiteCursor c = (SQLiteCursor)rawQueryWithFactory(
-                null, sql, selectionArgs, null);
-        c.setLoadStyle(initialRead, maxRead);
-        return c;
-    }
-
-    /**
      * Convenience method for inserting a row into the database.
      *
      * @param table the table to insert the row into
diff --git a/core/java/android/database/sqlite/SQLiteQuery.java b/core/java/android/database/sqlite/SQLiteQuery.java
index 06a41b2..7db0914 100644
--- a/core/java/android/database/sqlite/SQLiteQuery.java
+++ b/core/java/android/database/sqlite/SQLiteQuery.java
@@ -30,6 +30,11 @@
 public class SQLiteQuery extends SQLiteProgram {
     private static final String TAG = "SQLiteQuery";
 
+    private static native int nativeFillWindow(int databasePtr, int statementPtr, int windowPtr,
+            int startPos, int offsetParam);
+    private static native int nativeColumnCount(int statementPtr);
+    private static native String nativeColumnName(int statementPtr, int columnIndex);
+
     /** The index of the unbound OFFSET parameter */
     private int mOffsetIndex = 0;
 
@@ -68,19 +73,15 @@
      * @param window The window to fill into
      * @return number of total rows in the query
      */
-    /* package */ int fillWindow(CursorWindow window,
-            int maxRead, int lastPos) {
+    /* package */ int fillWindow(CursorWindow window) {
         mDatabase.lock(mSql);
         long timeStart = SystemClock.uptimeMillis();
         try {
             acquireReference();
             try {
                 window.acquireReference();
-                // if the start pos is not equal to 0, then most likely window is
-                // too small for the data set, loading by another thread
-                // is not safe in this situation. the native code will ignore maxRead
-                int numRows = native_fill_window(window.mWindowPtr, window.getStartPosition(),
-                        mOffsetIndex, maxRead, lastPos);
+                int numRows = nativeFillWindow(nHandle, nStatement, window.mWindowPtr,
+                        window.getStartPosition(), mOffsetIndex);
                 mDatabase.logTimeStat(mSql, timeStart);
                 return numRows;
             } catch (IllegalStateException e){
@@ -111,7 +112,7 @@
     /* package */ int columnCountLocked() {
         acquireReference();
         try {
-            return native_column_count();
+            return nativeColumnCount(nStatement);
         } finally {
             releaseReference();
         }
@@ -127,17 +128,17 @@
     /* package */ String columnNameLocked(int columnIndex) {
         acquireReference();
         try {
-            return native_column_name(columnIndex);
+            return nativeColumnName(nStatement, columnIndex);
         } finally {
             releaseReference();
         }
     }
-    
+
     @Override
     public String toString() {
         return "SQLiteQuery: " + mSql;
     }
-    
+
     @Override
     public void close() {
         super.close();
@@ -153,11 +154,4 @@
         }
         compileAndbindAllArgs();
     }
-
-    private final native int native_fill_window(int windowPtr,
-            int startPos, int offsetParam, int maxRead, int lastPos);
-
-    private final native int native_column_count();
-
-    private final native String native_column_name(int columnIndex);
 }
diff --git a/core/jni/android_database_SQLiteQuery.cpp b/core/jni/android_database_SQLiteQuery.cpp
index fe62256..022a64c 100644
--- a/core/jni/android_database_SQLiteQuery.cpp
+++ b/core/jni/android_database_SQLiteQuery.cpp
@@ -35,99 +35,20 @@
 
 namespace android {
 
-static jfieldID gHandleField;
-static jfieldID gStatementField;
-
-
-#define GET_STATEMENT(env, object) \
-        (sqlite3_stmt *)env->GetIntField(object, gStatementField)
-#define GET_HANDLE(env, object) \
-        (sqlite3 *)env->GetIntField(object, gHandleField)
-
-static int skip_rows(sqlite3_stmt *statement, int maxRows) {
-    int retryCount = 0;
-    for (int i = 0; i < maxRows; i++) {
-        int err = sqlite3_step(statement);
-        if (err == SQLITE_ROW){
-            // do nothing
-        } else if (err == SQLITE_DONE) {
-            return i;
-        } else if (err == SQLITE_LOCKED || err == SQLITE_BUSY) {
-            // The table is locked, retry
-            LOG_WINDOW("Database locked, retrying");
-           if (retryCount > 50) {
-                LOGE("Bailing on database busy rety");
-                break;
-            }
-            // Sleep to give the thread holding the lock a chance to finish
-            usleep(1000);
-            retryCount++;
-            continue;
-        } else {
-            return -1;
-        }
-    }
-    LOG_WINDOW("skip_rows row %d", maxRows);
-    return maxRows;
-}
-
-static int finish_program_and_get_row_count(sqlite3_stmt *statement) {
-    int numRows = 0;
-    int retryCount = 0;
-    while (true) {
-        int err = sqlite3_step(statement);
-        if (err == SQLITE_ROW){
-            numRows++;
-        } else if (err == SQLITE_LOCKED || err == SQLITE_BUSY) {
-            // The table is locked, retry
-            LOG_WINDOW("Database locked, retrying");
-            if (retryCount > 50) {
-                LOGE("Bailing on database busy rety");
-                break;
-            }
-            // Sleep to give the thread holding the lock a chance to finish
-            usleep(1000);
-            retryCount++;
-            continue;
-        } else {
-            // no need to throw exception
-            break;
-        }
-    }
-    sqlite3_reset(statement);
-    LOG_WINDOW("finish_program_and_get_row_count row %d", numRows);
-    return numRows;
-}
-
-static jint native_fill_window(JNIEnv* env, jobject object, jint windowPtr,
-                               jint startPos, jint offsetParam, jint maxRead, jint lastPos)
-{
-    int err;
-    sqlite3_stmt * statement = GET_STATEMENT(env, object);
-    int numRows = lastPos;
-    maxRead += lastPos;
-    int numColumns;
-    int retryCount;
-    int boundParams;
-    CursorWindow * window;
-    bool gotAllRows = true;
-    bool gotException = false;
-    
-    if (statement == NULL) {
-        LOGE("Invalid statement in fillWindow()");
-        jniThrowException(env, "java/lang/IllegalStateException",
-                          "Attempting to access a deactivated, closed, or empty cursor");
-        return 0;
-    }
+static jint nativeFillWindow(JNIEnv* env, jclass clazz, jint databasePtr,
+        jint statementPtr, jint windowPtr, jint startPos, jint offsetParam) {
+    sqlite3* database = reinterpret_cast<sqlite3*>(databasePtr);
+    sqlite3_stmt* statement = reinterpret_cast<sqlite3_stmt*>(statementPtr);
+    CursorWindow* window = reinterpret_cast<CursorWindow*>(windowPtr);
 
     // Only do the binding if there is a valid offsetParam. If no binding needs to be done
-    // offsetParam will be set to 0, an invliad value.
-    if(offsetParam > 0) {
+    // offsetParam will be set to 0, an invalid value.
+    if (offsetParam > 0) {
         // Bind the offset parameter, telling the program which row to start with
-        err = sqlite3_bind_int(statement, offsetParam, startPos);
+        int err = sqlite3_bind_int(statement, offsetParam, startPos);
         if (err != SQLITE_OK) {
             LOGE("Unable to bind offset position, offsetParam = %d", offsetParam);
-            throw_sqlite3_exception(env, GET_HANDLE(env, object));
+            throw_sqlite3_exception(env, database);
             return 0;
         }
         LOG_WINDOW("Bound to startPos %d", startPos);
@@ -135,136 +56,123 @@
         LOG_WINDOW("Not binding to startPos %d", startPos);
     }
 
-    // Get the native window
-    window = reinterpret_cast<CursorWindow*>(windowPtr);
-    if (!window) {
-        LOGE("Invalid CursorWindow");
-        jniThrowException(env, "java/lang/IllegalArgumentException",
-                          "Bad CursorWindow");
-        return 0;
-    }
-    LOG_WINDOW("Window: numRows = %d, size = %d, freeSpace = %d", window->getNumRows(), window->size(), window->freeSpace());
+    // We assume numRows is initially 0.
+    LOG_WINDOW("Window: numRows = %d, size = %d, freeSpace = %d",
+            window->getNumRows(), window->size(), window->freeSpace());
 
-    numColumns = sqlite3_column_count(statement);
+    int numColumns = sqlite3_column_count(statement);
     if (!window->setNumColumns(numColumns)) {
         LOGE("Failed to change column count from %d to %d", window->getNumColumns(), numColumns);
         jniThrowException(env, "java/lang/IllegalStateException", "numColumns mismatch");
         return 0;
     }
 
-    retryCount = 0;
-    if (startPos > 0) {
-        int num = skip_rows(statement, startPos);
-        if (num < 0) {
-            throw_sqlite3_exception(env, GET_HANDLE(env, object));
-            return 0;
-        } else if (num < startPos) {
-            LOGE("startPos %d > actual rows %d", startPos, num);
-            return num;
-        }
-    }
-    
-    while(startPos != 0 || numRows < maxRead) {
-        err = sqlite3_step(statement);
+    int retryCount = 0;
+    int totalRows = 0;
+    int addedRows = 0;
+    bool windowFull = false;
+    bool gotException = false;
+    const bool countAllRows = (startPos == 0); // when startPos is 0, we count all rows
+    while (!gotException && (!windowFull || countAllRows)) {
+        int err = sqlite3_step(statement);
         if (err == SQLITE_ROW) {
-            LOG_WINDOW("\nStepped statement %p to row %d", statement, startPos + numRows);
+            LOG_WINDOW("Stepped statement %p to row %d", statement, totalRows);
             retryCount = 0;
+            totalRows += 1;
 
-            // Allocate a new field directory for the row. This pointer is not reused
-            // since it mey be possible for it to be relocated on a call to alloc() when
-            // the field data is being allocated.
-            {
-                field_slot_t * fieldDir = window->allocRow();
-                if (!fieldDir) {
-                    LOG_WINDOW("Failed allocating fieldDir at startPos %d row %d", startPos, numRows);
-                    gotAllRows = false;
-                    goto return_count;
-                }
+            // Skip the row if the window is full or we haven't reached the start position yet.
+            if (startPos >= totalRows || windowFull) {
+                continue;
             }
 
-            // Pack the row into the window
-            int i;
-            for (i = 0; i < numColumns; i++) {
+            // Allocate a new field directory for the row. This pointer is not reused
+            // since it may be possible for it to be relocated on a call to alloc() when
+            // the field data is being allocated.
+            field_slot_t* fieldDir = window->allocRow();
+            if (!fieldDir) {
+                LOG_WINDOW("Failed allocating fieldDir at startPos %d row %d",
+                        startPos, addedRows);
+                windowFull = true;
+                continue;
+            }
+
+            // Pack the row into the window.
+            for (int i = 0; i < numColumns; i++) {
                 int type = sqlite3_column_type(statement, i);
                 if (type == SQLITE_TEXT) {
                     // TEXT data
 #if WINDOW_STORAGE_UTF8
-                    uint8_t const * text = (uint8_t const *)sqlite3_column_text(statement, i);
+                    const uint8_t* text = reinterpret_cast<const uint8_t*>(
+                            sqlite3_column_text(statement, i));
                     // SQLite does not include the NULL terminator in size, but does
                     // ensure all strings are NULL terminated, so increase size by
                     // one to make sure we store the terminator.
                     size_t size = sqlite3_column_bytes(statement, i) + 1;
 #else
-                    uint8_t const * text = (uint8_t const *)sqlite3_column_text16(statement, i);
+                    const uint8_t* text = reinterpret_cast<const uint8_t*>(
+                            sqlite3_column_text16(statement, i));
                     size_t size = sqlite3_column_bytes16(statement, i);
 #endif
                     int offset = window->alloc(size);
                     if (!offset) {
-                        window->freeLastRow();
                         LOG_WINDOW("Failed allocating %u bytes for text/blob at %d,%d", size,
-                                   startPos + numRows, i);
-                        gotAllRows = false;
-                        goto return_count;
+                                   startPos + addedRows, i);
+                        windowFull = true;
+                        break;
                     }
-
                     window->copyIn(offset, text, size);
 
-                    // This must be updated after the call to alloc(), since that
-                    // may move the field around in the window
-                    field_slot_t * fieldSlot = window->getFieldSlot(numRows, i);
+                    field_slot_t* fieldSlot = window->getFieldSlot(addedRows, i);
                     fieldSlot->type = FIELD_TYPE_STRING;
                     fieldSlot->data.buffer.offset = offset;
                     fieldSlot->data.buffer.size = size;
-
-                    LOG_WINDOW("%d,%d is TEXT with %u bytes", startPos + numRows, i, size);
+                    LOG_WINDOW("%d,%d is TEXT with %u bytes", startPos + addedRows, i, size);
                 } else if (type == SQLITE_INTEGER) {
                     // INTEGER data
                     int64_t value = sqlite3_column_int64(statement, i);
-                    if (!window->putLong(numRows, i, value)) {
-                        window->freeLastRow();
+                    if (!window->putLong(addedRows, i, value)) {
                         LOG_WINDOW("Failed allocating space for a long in column %d", i);
-                        gotAllRows = false;
-                        goto return_count;
+                        windowFull = true;
+                        break;
                     }
-                    LOG_WINDOW("%d,%d is INTEGER 0x%016llx", startPos + numRows, i, value);
+                    LOG_WINDOW("%d,%d is INTEGER 0x%016llx", startPos + addedRows, i, value);
                 } else if (type == SQLITE_FLOAT) {
                     // FLOAT data
                     double value = sqlite3_column_double(statement, i);
-                    if (!window->putDouble(numRows, i, value)) {
-                        window->freeLastRow();
+                    if (!window->putDouble(addedRows, i, value)) {
                         LOG_WINDOW("Failed allocating space for a double in column %d", i);
-                        gotAllRows = false;
-                        goto return_count;
+                        windowFull = true;
+                        break;
                     }
-                    LOG_WINDOW("%d,%d is FLOAT %lf", startPos + numRows, i, value);
+                    LOG_WINDOW("%d,%d is FLOAT %lf", startPos + addedRows, i, value);
                 } else if (type == SQLITE_BLOB) {
                     // BLOB data
                     uint8_t const * blob = (uint8_t const *)sqlite3_column_blob(statement, i);
                     size_t size = sqlite3_column_bytes16(statement, i);
                     int offset = window->alloc(size);
                     if (!offset) {
-                        window->freeLastRow();
                         LOG_WINDOW("Failed allocating %u bytes for blob at %d,%d", size,
-                                   startPos + numRows, i);
-                        gotAllRows = false;
-                        goto return_count;
+                                startPos + addedRows, i);
+                        windowFull = true;
+                        break;
                     }
-
                     window->copyIn(offset, blob, size);
 
-                    // This must be updated after the call to alloc(), since that
-                    // may move the field around in the window
-                    field_slot_t * fieldSlot = window->getFieldSlot(numRows, i);
+                    field_slot_t* fieldSlot = window->getFieldSlot(addedRows, i);
                     fieldSlot->type = FIELD_TYPE_BLOB;
                     fieldSlot->data.buffer.offset = offset;
                     fieldSlot->data.buffer.size = size;
-
-                    LOG_WINDOW("%d,%d is Blob with %u bytes @ %d", startPos + numRows, i, size, offset);
+                    LOG_WINDOW("%d,%d is Blob with %u bytes @ %d",
+                            startPos + addedRows, i, size, offset);
                 } else if (type == SQLITE_NULL) {
                     // NULL field
-                    window->putNull(numRows, i);
+                    if (!window->putNull(addedRows, i)) {
+                        LOG_WINDOW("Failed allocating space for a null in column %d", i);
+                        windowFull = true;
+                        break;
+                    }
 
-                    LOG_WINDOW("%d,%d is NULL", startPos + numRows, i);
+                    LOG_WINDOW("%d,%d is NULL", startPos + addedRows, i);
                 } else {
                     // Unknown data
                     LOGE("Unknown column type when filling database window");
@@ -274,14 +182,12 @@
                 }
             }
 
-            if (i < numColumns) {
-                // Not all the fields fit in the window
-                // Unknown data error happened
-                break;
+            // Update the final row tally.
+            if (windowFull || gotException) {
+                window->freeLastRow();
+            } else {
+                addedRows += 1;
             }
-
-            // Mark the row as complete in the window
-            numRows++;
         } else if (err == SQLITE_DONE) {
             // All rows processed, bail
             LOG_WINDOW("Processed all rows");
@@ -290,63 +196,41 @@
             // The table is locked, retry
             LOG_WINDOW("Database locked, retrying");
             if (retryCount > 50) {
-                LOGE("Bailing on database busy rety");
-                throw_sqlite3_exception(env, GET_HANDLE(env, object), "retrycount exceeded");
+                LOGE("Bailing on database busy retry");
+                throw_sqlite3_exception(env, database, "retrycount exceeded");
                 gotException = true;
-                break;
+            } else {
+                // Sleep to give the thread holding the lock a chance to finish
+                usleep(1000);
+                retryCount++;
             }
-
-            // Sleep to give the thread holding the lock a chance to finish
-            usleep(1000);
-
-            retryCount++;
-            continue;
         } else {
-            throw_sqlite3_exception(env, GET_HANDLE(env, object));
+            throw_sqlite3_exception(env, database);
             gotException = true;
-            break;
         }
     }
 
-    LOG_WINDOW("Resetting statement %p after fetching %d rows in %d bytes\n\n\n\n", statement,
-            numRows, window->size() - window->freeSpace());
-    LOG_WINDOW("Filled window with %d rows in %d bytes", numRows,
-            window->size() - window->freeSpace());
-    if (err == SQLITE_ROW) {
-        // there is more data to be returned. let the caller know by returning -1
-        return -1;
+    LOG_WINDOW("Resetting statement %p after fetching %d rows and adding %d rows"
+            "to the window in %d bytes",
+            statement, totalRows, addedRows, window->size() - window->freeSpace());
+    sqlite3_reset(statement);
+
+    // Report the total number of rows on request.
+    if (startPos > totalRows) {
+        LOGE("startPos %d > actual rows %d", startPos, totalRows);
     }
-  return_count:
-    if (startPos) {
-        sqlite3_reset(statement);
-        LOG_WINDOW("Not doing count(*) because startPos %d is non-zero", startPos);
-        return 0;
-    } else if (gotAllRows) {
-        sqlite3_reset(statement);
-        LOG_WINDOW("Not doing count(*) because we already know the count(*)");
-        return numRows;
-    } else if (gotException) {
-        return 0;
-    } else {
-        // since startPos == 0, we need to get the count(*) of the result set
-        return numRows + 1 + finish_program_and_get_row_count(statement);
-    }
+    return countAllRows ? totalRows : 0;
 }
 
-static jint native_column_count(JNIEnv* env, jobject object)
-{
-    sqlite3_stmt * statement = GET_STATEMENT(env, object);
-
+static jint nativeColumnCount(JNIEnv* env, jclass clazz, jint statementPtr) {
+    sqlite3_stmt* statement = reinterpret_cast<sqlite3_stmt*>(statementPtr);
     return sqlite3_column_count(statement);
 }
 
-static jstring native_column_name(JNIEnv* env, jobject object, jint columnIndex)
-{
-    sqlite3_stmt * statement = GET_STATEMENT(env, object);
-    char const * name;
-
-    name = sqlite3_column_name(statement, columnIndex);
-
+static jstring nativeColumnName(JNIEnv* env, jclass clazz, jint statementPtr,
+        jint columnIndex) {
+    sqlite3_stmt* statement = reinterpret_cast<sqlite3_stmt*>(statementPtr);
+    const char* name = sqlite3_column_name(statement, columnIndex);
     return env->NewStringUTF(name);
 }
 
@@ -354,30 +238,16 @@
 static JNINativeMethod sMethods[] =
 {
      /* name, signature, funcPtr */
-    {"native_fill_window", "(IIIII)I",
-            (void *)native_fill_window},
-    {"native_column_count", "()I", (void*)native_column_count},
-    {"native_column_name", "(I)Ljava/lang/String;", (void *)native_column_name},
+    { "nativeFillWindow", "(IIIII)I",
+            (void*)nativeFillWindow },
+    { "nativeColumnCount", "(I)I",
+            (void*)nativeColumnCount},
+    { "nativeColumnName", "(II)Ljava/lang/String;",
+            (void*)nativeColumnName},
 };
 
 int register_android_database_SQLiteQuery(JNIEnv * env)
 {
-    jclass clazz;
-
-    clazz = env->FindClass("android/database/sqlite/SQLiteQuery");
-    if (clazz == NULL) {
-        LOGE("Can't find android/database/sqlite/SQLiteQuery");
-        return -1;
-    }
-
-    gHandleField = env->GetFieldID(clazz, "nHandle", "I");
-    gStatementField = env->GetFieldID(clazz, "nStatement", "I");
-
-    if (gHandleField == NULL || gStatementField == NULL) {
-        LOGE("Error locating fields");
-        return -1;
-    }
-
     return AndroidRuntime::registerNativeMethods(env,
         "android/database/sqlite/SQLiteQuery", sMethods, NELEM(sMethods));
 }
diff --git a/core/tests/coretests/src/android/database/DatabaseCursorTest.java b/core/tests/coretests/src/android/database/DatabaseCursorTest.java
index d5b9ee6..179338d 100644
--- a/core/tests/coretests/src/android/database/DatabaseCursorTest.java
+++ b/core/tests/coretests/src/android/database/DatabaseCursorTest.java
@@ -290,110 +290,7 @@
         public void onInvalidated() {
         }
     }
-    
-    //@Large
-    @Suppress
-    public void testLoadingThreadDelayRegisterData() throws Exception {
-        mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, data INT);");
-        
-        final int count = 505; 
-        String sql = "INSERT INTO test (data) VALUES (?);";
-        SQLiteStatement s = mDatabase.compileStatement(sql);
-        for (int i = 0; i < count; i++) {
-            s.bindLong(1, i);
-            s.execute();
-        }
 
-        int maxRead = 500;
-        int initialRead = 5;
-        SQLiteCursor c = (SQLiteCursor)mDatabase.rawQuery("select * from test;",
-                null, initialRead, maxRead);
-        
-        TestObserver observer = new TestObserver(count, c);
-        c.getCount();
-        c.registerDataSetObserver(observer);
-        if (!observer.quit) {
-            Looper.loop();
-        }
-        c.close();
-    }
-    
-    //@LargeTest
-    @BrokenTest("Consistently times out")
-    @Suppress
-    public void testLoadingThread() throws Exception {
-        mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, data INT);");
-        
-        final int count = 50000; 
-        String sql = "INSERT INTO test (data) VALUES (?);";
-        SQLiteStatement s = mDatabase.compileStatement(sql);
-        for (int i = 0; i < count; i++) {
-            s.bindLong(1, i);
-            s.execute();
-        }
-
-        int maxRead = 1000;
-        int initialRead = 5;
-        SQLiteCursor c = (SQLiteCursor)mDatabase.rawQuery("select * from test;",
-                null, initialRead, maxRead);
-        
-        TestObserver observer = new TestObserver(count, c);
-        c.registerDataSetObserver(observer);
-        c.getCount();
-        
-        Looper.loop();
-        c.close();
-    } 
-    
-    //@LargeTest
-    @BrokenTest("Consistently times out")
-    @Suppress
-    public void testLoadingThreadClose() throws Exception {
-        mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, data INT);");
-        
-        final int count = 1000; 
-        String sql = "INSERT INTO test (data) VALUES (?);";
-        SQLiteStatement s = mDatabase.compileStatement(sql);
-        for (int i = 0; i < count; i++) {
-            s.bindLong(1, i);
-            s.execute();
-        }
-
-        int maxRead = 11;
-        int initialRead = 5;
-        SQLiteCursor c = (SQLiteCursor)mDatabase.rawQuery("select * from test;",
-                null, initialRead, maxRead);
-        
-        TestObserver observer = new TestObserver(count, c);
-        c.registerDataSetObserver(observer);
-        c.getCount();       
-        c.close();
-    }
-    
-    @LargeTest
-    public void testLoadingThreadDeactivate() throws Exception {
-        mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, data INT);");
-        
-        final int count = 1000; 
-        String sql = "INSERT INTO test (data) VALUES (?);";
-        SQLiteStatement s = mDatabase.compileStatement(sql);
-        for (int i = 0; i < count; i++) {
-            s.bindLong(1, i);
-            s.execute();
-        }
-
-        int maxRead = 11;
-        int initialRead = 5;
-        SQLiteCursor c = (SQLiteCursor)mDatabase.rawQuery("select * from test;",
-                null, initialRead, maxRead);
-        
-        TestObserver observer = new TestObserver(count, c);
-        c.registerDataSetObserver(observer);
-        c.getCount();       
-        c.deactivate();
-        c.close();
-    }
-    
     @LargeTest
     public void testManyRowsLong() throws Exception {
         mDatabase.execSQL("CREATE TABLE test (_id INTEGER PRIMARY KEY, data INT);");