Closed Bug 904081 Opened 8 years ago Closed 8 years ago

[FIG] Figgy Fennec crashing after Sync is setup

Categories

(Firefox for Android Graveyard :: General, defect, P1)

26 Branch
ARM
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: ibarlow, Assigned: lucasr)

References

Details

(Keywords: crash, Whiteboard: [fixed-fig])

Crash Data

Attachments

(1 file)

STR:
- Install Figgy Fennec
- Set up Sync
- Wait a minute or two for bookmarks to sync over
- Open FF
- FF crashes on startup

Crash log: http://cl.ly/0l3V3c0S2U1v
Seems like the relevant piece is:

E/SQLiteLog(12301): (1) Expression tree is too large (maximum depth 1000)
W/dalvikvm(12301): threadid=20: thread exiting with uncaught exception (group=0x415cf700)
E/GeckoAppShell(12301): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 3100 ("ModernAsyncTask #4")
E/GeckoAppShell(12301): android.database.sqlite.SQLiteException: Expression tree is too large (maximum depth 1000) (code 1): , while compiling: SELECT url, favicon FROM combined_with_favicons WHERE (url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = ? OR url = 
E/GeckoAppShell(12301): Main thread stack:
E/GeckoAppShell(12301): android.view.GLES20Canvas.nDrawDisplayList(Native Method)
E/GeckoAppShell(12301): android.view.GLES20Canvas.drawDisplayList(GLES20Canvas.java:390)
E/GeckoAppShell(12301): android.view.HardwareRenderer$GlRenderer.drawDisplayList(HardwareRenderer.java:1487)
E/GeckoAppShell(12301): android.view.HardwareRenderer$GlRenderer.draw(HardwareRenderer.java:1371)
E/GeckoAppShell(12301): android.view.ViewRootImpl.draw(ViewRootImpl.java:2367)
E/GeckoAppShell(12301): android.view.ViewRootImpl.performDraw(ViewRootImpl.java:2239)
E/GeckoAppShell(12301): android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1872)
E/GeckoAppShell(12301): android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1004)
E/GeckoAppShell(12301): android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5481)
E/GeckoAppShell(12301): android.view.Choreographer$CallbackRecord.run(Choreographer.java:749)
E/GeckoAppShell(12301): android.view.Choreographer.doCallbacks(Choreographer.java:562)
E/GeckoAppShell(12301): android.view.Choreographer.doFrame(Choreographer.java:532)
E/GeckoAppShell(12301): android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:735)
E/GeckoAppShell(12301): android.os.Handler.handleCallback(Handler.java:730)
E/GeckoAppShell(12301): android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoAppShell(12301): android.os.Looper.loop(Looper.java:137)
E/GeckoAppShell(12301): android.app.ActivityThread.main(ActivityThread.java:5103)
E/GeckoAppShell(12301): java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell(12301): java.lang.reflect.Method.invoke(Method.java:525)
E/GeckoAppShell(12301): com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
E/GeckoAppShell(12301): com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
E/GeckoAppShell(12301): dalvik.system.NativeStart.main(Native Method)

I'm seeing successful sync's in there (search for FxSync log tag).
Here is a crash ID: bp-1868ae51-fc36-4ee5-b622-abf8b2130812.
Crash Signature: [@ android.database.sqlite.SQLiteException: at android.database.sqlite.SQLiteConnection.nativePrepareStatement(Native Method) ]
Keywords: crash
OS: Mac OS X → Android
Hardware: x86 → ARM
Priority: -- → P1
Assignee: nobody → lucasr.at.mozilla
Attachment #790814 - Flags: review?(margaret.leibovic)
Comment on attachment 790814 [details] [diff] [review]
Avoid exceeding query var limit in getFaviconsForUrls()

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

Nice.
Attachment #790814 - Flags: review?(margaret.leibovic) → review+
I bit of context here: getFaviconsForUrls() was originally written with a very specific use case in mind (favicons in the awesomescreen search). This was a very controlled use case because there was a maximum number of items (100).

With the new code, we're abusing it by using it in lists that can have an arbitrary number of items (e.g. the bookmarks list). Which is why getFaviconsForUrls() is blowing up after setting up sync. This patch is just a band-aid. It will avoid the crash when we're fetching a large number of favicons. However...

Our current approach for loading favicons is very fragile for a number of reason:

1. It assumes the favicon mem cache will be able to hold all favicons for the list, which is not necessarily the case (especially for long lists).
2. It's not very memory efficient: we'll be allocating a huge array of strings just to run the query and, more importantly, a huge list of images to put in the memcache (which will then be discarded by the memcache just after).

The right thing to do here is to load favicons asynchronously and incrementally. I recommend using a little library called Smoothie which I wrote as a standalone open source project:

  https://github.com/lucasr/smoothie

It provides a simple API to do exactly what we want here. Also, it supports pre-fetching offscreen items so that we don't show temporary placeholders too often as you scroll. In any case, I filed bug 905685 to tackle a proper implementation for favicon loading.
Pushed: http://hg.mozilla.org/projects/fig/rev/a7454660ca73
Whiteboard: [fixed-fig]
https://hg.mozilla.org/mozilla-central/rev/a7454660ca73
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.