Bug 1249657 - Remove no-op transaction management, and fail directly in DB upgrades r?mcomella draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 15 Mar 2016 15:52:12 -0700
changeset 340744 8609f57a169631d3444d3a6df2dc44e042a56310
parent 340743 b44597ac0648eabfb42ef8aea30b5bbf4e73328f
child 516261 a8934442b3f5d97631db92c849744535a83a54e0
push id13051
push userahunt@mozilla.com
push dateTue, 15 Mar 2016 22:52:34 +0000
reviewersmcomella
bugs1249657
milestone48.0a1
Bug 1249657 - Remove no-op transaction management, and fail directly in DB upgrades r?mcomella Database upgrades are already run in a transaction, we shouldn't be trying to do our own DB management on top of this. If the upgrade logic fails we should propagate the Exception and let the framework deal with it. See: http://developer.android.com/reference/android/database/sqlite/SQLiteOpenHelper.html#onUpgrade%28android.database.sqlite.SQLiteDatabase,%20int,%20int%29 MozReview-Commit-ID: LsAgW3XOyWB
mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -920,70 +920,58 @@ public final class BrowserDatabaseHelper
         final String[] selectionArgs = { String.valueOf(Bookmarks.FIXED_READING_LIST_ID), "0" };
         final String[] projection = {   Bookmarks._ID,
                                         Bookmarks.GUID,
                                         Bookmarks.URL,
                                         Bookmarks.DATE_MODIFIED,
                                         Bookmarks.DATE_CREATED,
                                         Bookmarks.TITLE };
 
-        try {
-            db.beginTransaction();
 
-            // Create 'reading_list' table.
-            createReadingListTable(db, TABLE_READING_LIST);
+        // Create 'reading_list' table.
+        createReadingListTable(db, TABLE_READING_LIST);
 
-            // Get all the reading list items from bookmarks table.
-            final Cursor cursor = db.query(TABLE_BOOKMARKS, projection, selection, selectionArgs, null, null, null);
+        // Get all the reading list items from bookmarks table.
+        final Cursor cursor = db.query(TABLE_BOOKMARKS, projection, selection, selectionArgs, null, null, null);
 
-            if (cursor == null) {
-                // This should never happen.
-                db.setTransactionSuccessful();
-                return;
-            }
+        if (cursor == null) {
+            // This should never happen.
+            return;
+        }
 
-            try {
-                // Insert reading list items into reading_list table.
-                while (cursor.moveToNext()) {
-                    debug(DatabaseUtils.dumpCurrentRowToString(cursor));
-                    final ContentValues values = new ContentValues();
-
-                    // We don't preserve bookmark GUIDs.
-                    DatabaseUtils.cursorStringToContentValues(cursor, Bookmarks.URL, values, ReadingListItems.URL);
-                    DatabaseUtils.cursorStringToContentValues(cursor, Bookmarks.TITLE, values, ReadingListItems.TITLE);
-                    DatabaseUtils.cursorLongToContentValues(cursor, Bookmarks.DATE_CREATED, values, ReadingListItems.ADDED_ON);
-                    DatabaseUtils.cursorLongToContentValues(cursor, Bookmarks.DATE_MODIFIED, values, ReadingListItems.CLIENT_LAST_MODIFIED);
+        try {
+            // Insert reading list items into reading_list table.
+            while (cursor.moveToNext()) {
+                debug(DatabaseUtils.dumpCurrentRowToString(cursor));
+                final ContentValues values = new ContentValues();
 
-                    db.insertOrThrow(TABLE_READING_LIST, null, values);
-                }
-            } finally {
-                cursor.close();
+                // We don't preserve bookmark GUIDs.
+                DatabaseUtils.cursorStringToContentValues(cursor, Bookmarks.URL, values, ReadingListItems.URL);
+                DatabaseUtils.cursorStringToContentValues(cursor, Bookmarks.TITLE, values, ReadingListItems.TITLE);
+                DatabaseUtils.cursorLongToContentValues(cursor, Bookmarks.DATE_CREATED, values, ReadingListItems.ADDED_ON);
+                DatabaseUtils.cursorLongToContentValues(cursor, Bookmarks.DATE_MODIFIED, values, ReadingListItems.CLIENT_LAST_MODIFIED);
+
+                db.insertOrThrow(TABLE_READING_LIST, null, values);
             }
-
-            // Delete reading list items from bookmarks table.
-            db.delete(TABLE_BOOKMARKS,
-                      Bookmarks.PARENT + " = ? ",
-                      new String[] { String.valueOf(Bookmarks.FIXED_READING_LIST_ID) });
+        } finally {
+            cursor.close();
+        }
 
-            // Delete reading list special folder.
-            db.delete(TABLE_BOOKMARKS,
-                      Bookmarks._ID + " = ? ",
-                      new String[] { String.valueOf(Bookmarks.FIXED_READING_LIST_ID) });
-
-            // Create indices.
-            createReadingListIndices(db, TABLE_READING_LIST);
+        // Delete reading list items from bookmarks table.
+        db.delete(TABLE_BOOKMARKS,
+                  Bookmarks.PARENT + " = ? ",
+                  new String[] { String.valueOf(Bookmarks.FIXED_READING_LIST_ID) });
 
-            // Done.
-            db.setTransactionSuccessful();
+        // Delete reading list special folder.
+        db.delete(TABLE_BOOKMARKS,
+                  Bookmarks._ID + " = ? ",
+                  new String[] { String.valueOf(Bookmarks.FIXED_READING_LIST_ID) });
 
-        } catch (SQLException e) {
-            Log.e(LOGTAG, "Error migrating reading list items", e);
-        } finally {
-            db.endTransaction();
-        }
+        // Create indices.
+        createReadingListIndices(db, TABLE_READING_LIST);
     }
 
     private void upgradeDatabaseFrom18to19(SQLiteDatabase db) {
         // Redefine the "combined" view...
         createV19CombinedView(db);
 
         // Kill any history entries with NULL URL. This ostensibly can't happen...
         db.execSQL("DELETE FROM " + TABLE_HISTORY + " WHERE " + History.URL + " IS NULL");