Last Comment Bug 713228 - SQLiteDatabaseCorruptException: database disk image is malformed: PRAGMA synchronous=1;
: SQLiteDatabaseCorruptException: database disk image is malformed: PRAGMA sync...
Status: RESOLVED FIXED
: dataloss
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: P2 normal (vote)
: Firefox 12
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
Depends on:
Blocks: 699199 704682
  Show dependency treegraph
 
Reported: 2011-12-23 07:43 PST by Aaron Train [:aaronmt]
Modified: 2012-01-26 23:59 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
+
fixed
11+


Attachments
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator. (38.97 KB, patch)
2012-01-16 14:03 PST, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review+
Details | Diff | Splinter Review
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator. (47.15 KB, patch)
2012-01-17 15:53 PST, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review-
Details | Diff | Splinter Review
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator. (51.24 KB, patch)
2012-01-18 06:26 PST, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review-
Details | Diff | Splinter Review
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator. (57.21 KB, patch)
2012-01-19 10:12 PST, Gian-Carlo Pascutto [:gcp]
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Aaron Train [:aaronmt] 2011-12-23 07:43:55 PST
I/ProfMigr( 1022): Opening path: /data/data/org.mozilla.fennec_aurora/files/mozilla/q1exgqly.default/places.sqlite
I/GeckoAboutHome( 1022): error reading json file
I/GeckoAboutHome( 1022): java.io.IOException: Bad file number
I/GeckoAboutHome( 1022): 	at java.util.zip.Inflater.setFileInputImpl(Native Method)
I/GeckoAboutHome( 1022): 	at java.util.zip.Inflater.setFileInput(Inflater.java:410)
I/GeckoAboutHome( 1022): 	at java.util.zip.InflaterInputStream.fill(InflaterInputStream.java:221)
I/GeckoAboutHome( 1022): 	at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:178)
I/GeckoAboutHome( 1022): 	at java.util.zip.ZipFile$ZipInflaterInputStream.read(ZipFile.java:459)
I/GeckoAboutHome( 1022): 	at org.mozilla.gecko.AboutHomeContent$7.run(AboutHomeContent.java:270)
I/GeckoAboutHome( 1022): 	at android.os.Handler.handleCallback(Handler.java:587)
I/GeckoAboutHome( 1022): 	at android.os.Handler.dispatchMessage(Handler.java:92)
I/GeckoAboutHome( 1022): 	at android.os.Looper.loop(Looper.java:130)
I/GeckoAboutHome( 1022): 	at org.mozilla.gecko.GeckoAppShell$LooperThread.run(GeckoAppShell.java:155)
I/Database( 1022): sqlite returned: error code = 11, msg = database corruption found by source line 40112
I/Database( 1022): sqlite returned: error code = 11, msg = database disk image is malformed
E/Database( 1022): Failure 11 (database disk image is malformed) on 0x1ca680 when preparing 'PRAGMA synchronous=1;'.
E/Database( 1022): Removing corrupt database: /data/data/org.mozilla.fennec_aurora/files/mozilla/q1exgqly.default/places.sqlite
E/Database( 1022): !@ - DB-onCorruption /data/data/org.mozilla.fennec_aurora/files/mozilla/q1exgqly.default/places.sqlite
E/Database( 1022): !@)bAdRfS
W/GLThreadManager( 1022): checkGLESVersion mGLESVersion = 131072 mMultipleGLESContextsAllowed = true
W/GLThread( 1022): onSurfaceCreated
E/Database( 1022): !@Deleting and re-creating corrupt database /data/data/org.mozilla.fennec_aurora/files/mozilla/q1exgqly.default/places.sqlite
E/Database( 1022): android.database.sqlite.SQLiteDatabaseCorruptException: database disk image is malformed: PRAGMA synchronous=1;
E/Database( 1022): 	at android.database.sqlite.SQLiteDatabase.native_execSQL(Native Method)
E/Database( 1022): 	at android.database.sqlite.SQLiteDatabase.execSQL(SQLiteDatabase.java:1904)
E/Database( 1022): 	at android.database.sqlite.SQLiteDatabase.<init>(SQLiteDatabase.java:2010)
E/Database( 1022): 	at android.database.sqlite.SQLiteDatabase.openDatabase(SQLiteDatabase.java:905)
E/Database( 1022): 	at org.mozilla.gecko.ProfileMigrator$PlacesTask.openPlaces(ProfileMigrator.java:312)
E/Database( 1022): 	at org.mozilla.gecko.ProfileMigrator$PlacesTask.migratePlaces(ProfileMigrator.java:336)
E/Database( 1022): 	at org.mozilla.gecko.ProfileMigrator$PlacesTask.run(ProfileMigrator.java:371)
E/Database( 1022): 	at java.lang.Thread.run(Thread.java:1019)
E/Database( 1022): !@)bAdRfS
I/Database( 1022): sqlite returned: error code = 14, msg = cannot open file at source line 25472
E/Database( 1022): sqlite3_open_v2("/data/data/org.mozilla.fennec_aurora/files/mozilla/q1exgqly.default/places.sqlite", &handle, 1, NULL) failed
I/ProfMigr( 1022): Error on places database:unable to open database file
W/GLThread( 1022): onSurfaceChanged(480, 699)
I/GLThread( 1022): sending render notification tid=9
D/CLIPBOARD( 1022): Hide Clipboard dialog at Starting input: finished by someone else... !

STR:

1. Install Aurora XUL (http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2011-12-20-04-20-29-mozilla-aurora-android/fennec-10.0a2.multi.android-arm.apk)

2. Browse to CNN.com, bookmark CNN.com

3. Install Aurora Native (http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-aurora-android/fennec-11.0a2.multi.android-arm.apk)

ER: See CNN.com imported and bookmarked
AR: Dont see CNN.com imported and bookmarked

---
Tested via:

Samsung Galaxy SII (Android 2.3.4)
Samsung Nexus S (Android 4.0.3)

20111220042029
http://hg.mozilla.org/releases/mozilla-aurora/rev/0d8490e27457

20111223042034
http://hg.mozilla.org/releases/mozilla-aurora/rev/7c408ff60a8e
Comment 1 Aaron Train [:aaronmt] 2011-12-23 11:07:01 PST
Nexus One (Android 2.3.4), slightly different, same actual result

I/GeckoApp( 1804): checking profile migration in: /data/data/org.mozilla.fennec_aurora/files/mozilla/n1kk77jy.default
I/ProfMigr( 1804): Opening path: /data/data/org.mozilla.fennec_aurora/files/mozilla/n1kk77jy.default/places.sqlite
I/Database( 1804): sqlite returned: error code = 11, msg = database corruption found by source line 40107
I/Database( 1804): sqlite returned: error code = 11, msg = database disk image is malformed
I/ProfMigr( 1804): Failed to get bookmarks: database disk image is malformed: , while compiling: SELECT places.url AS a_url, places.title AS a_title FROM (moz_places as places JOIN moz_bookmarks as bookmarks ON places.id = bookmarks.fk) WHERE places.hidden <> 1 ORDER BY bookmarks.dateAdded
I/Database( 1804): sqlite returned: error code = 11, msg = database corruption found by source line 40107
I/Database( 1804): sqlite returned: error code = 11, msg = database disk image is malformed
I/ProfMigr( 1804): Failed to get bookmarks: database disk image is malformed: , while compiling: SELECT places.url AS a_url, places.title AS a_title, history.visit_date AS a_date FROM (moz_historyvisits AS history JOIN moz_places AS places ON places.id = history.place_id) WHERE places.hidden <> 1 ORDER BY history.visit_date DESC
I/Database( 1804): sqlite returned: error code = 11, msg = database corruption found by source line 40107
I/Database( 1804): sqlite returned: error code = 11, msg = database disk image is malformed
I/ProfMigr( 1804): Failed to get favicons: database disk image is malformed: , while compiling: SELECT places.url AS a_url, favicon.data AS a_data, favicon.mime_type AS a_mime FROM (moz_places AS places JOIN moz_favicons AS favicon ON places.favicon_id = favicon.id)
Comment 2 Gian-Carlo Pascutto [:gcp] 2011-12-23 14:36:43 PST
Notes:

- places.sqlite was written by the SQLite that is used by XUL Fennec. I guess that's our own version which we update frequently.

- We're trying to read it in from Android, so using whatever SQLite Android bundles.

- Android seems to think the best way to deal with a database it thinks is corrupted is to delete it automatically (gee, thanks!). That could cause dataloss here, but perhaps the READONLY flag protects us from it.
Comment 3 Gian-Carlo Pascutto [:gcp] 2012-01-04 07:30:08 PST
STR do not produce the issue on my Galaxy Tab 10.1 with Android 3.1. Slightly surprising as the Android SQLite should be inbetween the 2.3.4 and 4.0.3 that are reprotedly affected. Will try with the Nexus S next.
Comment 4 Gian-Carlo Pascutto [:gcp] 2012-01-06 07:26:17 PST
I'm able to reproduce this on the Samsung Galaxy SII (Android 2.3.4) combo. The same database that gives the error works correctly on the Galaxy Tab 10.1 and can be read locally.
Comment 5 Gian-Carlo Pascutto [:gcp] 2012-01-16 14:03:22 PST
Created attachment 588998 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Patch to add a Java<->JNI<->SQLite Bridge. Simple API that works for the current users (parameter support requested by password manager). Updated ProfileMigrator to use this instead of the Android DB.

This makes the profile migration succeed on my SGS2.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2012-01-16 20:59:05 PST
Comment on attachment 588998 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Review of attachment 588998 [details] [diff] [review]:
-----------------------------------------------------------------

Just to cross your Ts you should get a storage peer to sign off on adding this code to their module (https://wiki.mozilla.org/Modules/All#storage).

::: mobile/android/base/ProfileMigrator.java
@@ +44,5 @@
>  
>  import android.database.Cursor;
>  import android.database.sqlite.SQLiteDatabase;
>  import android.database.sqlite.SQLiteException;
>  import android.database.sqlite.SQLiteStatement;

can these be removed now?

@@ +91,5 @@
>          + "places.id = bookmarks.fk) WHERE places.hidden <> 1 "
>          + "ORDER BY bookmarks.dateAdded";
> +    // Result column of relevant data
> +    final int bookmarkUrl   = 0;
> +    final int bookmarkTitle = 1;

this strikes me as slightly fragile. Please file a follow up to add an API to get column indexes from column names.

@@ +187,2 @@
>              try {
> +                ArrayList queryResult = db.query(historyQuery);

can we make that an ArrayList<Object[]> to avoid the cast below? Same comment for future uses as well obviously

::: mobile/android/base/sqlite/ByteArray.java
@@ +40,5 @@
> +/*
> + * Helper class which holds a byte[]-array, but
> + * which is derived from Object (which byte[] isn't).
> + */
> +public class ByteArray {

why not use the built in ByteBuffer?

::: mobile/android/base/sqlite/SQLiteBridge.java
@@ +77,5 @@
> +    // The result is returned as an ArrayList of Object[] arrays, with each
> +    // row being an entry in the ArrayList, and each column being one Object
> +    // in the Object[] array. The columns are of type null, ByteArray (BLOB),
> +    // or String (everything else).
> +    public ArrayList query(String aQuery, String[] aParams) throws SQLiteBridgeException {

ArrayList<Object[]>

::: mozglue/android/APKOpen.cpp
@@ -543,5 @@
>    MOZLOAD("mozalloc");
>    MOZLOAD("nspr4");
>    MOZLOAD("plc4");
>    MOZLOAD("plds4");
> -  MOZLOAD("mozsqlite3");

file a follow up to do this lib load (and your new library) through our custom lib loader

::: storage/android/SQLiteBridge.cpp
@@ +62,5 @@
> +    jenv->DeleteLocalRef(cls);
> +}
> +
> +jobject
> +JNI_Construct_ByteArray(JNIEnv* jenv, jbyteArray jBytes)

unneeded if you use ByteBuffer
Comment 7 Mike Hommey [:glandium] 2012-01-17 06:38:05 PST
I'll put here what I wrote on irc: i'd suggest not creating a separate lib for the sqlite bridge, but put that in mozglue, under mozglue/android, using the same tricks as for the jni bridge to libxul.so.
Comment 8 Gian-Carlo Pascutto [:gcp] 2012-01-17 15:53:02 PST
Created attachment 589331 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

So, I initially only addressed Brad's comments, but I ran into a bug in elfhack, which crashed on libsqlitebridge. I bit the bullet and split the entire Gecko lib loading into 2 parts, moved everything from libsqlitebridge into mozglue using dlsym and made it use our custom linker. So basically, all the benefits, none of the disadvantages of the previous approach. (Ok, maybe the dylsym code in APKOpen.cpp isn't as pretty as a separate lib)

I also added getColumnIndex for the SQL query results.

There's a huge amount of changes, so requesting rereview.
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2012-01-17 22:52:54 PST
Comment on attachment 589331 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Review of attachment 589331 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoApp.java
@@ +1435,5 @@
>          if (sGREDir == null)
>              sGREDir = new File(this.getApplicationInfo().dataDir);
>  
> +        GeckoAppShell.loadSQLiteLibs(mAppContext.getApplication().getPackageResourcePath());
> +        checkMigrateProfile();

why aren't you doing this delayed on the background thread anymore?

::: mobile/android/base/ProfileMigrator.java
@@ +238,5 @@
>              BitmapDrawable image = (BitmapDrawable) Drawable.createFromStream(byteStream, "src");
>              if (image != null) {
>                  try {
>                      BrowserDB.updateFaviconForUrl(mCr, url, image);
> +                } catch (Exception e) {

what is being thrown here? I'm not a fan of blanket catching Exception in general.

::: mozglue/android/APKOpen.cpp
@@ +644,5 @@
> +  GETFUNC(sqlite3_column_type);
> +  GETFUNC(sqlite3_column_blob);
> +  GETFUNC(sqlite3_column_bytes);
> +  GETFUNC(sqlite3_column_text);
> +#undef GETFUNC

nit, move this function wrapping code to an init function in SQLiteBridge.cpp

::: mozglue/android/SQLiteBridge.cpp
@@ +79,5 @@
> +}
> +
> +// Calls public static ByteBuffer wrap (byte[] array)
> +static jobject
> +JNI_Construct_ByteBuffer(JNIEnv* jenv, jbyteArray jBytes)

rather than wrapping the bytearray, you should allocate a ByteBuffer, then get access to its data directly to operate on  http://docs.oracle.com/javase/1.4.2/docs/guide/jni/jni-14.html#NewDirectByteBuffer

@@ +85,5 @@
> +    jclass jcls;
> +    jmethodID jmid;
> +    jobject jObj;
> +
> +    jcls = jenv->FindClass("java/nio/ByteBuffer");

these sorts of operations (getting classes and method ids) should be done once in an init function

@@ +143,5 @@
> +    dbPath = jenv->GetStringUTFChars(jDb, NULL);
> +
> +    jobject jNull = jenv->NewGlobalRef(NULL);
> +    jclass objectClass = jenv->FindClass("java/lang/Object");
> +    jclass stringClass = jenv->FindClass("java/lang/String");

init function

@@ +243,5 @@
> +                int colLen = f_sqlite3_column_bytes(ppStmt, i);
> +
> +                // byte[] = jbyteArray, but that isn't a real Object
> +                // so we need to use the ByteBuffer wrapper
> +                jbyteArray jBytes = jenv->NewByteArray(colLen);

construct a ByteBuffer and get direct access to its buffer with GetDirectBufferAddress

@@ +247,5 @@
> +                jbyteArray jBytes = jenv->NewByteArray(colLen);
> +                if (jBytes == NULL) {
> +                    goto error_close;
> +                }
> +                jenv->SetByteArrayRegion(jBytes, 0, colLen, (jbyte*)blob);

this will just be a memcpy
Comment 10 Mike Hommey [:glandium] 2012-01-17 23:54:34 PST
Comment on attachment 589331 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Review of attachment 589331 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/android/APKOpen.cpp
@@ +616,5 @@
> +  MOZLOAD("mozalloc");
> +  MOZLOAD("nspr4");
> +  MOZLOAD("plc4");
> +  MOZLOAD("plds4");
> +  sqlite_handle = MOZLOAD("mozsqlite3");

You shouldn't need to MOZLOAD anything other than mozsqlite3.
Note this is going to conflict with bug 683127, which I'm hoping to land this week.
Comment 11 Gian-Carlo Pascutto [:gcp] 2012-01-18 06:26:48 PST
Created attachment 589481 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

>why aren't you doing this delayed on the background thread anymore?

1) SQLiteLoadLibs needs to finish before GeckoLoadLibs starts, so those 2 need some kind synchronisation.
2) We already agreed that profile migration should be modal and blocking.

However, you're right that this ain't good - it will block the Java app loading even if profile migration doesn't need to run, and it will just show a blank until I get the splash screen for it landed. 

I moved it back to a thread, though it required some juggling because of (1).

>what is being thrown here? I'm not a fan of blanket catching Exception in general.

SQLiteConstraintExceptions (see bug 716729). I've narrowed it down now - it will go away eventually.

>rather than wrapping the bytearray, you should allocate a ByteBuffer, then get access to 
>its data directly to operate on  http://docs.oracle.com/javase/1.4.2/docs/guide/jni/jni-
>14.html#NewDirectByteBuffer
>construct a ByteBuffer and get direct access to its buffer with GetDirectBufferAddress

I implemented this as you suggested. However, this only works with direct ByteBuffers, which have no backing byte[]. Because the Java code does need that, it will end up copying everything again anyway. And the burden to do that has now shifted from the library code (once) to all users (every time). So this wasn't much of a win - had I known from the start I would have pushed back.

>Note this is going to conflict with bug 683127, which I'm hoping to land this week.

That sucks. This one needs to land too, it's a dataloss bug and it blocks bug 704682.
Comment 12 Gian-Carlo Pascutto [:gcp] 2012-01-18 06:28:34 PST
https://tbpl.mozilla.org/?tree=Try&rev=c65f6933a782
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2012-01-18 16:08:29 PST
(In reply to Gian-Carlo Pascutto (:gcp) from comment #11)
> Created attachment 589481 [details] [diff] [review]
> Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.
> 
> >why aren't you doing this delayed on the background thread anymore?
> 
> 1) SQLiteLoadLibs needs to finish before GeckoLoadLibs starts, so those 2
> need some kind synchronisation.
> 2) We already agreed that profile migration should be modal and blocking.
Let's not make this change here.

Also, this shouldn't block gecko from starting. Please put the migration check back to the delayed runnable.

 
> 
> However, you're right that this ain't good - it will block the Java app
> loading even if profile migration doesn't need to run, and it will just show
> a blank until I get the splash screen for it landed. 
> 
> I moved it back to a thread, though it required some juggling because of (1).
we should start the gecko thread, then check if we need to do a migration. If we need to do a migration we should put up a modal dialog, not block gecko from starting.
Comment 14 Gian-Carlo Pascutto [:gcp] 2012-01-18 16:11:56 PST
>Also, this shouldn't block gecko from starting. Please put the migration check 
>back to the delayed runnable.

The checkMigrateProfile is calling ProfileMigrator.launchBackground(), which does GeckoAppShell.getHandler().post(new PlacesTask());

This means it shouldn't be blocking Gecko, as that is at that moment starting in another thread. Right?
Comment 15 Brad Lassey [:blassey] (use needinfo?) 2012-01-18 16:21:57 PST
Comment on attachment 589481 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Review of attachment 589481 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoApp.java
@@ +1447,5 @@
>  
>          sGeckoThread = new GeckoThread(intent, mLastUri, mRestoreSession);
>          if (!ACTION_DEBUG.equals(intent.getAction()) &&
> +            checkAndSetLaunchState(LaunchState.Launching, LaunchState.Launched)) {
> +            new Thread(new LoadLibTask()).start();

this is going to impact start up time, please don't change this.

Instead, change loadSQLiteLibs() to ensureSQLiteLibsLoaded() and add a boolean variable to GeckoAppShell that sSQLiteLibsLoaded that says whether they need to be loaded or not. Protect the ensureSQLiteLibsLoaded function with a synchronised so two different threads don't try to load the libs.
Comment 16 Gian-Carlo Pascutto [:gcp] 2012-01-19 10:12:16 PST
Created attachment 589896 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Incorporated review comments. Added ByteBufferInputStream to avoid extra buffer copies.
Comment 17 Brad Lassey [:blassey] (use needinfo?) 2012-01-19 11:21:11 PST
Comment on attachment 589896 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

Review of attachment 589896 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoApp.java
@@ +1469,2 @@
>              sGeckoThread.start();
> +        }

remove the brackets

::: mobile/android/base/GeckoAppShell.java
@@ +420,5 @@
>          }
> +        return extractLibs;
> +    }
> +
> +    public synchronized static void ensureSQLiteLibsLoaded(String apkName) {

synchronizing on a static method uses the entire class as a lock.

Instead, make sSQLiteLibsLoaded a Boolean instead of a boolean and synchronize on that. 

To be clear, this should be: 
public static void ensureSQLiteLibsLoaded(String apkName) {
    if (sSQLiteLibsLoaded)
        return;
    synchronized(sSQLiteLibsLoaded) {
        if (sSQLiteLibsLoaded)
            return;
        loadSQLiteLibsNative(apkName, loadLibsSetup(apkName));
        sSQLiteLibsLoaded = true;
    }
}

::: mobile/android/base/GeckoThread.java
@@ +91,2 @@
>          GeckoAppShell.loadGeckoLibs(
>              app.getApplication().getPackageResourcePath());

not sure what the cost of app.getApplication().getPackageResourcePath() is, but just to be safe, only do it once and reuse the result

::: mobile/android/base/sqlite/ByteBufferInputStream.java
@@ +61,5 @@
> +        return mByteBuffer.get();
> +    }
> +
> +    @Override
> +    public int read(byte[] aBytes, int aOffset, int aLen) throws IOException {

I think both read methods need to be synchronized to be thread safe

::: mozglue/android/APKOpen.cpp
@@ +300,5 @@
> +SHELL_WRAPPER3(notifySmsDeleteFailed, jint, jint, jlong)
> +SHELL_WRAPPER2(notifyNoMessageInList, jint, jlong)
> +SHELL_WRAPPER8(notifyListCreated, jint, jint, jstring, jstring, jstring, jlong, jint, jlong)
> +SHELL_WRAPPER7(notifyGotNextMessage, jint, jstring, jstring, jstring, jlong, jint, jlong)
> +SHELL_WRAPPER3(notifyReadingMessageListFailed, jint, jint, jlong)

this patch is already big enough. Please split the cleaning up of these semi-colons into a separate patch. r=me on that clean up so it can land separately
Comment 19 Gian-Carlo Pascutto [:gcp] 2012-01-19 12:40:22 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cc27651eef8
Comment 21 Mike Hommey [:glandium] 2012-01-21 11:29:36 PST
FWIW, bug 683127 broke this, and this wasn't noticed until I got a crashing nightly in my hands, which means there are not tests for places migration.
Comment 22 Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-22 15:07:17 PST
:glandium, if this is broke again by another bug, should we reopen this bug?
Comment 23 Mike Hommey [:glandium] 2012-01-22 23:04:50 PST
No, bug 683127 was disabled.
Comment 24 Alex Keybl [:akeybl] 2012-01-23 09:19:14 PST
Comment on attachment 589896 [details] [diff] [review]
Patch 1. Add Bridge to use our own SQLite version. Update Profile Migrator.

[Triage Comment]
Mobile only - approved for Aurora.
Comment 25 Gian-Carlo Pascutto [:gcp] 2012-01-26 01:37:11 PST
I now understand the root cause for this:

http://www.sqlite.org/wal.html

WAL support was added in 3.7.0. We use WAL support in our own SQLite database engine. 

It is *not* available in pre-Honeycomb Android:
http://stackoverflow.com/questions/2421189/version-of-sqlite-used-in-android

SQLite documentation says:

http://www.sqlite.org/wal.html
Backwards Compatibility

...To prevent older versions of SQLite from trying to recover a WAL-mode database (and making matters worse) the database file format version numbers (bytes 18 and 19 in the database header) are increased from 1 to 2 in WAL mode. Thus, if an older version of SQLite attempts to connect to an SQLite database that is operating in WAL mode, it will report an error along the lines of "file is encrypted or is not a database".

This explains this bug: Android versions prior to HoneyComb will not be able to read the XUL database because it was written with WAL and they don't support that. Android versions post-Honeycomb (i.e. my Galaxy Tab) work fine as they have more recent SQLite's on board.

Note that the claimed test result on the Samsung Nexus S (Android 4.0.3) is inconsistent. Might that have been a mistake?
Comment 26 Mike Hommey [:glandium] 2012-01-26 23:59:35 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/063d3bf55986

Note You need to log in before you can comment on or make changes to this bug.