Bug 947939 - Part 2: transaction handling in deletes and updates, more efficient bulk updates. r=nalexander
authorRichard Newman <rnewman@mozilla.com>
Tue, 25 Feb 2014 12:27:54 -0800
changeset 170956 8e8a2c4025eeb7d110cc80c8dd166d4f78387c19
parent 170955 ba49fd7dcf7e7cadf31e3396caa58f342dce4318
child 170957 c349f7cdd5c1c850c30ee768797350f3200ff42d
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersnalexander
bugs947939
milestone30.0a1
Bug 947939 - Part 2: transaction handling in deletes and updates, more efficient bulk updates. r=nalexander
mobile/android/base/db/BrowserProvider.java
mobile/android/base/db/TransactionalProvider.java
--- a/mobile/android/base/db/BrowserProvider.java
+++ b/mobile/android/base/db/BrowserProvider.java
@@ -285,16 +285,36 @@ public class BrowserProvider extends Tra
         if (i < items) {
             builder.append("?");
         }
         builder.append(")");
         return builder.toString();
     }
 
     /**
+     * Turn a single-column cursor of longs into a single SQL "IN" clause.
+     * We can do this without using selection arguments because Long isn't
+     * vulnerable to injection.
+     */
+    private static String computeSQLInClauseFromLongs(final Cursor cursor, String field) {
+        final StringBuilder builder = new StringBuilder(field);
+        builder.append(" IN (");
+        final int commaLimit = cursor.getCount() - 1;
+        int i = 0;
+        while (cursor.moveToNext()) {
+            builder.append(cursor.getLong(0));
+            if (i++ < commaLimit) {
+                builder.append(", ");
+            }
+        }
+        builder.append(")");
+        return builder.toString();
+    }
+
+    /**
      * Clean up some deleted records from the specified table.
      *
      * If called in an existing transaction, it is the caller's responsibility
      * to ensure that the transaction is already upgraded to a writer, because
      * this method issues a read followed by a write, and thus is potentially
      * vulnerable to an unhandled SQLITE_BUSY failure during the upgrade.
      *
      * If not called in an existing transaction, no new explicit transaction
@@ -336,18 +356,16 @@ public class BrowserProvider extends Tra
 
     /**
      * Remove enough history items to bring the database count below <code>retain</code>,
      * removing no items with a modified time after <code>keepAfter</code>.
      *
      * Provide <code>keepAfter</code> less than or equal to zero to skip that check.
      *
      * Items will be removed according to an approximate frecency calculation.
-     *
-     * Call this method within a transaction.
      */
     private void expireHistory(final SQLiteDatabase db, final int retain, final long keepAfter) {
         Log.d(LOGTAG, "Expiring history.");
         final long rows = DatabaseUtils.queryNumEntries(db, TABLE_HISTORY);
 
         if (retain >= rows) {
             debug("Not expiring history: only have " + rows + " rows.");
             return;
@@ -366,16 +384,18 @@ public class BrowserProvider extends Tra
                     "ORDER BY " + sortOrder + " LIMIT " + toRemove +
                   ")";
         } else {
             sql = "DELETE FROM " + TABLE_HISTORY + " WHERE " + History._ID + " " +
                   "IN ( SELECT " + History._ID + " FROM " + TABLE_HISTORY + " " +
                   "ORDER BY " + sortOrder + " LIMIT " + toRemove + ")";
         }
         trace("Deleting using query: " + sql);
+
+        beginWrite(db);
         db.execSQL(sql);
     }
 
     /**
      * Remove any thumbnails that for sites that aren't likely to be ever shown.
      * Items will be removed according to a frecency calculation and only if they are not pinned
      *
      * Call this method within a transaction.
@@ -450,16 +470,17 @@ public class BrowserProvider extends Tra
         return null;
     }
 
     @SuppressWarnings("fallthrough")
     @Override
     public int deleteInTransaction(Uri uri, String selection, String[] selectionArgs) {
         trace("Calling delete in transaction on URI: " + uri);
         final SQLiteDatabase db = getWritableDatabase(uri);
+
         final int match = URI_MATCHER.match(uri);
         int deleted = 0;
 
         switch (match) {
             case BOOKMARKS_ID:
                 trace("Delete on BOOKMARKS_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_BOOKMARKS + "._id = ?");
@@ -477,16 +498,17 @@ public class BrowserProvider extends Tra
                 trace("Delete on HISTORY_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_HISTORY + "._id = ?");
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[] { Long.toString(ContentUris.parseId(uri)) });
                 // fall through
             case HISTORY: {
                 trace("Deleting history: " + uri);
+                beginWrite(db);
                 deleted = deleteHistory(uri, selection, selectionArgs);
                 deleteUnusedImages(uri);
                 break;
             }
 
             case HISTORY_OLD: {
                 String priority = uri.getQueryParameter(BrowserContract.PARAM_EXPIRE_PRIORITY);
                 long keepAfter = System.currentTimeMillis() - DEFAULT_EXPIRY_PRESERVE_WINDOW;
@@ -506,29 +528,31 @@ public class BrowserProvider extends Tra
                 debug("Delete on FAVICON_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_FAVICONS + "._id = ?");
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[] { Long.toString(ContentUris.parseId(uri)) });
                 // fall through
             case FAVICONS: {
                 trace("Deleting favicons: " + uri);
+                beginWrite(db);
                 deleted = deleteFavicons(uri, selection, selectionArgs);
                 break;
             }
 
             case THUMBNAIL_ID:
                 debug("Delete on THUMBNAIL_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_THUMBNAILS + "._id = ?");
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[] { Long.toString(ContentUris.parseId(uri)) });
                 // fall through
             case THUMBNAILS: {
                 trace("Deleting thumbnails: " + uri);
+                beginWrite(db);
                 deleted = deleteThumbnails(uri, selection, selectionArgs);
                 break;
             }
 
             default:
                 throw new UnsupportedOperationException("Unknown delete URI " + uri);
         }
 
@@ -585,114 +609,119 @@ public class BrowserProvider extends Tra
     @Override
     public int updateInTransaction(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         trace("Calling update in transaction on URI: " + uri);
 
         int match = URI_MATCHER.match(uri);
         int updated = 0;
 
+        final SQLiteDatabase db = getWritableDatabase(uri);
         switch (match) {
             // We provide a dedicated (hacky) API for callers to bulk-update the positions of
             // folder children by passing an array of GUID strings as `selectionArgs`.
             // Each child will have its position column set to its index in the provided array.
             //
             // This avoids callers having to issue a large number of UPDATE queries through
             // the usual channels. See Bug 728783.
             //
             // Note that this is decidedly not a general-purpose API; use at your own risk.
             // `values` and `selection` are ignored.
             case BOOKMARKS_POSITIONS: {
                 debug("Update on BOOKMARKS_POSITIONS: " + uri);
+
+                // This already starts and finishes its own transaction.
                 updated = updateBookmarkPositions(uri, selectionArgs);
                 break;
             }
 
             case BOOKMARKS_PARENT: {
                 debug("Update on BOOKMARKS_PARENT: " + uri);
-                updated = updateBookmarkParents(uri, values, selection, selectionArgs);
+                beginWrite(db);
+                updated = updateBookmarkParents(db, values, selection, selectionArgs);
                 break;
             }
 
             case BOOKMARKS_ID:
                 debug("Update on BOOKMARKS_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_BOOKMARKS + "._id = ?");
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[] { Long.toString(ContentUris.parseId(uri)) });
                 // fall through
             case BOOKMARKS: {
                 debug("Updating bookmark: " + uri);
-                if (shouldUpdateOrInsert(uri))
+                if (shouldUpdateOrInsert(uri)) {
                     updated = updateOrInsertBookmark(uri, values, selection, selectionArgs);
-                else
+                } else {
                     updated = updateBookmarks(uri, values, selection, selectionArgs);
+                }
                 break;
             }
 
             case HISTORY_ID:
                 debug("Update on HISTORY_ID: " + uri);
 
                 selection = DBUtils.concatenateWhere(selection, TABLE_HISTORY + "._id = ?");
                 selectionArgs = DBUtils.appendSelectionArgs(selectionArgs,
                         new String[] { Long.toString(ContentUris.parseId(uri)) });
                 // fall through
             case HISTORY: {
                 debug("Updating history: " + uri);
-                if (shouldUpdateOrInsert(uri))
+                if (shouldUpdateOrInsert(uri)) {
                     updated = updateOrInsertHistory(uri, values, selection, selectionArgs);
-                else
+                } else {
                     updated = updateHistory(uri, values, selection, selectionArgs);
+                }
                 break;
             }
 
             case FAVICONS: {
                 debug("Update on FAVICONS: " + uri);
 
                 String url = values.getAsString(Favicons.URL);
                 String faviconSelection = null;
                 String[] faviconSelectionArgs = null;
 
                 if (!TextUtils.isEmpty(url)) {
                     faviconSelection = Favicons.URL + " = ?";
                     faviconSelectionArgs = new String[] { url };
                 }
 
-                if (shouldUpdateOrInsert(uri))
+                if (shouldUpdateOrInsert(uri)) {
                     updated = updateOrInsertFavicon(uri, values, faviconSelection, faviconSelectionArgs);
-                else
+                } else {
                     updated = updateExistingFavicon(uri, values, faviconSelection, faviconSelectionArgs);
-
+                }
                 break;
             }
 
             case THUMBNAILS: {
                 debug("Update on THUMBNAILS: " + uri);
 
                 String url = values.getAsString(Thumbnails.URL);
 
                 // if no URL is provided, update all of the entries
-                if (TextUtils.isEmpty(values.getAsString(Thumbnails.URL)))
+                if (TextUtils.isEmpty(values.getAsString(Thumbnails.URL))) {
                     updated = updateExistingThumbnail(uri, values, null, null);
-                else if (shouldUpdateOrInsert(uri))
+                } else if (shouldUpdateOrInsert(uri)) {
                     updated = updateOrInsertThumbnail(uri, values, Thumbnails.URL + " = ?",
                                                       new String[] { url });
-                else
+                } else {
                     updated = updateExistingThumbnail(uri, values, Thumbnails.URL + " = ?",
                                                       new String[] { url });
-
+                }
                 break;
             }
 
             default:
                 throw new UnsupportedOperationException("Unknown update URI " + uri);
         }
 
         debug("Updated " + updated + " rows for URI: " + uri);
-
         return updated;
     }
 
     @Override
     public Cursor query(Uri uri, String[] projection, String selection,
             String[] selectionArgs, String sortOrder) {
         SQLiteDatabase db = getReadableDatabase(uri);
         final int match = URI_MATCHER.match(uri);
@@ -902,16 +931,18 @@ public class BrowserProvider extends Tra
         }
 
         return 0;
     }
 
     /**
      * Update the positions of bookmarks in batches.
      *
+     * Begins and ends its own transactions.
+     *
      * @see #updateBookmarkPositionsInTransaction(SQLiteDatabase, String[], int, int)
      */
     int updateBookmarkPositions(Uri uri, String[] guids) {
         if (guids == null) {
             return 0;
         }
 
         int guidsCount = guids.length;
@@ -919,16 +950,17 @@ public class BrowserProvider extends Tra
             return 0;
         }
 
         int offset = 0;
         int updated = 0;
 
         final SQLiteDatabase db = getWritableDatabase(uri);
         db.beginTransaction();
+
         while (offset < guidsCount) {
             try {
                 updated += updateBookmarkPositionsInTransaction(db, guids, offset,
                                                                 MAX_POSITION_UPDATES_PER_QUERY);
             } catch (SQLException e) {
                 Log.e(LOGTAG, "Got SQLite exception updating bookmark positions at offset " + offset, e);
 
                 // Need to restart the transaction.
@@ -949,18 +981,18 @@ public class BrowserProvider extends Tra
 
         return updated;
     }
 
     /**
      * Construct and execute an update expression that will modify the positions
      * of records in-place.
      */
-    int updateBookmarkPositionsInTransaction(final SQLiteDatabase db, final String[] guids,
-                                             final int offset, final int max) {
+    private static int updateBookmarkPositionsInTransaction(final SQLiteDatabase db, final String[] guids,
+                                                            final int offset, final int max) {
         int guidsCount = guids.length;
         int processCount = Math.min(max, guidsCount - offset);
 
         // Each must appear twice: once in a CASE, and once in the IN clause.
         String[] args = new String[processCount * 2];
         System.arraycopy(guids, offset, args, 0, processCount);
         System.arraycopy(guids, offset, args, processCount, processCount);
 
@@ -993,23 +1025,23 @@ public class BrowserProvider extends Tra
         // We can't easily get a modified count without calling something like changes().
         return processCount;
     }
 
     /**
      * Construct an update expression that will modify the parents of any records
      * that match.
      */
-    int updateBookmarkParents(Uri uri, ContentValues values, String selection, String[] selectionArgs) {
+    private int updateBookmarkParents(SQLiteDatabase db, ContentValues values, String selection, String[] selectionArgs) {
         trace("Updating bookmark parents of " + selection + " (" + selectionArgs[0] + ")");
         String where = Bookmarks._ID + " IN (" +
                        " SELECT DISTINCT " + Bookmarks.PARENT +
                        " FROM " + TABLE_BOOKMARKS +
                        " WHERE " + selection + " )";
-        return getWritableDatabase(uri).update(TABLE_BOOKMARKS, values, where, selectionArgs);
+        return db.update(TABLE_BOOKMARKS, values, where, selectionArgs);
     }
 
     long insertBookmark(Uri uri, ContentValues values) {
         // Generate values if not specified. Don't overwrite
         // if specified by caller.
         long now = System.currentTimeMillis();
         if (!values.containsKey(Bookmarks.DATE_CREATED)) {
             values.put(Bookmarks.DATE_CREATED, now);
@@ -1025,87 +1057,87 @@ public class BrowserProvider extends Tra
 
         if (!values.containsKey(Bookmarks.POSITION)) {
             debug("Inserting bookmark with no position for URI");
             values.put(Bookmarks.POSITION,
                        Long.toString(BrowserContract.Bookmarks.DEFAULT_POSITION));
         }
 
         String url = values.getAsString(Bookmarks.URL);
-        Integer type = values.getAsInteger(Bookmarks.TYPE);
 
         debug("Inserting bookmark in database with URL: " + url);
         final SQLiteDatabase db = getWritableDatabase(uri);
+        beginWrite(db);
         return db.insertOrThrow(TABLE_BOOKMARKS, Bookmarks.TITLE, values);
     }
 
 
     int updateOrInsertBookmark(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         int updated = updateBookmarks(uri, values, selection, selectionArgs);
-        if (updated > 0)
+        if (updated > 0) {
             return updated;
+        }
 
+        // Transaction already begun by updateBookmarks.
         if (0 <= insertBookmark(uri, values)) {
             // We 'updated' one row.
             return 1;
         }
 
         // If something went wrong, then we updated zero rows.
         return 0;
     }
 
     int updateBookmarks(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         trace("Updating bookmarks on URI: " + uri);
 
-        int updated = 0;
-
         final String[] bookmarksProjection = new String[] {
                 Bookmarks._ID, // 0
-                Bookmarks.URL, // 1
         };
 
-        trace("Quering bookmarks to update on URI: " + uri);
+        if (!values.containsKey(Bookmarks.DATE_MODIFIED)) {
+            values.put(Bookmarks.DATE_MODIFIED, System.currentTimeMillis());
+        }
 
+        trace("Querying bookmarks to update on URI: " + uri);
         final SQLiteDatabase db = getWritableDatabase(uri);
+
+        // Compute matching IDs.
         final Cursor cursor = db.query(TABLE_BOOKMARKS, bookmarksProjection,
                                        selection, selectionArgs, null, null, null);
 
+        // Now that we're done reading, open a transaction.
+        final String inClause;
         try {
-            if (!values.containsKey(Bookmarks.DATE_MODIFIED)) {
-                values.put(Bookmarks.DATE_MODIFIED, System.currentTimeMillis());
-            }
-            while (cursor.moveToNext()) {
-                long id = cursor.getLong(0);
-                trace("Updating bookmark with ID: " + id);
-                updated += db.update(TABLE_BOOKMARKS, values, "_id = ?",
-                                     new String[] { Long.toString(id) });
-            }
+            inClause = computeSQLInClauseFromLongs(cursor, Bookmarks._ID);
         } finally {
             cursor.close();
         }
 
-        return updated;
+        beginWrite(db);
+        return db.update(TABLE_BOOKMARKS, values, inClause, null);
     }
 
     long insertHistory(Uri uri, ContentValues values) {
         final long now = System.currentTimeMillis();
         values.put(History.DATE_CREATED, now);
         values.put(History.DATE_MODIFIED, now);
 
         // Generate GUID for new history entry. Don't override specified GUIDs.
         if (!values.containsKey(History.GUID)) {
           values.put(History.GUID, Utils.generateGuid());
         }
 
         String url = values.getAsString(History.URL);
 
         debug("Inserting history in database with URL: " + url);
         final SQLiteDatabase db = getWritableDatabase(uri);
+        beginWrite(db);
         return db.insertOrThrow(TABLE_HISTORY, History.VISITS, values);
     }
 
     int updateOrInsertHistory(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         final int updated = updateHistory(uri, values, selection, selectionArgs);
         if (updated > 0) {
             return updated;
@@ -1187,42 +1219,42 @@ public class BrowserProvider extends Tra
         return insertFavicon(getWritableDatabase(uri), values);
     }
 
     long insertFavicon(SQLiteDatabase db, ContentValues values) {
         // This method is a dupicate of BrowserDatabaseHelper.insertFavicon.
         // If changes are needed, please update both
         String faviconUrl = values.getAsString(Favicons.URL);
         String pageUrl = null;
-        long faviconId;
 
         trace("Inserting favicon for URL: " + faviconUrl);
 
         DBUtils.stripEmptyByteArray(values, Favicons.DATA);
 
         // Extract the page URL from the ContentValues
         if (values.containsKey(Favicons.PAGE_URL)) {
             pageUrl = values.getAsString(Favicons.PAGE_URL);
             values.remove(Favicons.PAGE_URL);
         }
 
         // If no URL is provided, insert using the default one.
         if (TextUtils.isEmpty(faviconUrl) && !TextUtils.isEmpty(pageUrl)) {
             values.put(Favicons.URL, org.mozilla.gecko.favicons.Favicons.guessDefaultFaviconURL(pageUrl));
         }
 
-        long now = System.currentTimeMillis();
+        final long now = System.currentTimeMillis();
         values.put(Favicons.DATE_CREATED, now);
         values.put(Favicons.DATE_MODIFIED, now);
-        faviconId = db.insertOrThrow(TABLE_FAVICONS, null, values);
+
+        beginWrite(db);
+        final long faviconId = db.insertOrThrow(TABLE_FAVICONS, null, values);
 
         if (pageUrl != null) {
             updateFaviconIdsForUrl(db, pageUrl, faviconId);
         }
-
         return faviconId;
     }
 
     int updateOrInsertFavicon(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         return updateFavicon(uri, values, selection, selectionArgs,
                 true /* insert if needed */);
     }
@@ -1272,65 +1304,69 @@ public class BrowserProvider extends Tra
                 try {
                     if (cursor.moveToFirst()) {
                         faviconId = cursor.getLong(cursor.getColumnIndexOrThrow(Favicons._ID));
                     }
                 } finally {
                     cursor.close();
                 }
             }
+            if (pageUrl != null) {
+                beginWrite(db);
+            }
         } else if (insertIfNeeded) {
             values.put(Favicons.DATE_CREATED, now);
 
             trace("No update, inserting favicon for URL: " + faviconUrl);
+            beginWrite(db);
             faviconId = db.insert(TABLE_FAVICONS, null, values);
             updated = 1;
         }
 
         if (pageUrl != null) {
             updateFaviconIdsForUrl(db, pageUrl, faviconId);
         }
 
         return updated;
     }
 
-    long insertThumbnail(Uri uri, ContentValues values) {
-        String url = values.getAsString(Thumbnails.URL);
-        final SQLiteDatabase db = getWritableDatabase(uri);
+    private long insertThumbnail(Uri uri, ContentValues values) {
+        final String url = values.getAsString(Thumbnails.URL);
 
         trace("Inserting thumbnail for URL: " + url);
 
         DBUtils.stripEmptyByteArray(values, Thumbnails.DATA);
 
+        final SQLiteDatabase db = getWritableDatabase(uri);
+        beginWrite(db);
         return db.insertOrThrow(TABLE_THUMBNAILS, null, values);
     }
 
-    int updateOrInsertThumbnail(Uri uri, ContentValues values, String selection,
+    private int updateOrInsertThumbnail(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         return updateThumbnail(uri, values, selection, selectionArgs,
                 true /* insert if needed */);
     }
 
-    int updateExistingThumbnail(Uri uri, ContentValues values, String selection,
+    private int updateExistingThumbnail(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
         return updateThumbnail(uri, values, selection, selectionArgs,
                 false /* only update, no insert */);
     }
 
-    int updateThumbnail(Uri uri, ContentValues values, String selection,
+    private int updateThumbnail(Uri uri, ContentValues values, String selection,
             String[] selectionArgs, boolean insertIfNeeded) {
-        String url = values.getAsString(Thumbnails.URL);
-        int updated = 0;
-        final SQLiteDatabase db = getWritableDatabase(uri);
-
+        final String url = values.getAsString(Thumbnails.URL);
         DBUtils.stripEmptyByteArray(values, Thumbnails.DATA);
 
         trace("Updating thumbnail for URL: " + url);
 
-        updated = db.update(TABLE_THUMBNAILS, values, selection, selectionArgs);
+        final SQLiteDatabase db = getWritableDatabase(uri);
+        beginWrite(db);
+        int updated = db.update(TABLE_THUMBNAILS, values, selection, selectionArgs);
 
         if (updated == 0 && insertIfNeeded) {
             trace("No update, inserting thumbnail for URL: " + url);
             db.insert(TABLE_THUMBNAILS, null, values);
             updated = 1;
         }
 
         return updated;
@@ -1381,16 +1417,17 @@ public class BrowserProvider extends Tra
     }
 
     int deleteBookmarks(Uri uri, String selection, String[] selectionArgs) {
         debug("Deleting bookmarks for URI: " + uri);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
 
         if (isCallerSync(uri)) {
+            beginWrite(db);
             return db.delete(TABLE_BOOKMARKS, selection, selectionArgs);
         }
 
         debug("Marking bookmarks as deleted for URI: " + uri);
 
         ContentValues values = new ContentValues();
         values.put(Bookmarks.IS_DELETED, 1);
 
@@ -1451,39 +1488,46 @@ public class BrowserProvider extends Tra
                deleteThumbnails(uri, thumbnailSelection, null);
     }
 
     @Override
     public ContentProviderResult[] applyBatch (ArrayList<ContentProviderOperation> operations)
         throws OperationApplicationException {
         final int numOperations = operations.size();
         final ContentProviderResult[] results = new ContentProviderResult[numOperations];
-        boolean failures = false;
-        SQLiteDatabase db = null;
 
-        if (numOperations >= 1) {
-            // We only have 1 database for all Uri's that we can get
-            db = getWritableDatabase(operations.get(0).getUri());
-        } else {
+        if (numOperations < 1) {
+            debug("applyBatch: no operations; returning immediately.");
             // The original Android implementation returns a zero-length
-            // array in this case, we do the same.
+            // array in this case. We do the same.
             return results;
         }
 
+        boolean failures = false;
+
+        // We only have 1 database for all Uris that we can get.
+        SQLiteDatabase db = getWritableDatabase(operations.get(0).getUri());
+
         // Note that the apply() call may cause us to generate
-        // additional transactions for the invidual operations.
+        // additional transactions for the individual operations.
         // But Android's wrapper for SQLite supports nested transactions,
         // so this will do the right thing.
-        db.beginTransaction();
+        //
+        // Note further that in some circumstances this can result in
+        // exceptions: if this transaction is first involved in reading,
+        // and then (naturally) tries to perform writes, SQLITE_BUSY can
+        // be raised. See Bug 947939 and friends.
+        beginBatch(db);
 
         for (int i = 0; i < numOperations; i++) {
             try {
-                results[i] = operations.get(i).apply(this, results, i);
+                final ContentProviderOperation operation = operations.get(i);
+                results[i] = operation.apply(this, results, i);
             } catch (SQLException e) {
-                Log.w(LOGTAG, "SQLite Exception during applyBatch: ", e);
+                Log.w(LOGTAG, "SQLite Exception during applyBatch.", e);
                 // The Android API makes it implementation-defined whether
                 // the failure of a single operation makes all others abort
                 // or not. For our use cases, best-effort operation makes
                 // more sense. Rolling back and forcing the caller to retry
                 // after it figures out what went wrong isn't very convenient
                 // anyway.
                 // Signal failed operation back, so the caller knows what
                 // went through and what didn't.
@@ -1506,18 +1550,18 @@ public class BrowserProvider extends Tra
                 failures = true;
                 db.setTransactionSuccessful();
                 db.endTransaction();
                 db.beginTransaction();
             }
         }
 
         trace("Flushing DB applyBatch...");
-        db.setTransactionSuccessful();
-        db.endTransaction();
+        markBatchSuccessful(db);
+        endBatch(db);
 
         if (failures) {
             throw new OperationApplicationException();
         }
 
         return results;
     }
 
--- a/mobile/android/base/db/TransactionalProvider.java
+++ b/mobile/android/base/db/TransactionalProvider.java
@@ -52,31 +52,33 @@ public abstract class TransactionalProvi
      * @param values column values to be inserted
      * @return       a URI for the newly inserted item
      */
     abstract protected Uri insertInTransaction(Uri uri, ContentValues values);
 
     /*
      * Deletes items from the database within a DB transaction.
      *
-     * @param uri            query URI
-     * @param selection      An optional filter to match rows to update.
-     * @param selectionArgs  arguments for the selection
-     * @return               number of rows impacted by the deletion
+     * @param uri            Query URI.
+     * @param selection      An optional filter to match rows to delete.
+     * @param selectionArgs  An array of arguments to substitute into the selection.
+     *
+     * @return number of rows impacted by the deletion.
      */
     abstract protected int deleteInTransaction(Uri uri, String selection, String[] selectionArgs);
 
     /*
      * Updates the database within a DB transaction.
      *
-     * @param uri            Query URI
+     * @param uri            Query URI.
      * @param values         A set of column_name/value pairs to add to the database.
      * @param selection      An optional filter to match rows to update.
-     * @param selectionArgs  Arguments for the selection
-     * @return               number of rows impacted by the update
+     * @param selectionArgs  An array of arguments to substitute into the selection.
+     *
+     * @return               number of rows impacted by the update.
      */
     abstract protected int updateInTransaction(Uri uri, ContentValues values, String selection, String[] selectionArgs);
 
     /*
      * Fetches a readable database based on the profile indicated in the
      * passed URI. If the URI does not contain a profile param, the default profile
      * is used.
      *
@@ -124,98 +126,191 @@ public abstract class TransactionalProvi
                         return createDatabaseHelper(context, databasePath);
                     }
                 });
         }
 
         return true;
     }
 
+    /**
+     * Return true if OS version and database parallelism support indicates
+     * that this provider should bundle writes into transactions.
+     */
+    @SuppressWarnings("static-method")
+    protected boolean shouldUseTransactions() {
+        return Build.VERSION.SDK_INT >= 11;
+    }
+
+    /**
+     * Track whether we're in a batch operation.
+     *
+     * When we're in a batch operation, individual write steps won't even try
+     * to start a transaction... and neither will they attempt to finish one.
+     *
+     * Set this to <code>Boolean.TRUE</code> when you're entering a batch --
+     * a section of code in which {@link ContentProvider} methods will be
+     * called, but nested transactions should not be started. Callers are
+     * responsible for beginning and ending the enclosing transaction, and
+     * for setting this to <code>Boolean.FALSE</code> when done.
+     *
+     * This is a ThreadLocal separate from `db.inTransaction` because batched
+     * operations start transactions independent of individual ContentProvider
+     * operations. This doesn't work well with the entire concept of this
+     * abstract class -- that is, automatically beginning and ending transactions
+     * for each insert/delete/update operation -- and doing so without
+     * causing arbitrary nesting requires external tracking.
+     *
+     * Note that beginWrite takes a DB argument, but we don't differentiate
+     * between databases in this tracking flag. If your ContentProvider manages
+     * multiple database transactions within the same thread, you'll need to
+     * amend this scheme -- but then, you're already doing some serious wizardry,
+     * so rock on.
+     */
+    final ThreadLocal<Boolean> isInBatchOperation = new ThreadLocal<Boolean>();
+
+    private boolean isInBatch() {
+        final Boolean isInBatch = isInBatchOperation.get();
+        if (isInBatch == null) {
+            return false;
+        }
+        return isInBatch.booleanValue();
+    }
+
+    /**
+     * If we're not currently in a transaction, and we should be, start one.
+     */
+    protected void beginWrite(final SQLiteDatabase db) {
+        if (isInBatch()) {
+            trace("Not bothering with an intermediate write transaction: inside batch operation.");
+            return;
+        }
+
+        if (shouldUseTransactions() && !db.inTransaction()) {
+            trace("beginWrite: beginning transaction.");
+            db.beginTransaction();
+        }
+    }
+
+    /**
+     * If we're not in a batch, but we are in a write transaction, mark it as
+     * successful.
+     */
+    protected void markWriteSuccessful(final SQLiteDatabase db) {
+        if (isInBatch()) {
+            trace("Not marking write successful: inside batch operation.");
+            return;
+        }
+
+        if (shouldUseTransactions() && db.inTransaction()) {
+            trace("Marking write transaction successful.");
+            db.setTransactionSuccessful();
+        }
+    }
+
+    /**
+     * If we're not in a batch, but we are in a write transaction,
+     * end it.
+     *
+     * @see TransactionalProvider#markWriteSuccessful(SQLiteDatabase)
+     */
+    protected void endWrite(final SQLiteDatabase db) {
+        if (isInBatch()) {
+            trace("Not ending write: inside batch operation.");
+            return;
+        }
+
+        if (shouldUseTransactions() && db.inTransaction()) {
+            trace("endWrite: ending transaction.");
+            db.endTransaction();
+        }
+    }
+
+    protected void beginBatch(final SQLiteDatabase db) {
+        trace("Beginning batch.");
+        isInBatchOperation.set(Boolean.TRUE);
+        db.beginTransaction();
+    }
+
+    protected void markBatchSuccessful(final SQLiteDatabase db) {
+        if (isInBatch()) {
+            trace("Marking batch successful.");
+            db.setTransactionSuccessful();
+            return;
+        }
+        Log.w(LOGTAG, "Unexpectedly asked to mark batch successful, but not in batch!");
+        throw new IllegalStateException("Not in batch.");
+    }
+
+    protected void endBatch(final SQLiteDatabase db) {
+        trace("Ending batch.");
+        db.endTransaction();
+        isInBatchOperation.set(Boolean.FALSE);
+    }
+
     @Override
     public int delete(Uri uri, String selection, String[] selectionArgs) {
-        trace("Calling delete on URI: " + uri);
+        trace("Calling delete on URI: " + uri + ", " + selection + ", " + selectionArgs);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
         int deleted = 0;
 
-        if (Build.VERSION.SDK_INT >= 11) {
-            trace("Beginning delete transaction: " + uri);
-            db.beginTransaction();
-            try {
-                deleted = deleteInTransaction(uri, selection, selectionArgs);
-                db.setTransactionSuccessful();
-                trace("Successful delete transaction: " + uri);
-            } finally {
-                db.endTransaction();
-            }
-        } else {
+        try {
             deleted = deleteInTransaction(uri, selection, selectionArgs);
+            markWriteSuccessful(db);
+        } finally {
+            endWrite(db);
         }
 
         if (deleted > 0) {
             getContext().getContentResolver().notifyChange(uri, null);
         }
 
         return deleted;
     }
 
     @Override
     public Uri insert(Uri uri, ContentValues values) {
         trace("Calling insert on URI: " + uri);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
         Uri result = null;
         try {
-            if (Build.VERSION.SDK_INT >= 11) {
-                trace("Beginning insert transaction: " + uri);
-                db.beginTransaction();
-                try {
-                    result = insertInTransaction(uri, values);
-                    db.setTransactionSuccessful();
-                    trace("Successful insert transaction: " + uri);
-                } finally {
-                    db.endTransaction();
-                }
-            } else {
-                result = insertInTransaction(uri, values);
-            }
+            result = insertInTransaction(uri, values);
+            markWriteSuccessful(db);
         } catch (SQLException sqle) {
             Log.e(LOGTAG, "exception in DB operation", sqle);
         } catch (UnsupportedOperationException uoe) {
             Log.e(LOGTAG, "don't know how to perform that insert", uoe);
+        } finally {
+            endWrite(db);
         }
 
         if (result != null) {
             getContext().getContentResolver().notifyChange(uri, null);
         }
 
         return result;
     }
 
 
     @Override
     public int update(Uri uri, ContentValues values, String selection,
             String[] selectionArgs) {
-        trace("Calling update on URI: " + uri);
+        trace("Calling update on URI: " + uri + ", " + selection + ", " + selectionArgs);
 
         final SQLiteDatabase db = getWritableDatabase(uri);
         int updated = 0;
 
-        if (Build.VERSION.SDK_INT >= 11) {
-            trace("Beginning update transaction: " + uri);
-            db.beginTransaction();
-            try {
-                updated = updateInTransaction(uri, values, selection, selectionArgs);
-                db.setTransactionSuccessful();
-                trace("Successful update transaction: " + uri);
-            } finally {
-                db.endTransaction();
-            }
-        } else {
-            updated = updateInTransaction(uri, values, selection, selectionArgs);
+        try {
+            updated = updateInTransaction(uri, values, selection,
+                                          selectionArgs);
+            markWriteSuccessful(db);
+        } finally {
+            endWrite(db);
         }
 
         if (updated > 0) {
             getContext().getContentResolver().notifyChange(uri, null);
         }
 
         return updated;
     }
@@ -226,27 +321,29 @@ public abstract class TransactionalProvi
             return 0;
         }
 
         int numValues = values.length;
         int successes = 0;
 
         final SQLiteDatabase db = getWritableDatabase(uri);
 
-        db.beginTransaction();
+        debug("bulkInsert: explicitly starting transaction.");
+        beginBatch(db);
 
         try {
             for (int i = 0; i < numValues; i++) {
                 insertInTransaction(uri, values[i]);
                 successes++;
             }
             trace("Flushing DB bulkinsert...");
-            db.setTransactionSuccessful();
+            markBatchSuccessful(db);
         } finally {
-            db.endTransaction();
+            debug("bulkInsert: explicitly ending transaction.");
+            endBatch(db);
         }
 
         if (successes > 0) {
             mContext.getContentResolver().notifyChange(uri, null);
         }
 
         return successes;
     }