Closed Bug 785945 Opened 7 years ago Closed 7 years ago

Awesomescreen entries are very slow to appear

Categories

(Firefox for Android :: Awesomescreen, defect)

ARM
All
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 19
Tracking Status
firefox-esr10 --- verified
fennec 17+ ---

People

(Reporter: Margaret, Assigned: lucasr)

References

Details

(Keywords: perf, Whiteboard: ui-responsiveness)

Attachments

(9 files, 1 obsolete file)

I feel like awesomescreen performance on my default profile (on a Galaxy Nexus running JB) has gradually become worse, potentially because of an increasing history database.

This morning dbaron complained to me about performance on his phone this morning, and it took at least 5 seconds for the top sites tab to populate, and that's not acceptable. We should look into how to fix this.
(In reply to Margaret Leibovic [:margaret] from comment #0)
> I feel like awesomescreen performance on my default profile (on a Galaxy
> Nexus running JB) has gradually become worse, potentially because of an
> increasing history database.

We log the time it took to run the query on the content provider. See:

  https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/awesomebar/AllPagesTab.java#160

What's the time you see in logcat? Two things might be causing the perf problem:

1. The obvious one: the database query is too slow.
2. On startup, we're flooding the background thread with runnables that are executed before the runnable that actually runs the awesomescreen query.

I reason I think 2 is happening is because the query tends to get much faster once the Fennec is fully loaded. Maybe this is just a result of SQLite memcaching results. Needs investigation.

> This morning dbaron complained to me about performance on his phone this
> morning, and it took at least 5 seconds for the top sites tab to populate,
> and that's not acceptable. We should look into how to fix this.

Ouch. Is that on startup or after Fennec is fully loaded?
tracking-fennec: --- → ?
Keywords: perf
It varies, but it can be slow (Galaxy Nexus with JB). Here's a sample:
I/ALL_PAGES( 1633): Got cursor in 15593ms <-- after quitting/restarting
I/ALL_PAGES( 1633): Got cursor in 2828ms
I/ALL_PAGES( 1633): Got cursor in 2534ms
I/ALL_PAGES( 1633): Got cursor in 891ms
I/ALL_PAGES( 1633): Got cursor in 352ms
I/ALL_PAGES( 1633): Got cursor in 430ms
I/ALL_PAGES( 1633): Got cursor in 383ms
I/ALL_PAGES( 1633): Got cursor in 488ms
I/ALL_PAGES( 1633): Got cursor in 305ms
I/ALL_PAGES( 1633): Got cursor in 1373ms
I/ALL_PAGES( 1633): Got cursor in 661ms
I/ALL_PAGES( 1633): Got cursor in 1084ms
I/ALL_PAGES( 1633): Got cursor in 700ms
I/ALL_PAGES( 2508): Got cursor in 11133ms <-- after quitting/restarting
I/ALL_PAGES( 2508): Got cursor in 1046ms

Also, I was opening/closing the awesomescreen with logcat active, and I noticed that I would always see (at least) two messages being logged when opening the awesome screen. Looking at the code, it doesn't seem like that's expected, and I'm wondering if we could be accidentally issuing multiple queries (and if that would slow down the DB).

Playing around with a new profile, I'm seeing:
I/ALL_PAGES( 2731): Got cursor in 529ms <-- startup
I/ALL_PAGES( 2731): Got cursor in 20ms
I/ALL_PAGES( 2731): Got cursor in 49ms
I/ALL_PAGES( 2731): Got cursor in 7ms
I/ALL_PAGES( 2731): Got cursor in 40ms
I/ALL_PAGES( 2731): Got cursor in 29ms
I/ALL_PAGES( 2731): Got cursor in 7ms
I/ALL_PAGES( 2731): Got cursor in 13ms

This makes me think that a big history DB is probably what's really killing us. Maybe this is a reason to make bug 744961 more important. Or we should figure out how to optimize what we're doing right now.
Summary: Aweseomescreen entries are very slow to appear → Awesomescreen entries are very slow to appear
> 2. On startup, we're flooding the background thread with runnables that are
> executed before the runnable that actually runs the awesomescreen query.

Might I suggest that we log from inside run() with a per-runnable ID? Seems like an easy hypothesis to test how startup is being scheduled, possibly with some parallelism wins to be found.


(In reply to Margaret Leibovic [:margaret] from comment #2)

> Also, I was opening/closing the awesomescreen with logcat active, and I
> noticed that I would always see (at least) two messages being logged when
> opening the awesome screen. Looking at the code, it doesn't seem like that's
> expected, and I'm wondering if we could be accidentally issuing multiple
> queries (and if that would slow down the DB).

Add the object itself to the log output, see if it's the same AllPagesTab instance doing the logging -- maybe we're building the tab twice!

Failing that, perhaps something is causing a data change event that causes the cursor adapter to re-evaluate its filter, which will fetch a cursor again.

One way to tell is to attach a debugger and breakpoint there…


But yes, we should be spotting instances where it takes a ridiculous amount of time, or we run out of memory, and pruning history accordingly. We should attack both fronts, IMO.
Oh, another thing -- when we log that we grabbed a cursor, it should be cheap for us to also log how many rows Android thinks it has. That might be telling, even if we LIMIT the query.
Since our frecency code does date-sensitive calculations in its ORDER BY clause, it can't use an index, and must always do a full table scan (including frecency calculation for every row).

Desktop Firefox gets around this by storing the frecency score in the database and updating it on idle-daily.  However, this requires an expensive idle-daily job that we should try to avoid.  Bug 704025 proposes a frecency algorithm that is index-friendly without the need for daily updates.  Perhaps we should use this in Fennec.

If we can't use an index, then we will always slow down linearly as the number of history items and bookmarks grows, which means we will need to enforce strict limits on those numbers.
If a low risk patch comes in we can consider for Fx16, but that's doubtful.
tracking-fennec: ? → 17+
The suggestion I casually discussed with mconnor (who still remembers the implementation of desktop awesomebar) and margaret was to pre-limit on the important variables inside the query, such that we only bother doing the ORDER BY on a small set of rows.

E.g., grab the N most recent, and the M most frequent (both of which should already be indexed), then order and limit outside that.

The theory is that no row that isn't either recent or frequent is going to have a good chance of being frecent.

This should be straightforward to (a) hack together, and (b) experimentally verify.
Depends on: 789947
Assignee: nobody → margaret.leibovic
ph34r.

SELECT *,
       (visits * 
        MAX(1, (100 * 225) /
               (agedays * agedays + 225))) AS frecency
FROM (
SELECT *,
       ((date - strftime('%s000', 'now')) /
        (24 * 60 * 60 * 1000)) AS agedays FROM 
  (SELECT _id, guid, url, title, date, visits
   FROM history_with_images
   ORDER BY date DESC
   LIMIT 30)
UNION
SELECT *,
       ((date - strftime('%s000', 'now')) /
        (24 * 60 * 60 * 1000)) AS agedays FROM
 (SELECT _id, guid, url, title, date, visits
  FROM history_with_images
  ORDER BY visits DESC
  LIMIT 30)
)

ORDER BY frecency DESC;


Please excuse the annoying repetition. SQL doesn't allow self-reference between column aliases.

In English:

* Grab the top most recently visited pages;
* Grab the top most frequently visited pages;
* Merge them;
* Compute their age in days (surely we can do better with more granularity?);
* Compute the dynamic frecency on top, and sort.
Attached patch WIPSplinter Review
I tried to hack my debug build to use my default browser.db, but I couldn't get it to work (I always got an exception about the db being locked). Instead, I ran testBrowserProviderPerf with and without this patch, but it regressed, so I don't know if this is actually a good fix :(

Without patch: 398, 395
With patch: 767, 770

I also noticed that testBrowserProviderPerf only measures a query with a filter string, so I decided to tweak the test to run with an empty filter string. The numbers without the patch match the numbers I was seeing with my slow default profile, so that seems to indicate we made a good testcase. But unfortunately, the numbers with my patch are pretty terrible :(

Without patch: 1344, 1341
With patch: 2576, 2629
Attachment #664729 - Flags: feedback?(rnewman)
(In reply to Margaret Leibovic [:margaret] from comment #9)

> With patch: 2576, 2629

Getting rid of the "GROUP BY"s improved this to 2029, 2060, but we'd need to do something to keep that functionality somehow.
In case it helps with playing around with SQL, here's what the query looks like after all the concatenation (you'll have to replace ?s with filter strings):

SELECT *,
       ((CASE WHEN bookmark_id > -1 THEN 100 ELSE 0 END) +
        visits * MAX(1, (100 * 225) / (agedays * agedays + 225))) AS frecency
FROM (
SELECT *,
       ((date - strftime('%s000', 'now')) / 86400000) AS agedays FROM
  (SELECT _id,url,title,favicon,display,bookmark_id,history_id,bookmark_id,date,visits
   FROM combined_with_images
   WHERE (url LIKE ? OR title LIKE ?) ORDER BY date DESC LIMIT 100)
UNION
SELECT *,
       ((date - strftime('%s000', 'now')) / 86400000) AS agedays FROM
  (SELECT _id,url,title,favicon,display,bookmark_id,history_id,bookmark_id,date,visits
   FROM combined_with_images WHERE (url LIKE ? OR title LIKE ?)
   ORDER BY visits DESC LIMIT 100)
) ORDER BY frecency DESC;
Margaret: there are two things different in this query.

* We're not substituting System.currentTimeMillis(), so we're doing a bunch of strftime() calls.
* There's a lot more SQL to plan and parse.

So here are two things to try:

* Switch the timestamp calculation to concatenate the current timestamp into the string, just as we do in the existing query code.
* Define a view for each of the subqueries, see if that reduces SQL parsing overhead.

There's one additional point.

LIKE can't use an index.

That means that any query in which you expect to use string filtering won't be improved if you try to eliminate full table scans… y'can't, because SQLite already has to do a scan to implement LIKE.

We might only be able to get a win here for Top Sites -- use a low LIMIT and no LIKE.

(From the SQLite Optimization FAQ: "However, there are a number of cases in which an index can slow a query down: … If most or all of the table would need to be loaded anyway. (e.g. when an expression contains a LIKE operator)")
To be clear: the reason this query got slower is that we exchanged *one* full-table scan for *two* full-table scans. The reason it's quicker than running the query twice is that we do some of the work only on a small set of results.

If this were a full-fledged DB like Postgres, this is the point at which I'd suggest a trigram index.
Depends on: 794631
Comment on attachment 664729 [details] [diff] [review]
WIP

After some more investigation yesterday, we found that what's actually really killing us is our giant combined view. Just doing "SELECT <stuff we want> FROM combined_with_images ORDER BY date DESC LIMIT 100" was taking over 1000ms on my device. I'm going to investigate postponing the join on the images table until after we get the rows we want, and see if that improves things.
Attachment #664729 - Flags: feedback?(rnewman)
Just wanted to write this down: We talked previously about _not_ joining to favicons when doing the primary query. We would create the list of history/bookmarks with placeholder (or empty) images. As a row was being shown, we would start a runnable to load the favicon via the Favicon Java code, which could cache the favicons too.

This is similar to what Twitter does with avatars.
(In reply to Mark Finkle (:mfinkle) from comment #15)
> Just wanted to write this down: We talked previously about _not_ joining to
> favicons when doing the primary query. We would create the list of
> history/bookmarks with placeholder (or empty) images. As a row was being
> shown, we would start a runnable to load the favicon via the Favicon Java
> code, which could cache the favicons too.
> 
> This is similar to what Twitter does with avatars.

Margaret, I have a lot of code from Pattrn that I can provide as reference for this btw. Just let me know if you're interested :-)
Whiteboard: ui-responsiveness
My priorities are changing, so unfortunately I won't have time to continue looking into this right now.
Assignee: margaret.leibovic → nobody
Assignee: nobody → lucasr.at.mozilla
Attachment #674671 - Flags: review?(mark.finkle)
Attachment #674672 - Flags: review?(mark.finkle)
So, after quite a bit testing... I noticed that the presence of a large quantity of images in our DB can slow us down quite a bit. The problem obviously gets much worse when we store large images (I'm looking at you thumbnails).

So, as a first measure, we should not touch images on queries that we want to be fast. These patches change the "All Pages" tab in awesome screen to load favicon asynchronously. I'll do the same on about:home (where the images have even bigger impact).

It's hard to tell how much faster this will get—it's very annoying to test this kind of change as it involves DB schema upgrades and all. There's definitely some worthy gain as we're removing one table scan (from the images left join) and tons of large blobs from the query. We should probably land these in nightly and see how it goes.

These patches use LruCache which is available in Android's support library. For now, I'm simply copying LruCache into our repo. We can remove it once we get the build infra ready to use the support library.
Temporary copy just until we get built with Android's support library.
Attachment #674675 - Flags: review?(mark.finkle)
(In reply to Lucas Rocha (:lucasr) from comment #22)
> Created attachment 674675 [details] [diff] [review]
> Copy LruCache into our repo
> 
> Temporary copy just until we get built with Android's support library.

Please file a follow-up to track removing this, and mark it as a dependency of the support library. It's one of those things that gets forgotten!

(And I'd love to know the bug # for the support library availability; there's a few things from there that s-i would like to use.)

Ta!
That's in bug 759041. I can land that piece now if it helps the world.
Attachment #674670 - Flags: review?(mark.finkle) → review+
Attachment #674672 - Attachment is obsolete: true
Attachment #674672 - Flags: review?(mark.finkle)
Attachment #675191 - Flags: review?(mark.finkle)
Comment on attachment 675188 [details] [diff] [review]
Add a LRU memory cache to Favicons and corresponding API

I added some memory pressure bits so that we drop the favicon memory cache if we're under medium+ pressure. It would be nice if kats had a look.
Attachment #675188 - Flags: feedback?(bugmail.mozilla)
(In reply to Lucas Rocha (:lucasr) from comment #25)
> Created attachment 675188 [details] [diff] [review]
> Add a LRU memory cache to Favicons and corresponding API

Not entirely sure how to properly size this cache here. It needs to be big enough to hold a page of awesomescreen results (up to 100 entries now, I'll probably reduce it to 50). Any input here on this regard is welcome.
Comment on attachment 675188 [details] [diff] [review]
Add a LRU memory cache to Favicons and corresponding API

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

::: mobile/android/base/MemoryMonitor.java
@@ +155,5 @@
>              }
>              ScreenshotHandler.disableScreenshot(false);
>              GeckoAppShell.geckoEventSync();
> +
> +            GeckoApp.mAppContext.getFavicons().clearMemCache();

I'd rather put this line above the geckoEventSync() so that it can run in parallel with gecko's onLowMemory handling.

Also, in general, I like use SoftReference to hold on to caches like this since then dalvik can drop the cache whenever it decides it needs more memory, rather than going to the extreme of having to send out memory-pressure events. Depends on how tightly we want to hold on to the cache, I guess.
Attachment #675188 - Flags: feedback?(bugmail.mozilla) → feedback+
Comment on attachment 674671 [details] [diff] [review]
Add API to get multiple favicons at once from DB

Long term, I am on the fence about using a batch-like API for getting the favicons all at once. But I am willing to see how this works and tweak it over time.
Attachment #674671 - Flags: review?(mark.finkle) → review+
Attachment #674675 - Attachment is patch: true
Attachment #674675 - Flags: review?(mark.finkle) → review+
Comment on attachment 675188 [details] [diff] [review]
Add a LRU memory cache to Favicons and corresponding API

>diff --git a/mobile/android/base/MemoryMonitor.java b/mobile/android/base/MemoryMonitor.java

>+
>+            GeckoApp.mAppContext.getFavicons().clearMemCache();

We are trying to reduce usage of GeckoApp.mAppContext so we might want to think of other ways to do this in a followup bug.
Attachment #675188 - Flags: review?(mark.finkle) → review+
Comment on attachment 675190 [details] [diff] [review]
Load favicon images asynchronously in the All Pages tab

>+    private List<String> getUrlsWithoutFavicon() {

>+            Favicons favicons = GeckoApp.mAppContext.getFavicons();

Will uses of mAppContext fail if the device has "Kill activities" setting on?

>+    public void storeFaviconsInMemCache(Cursor c) {

>+            Favicons favicons = GeckoApp.mAppContext.getFavicons();

Same
Attachment #675190 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #32)
> Comment on attachment 674671 [details] [diff] [review]
> Add API to get multiple favicons at once from DB
> 
> Long term, I am on the fence about using a batch-like API for getting the
> favicons all at once. But I am willing to see how this works and tweak it
> over time.

Two main reasons I implemented this using a batch-line API:

1. We touch the disk only once for the current result set in awesome screen (note that we only fetch the favicons from DB which are not in the memory cache yet)
2. Favicons are displayed immediately as you scroll the current result set (as opposed to a visible delay while scrolling, in case we loaded the images as you scroll)

In other words: we get the benefit of loading all the images asynchronously without continuously touching the disk nor having a strange delay to show favicons as you scroll the list.
Just some (non-scientific) benchmark results. This is based on the averaged telemetry data after running Fennec several times locally. I have a medium-sized database with a good number of favicons and thumbnails. The perf gain might be bigger on larger databases.

awesomescreen (time to show initial results):
 * Without the patches: 201.25ms
 * With the patches: 128.25ms
 * Result: ~36% faster

about:home (time to show top sites):
 * Without the patches: 1854.6ms
 * With the patches: 1236.2ms
 * Result: ~33% faster

So, this is already a substantial performance gain. I'll experiment with the ideas described in comment #8 to see how much faster we can get with a different query.
Are there try builds(In reply to Lucas Rocha (:lucasr) from comment #36)
> awesomescreen (time to show initial results):
>  * Without the patches: 201.25ms
>  * With the patches: 128.25ms
>  * Result: ~36% faster
> 
> about:home (time to show top sites):
>  * Without the patches: 1854.6ms
>  * With the patches: 1236.2ms
>  * Result: ~33% faster

Are there try builds for Fennec? My HTC One V shows waaay slower times using the nightly than what you see.
Is it possible to determine the size of my DB (that is synced over from desktop), i.e. number of history records, favicons, thumbnails?
Attachment #675518 - Flags: review?(mark.finkle) → review+
Attachment #675191 - Flags: review?(mark.finkle) → review+
Comment on attachment 675193 [details] [diff] [review]
Load top site thumbnails asynchronously in about:home

>diff --git a/mobile/android/base/resources/values/styles.xml b/mobile/android/base/resources/values/styles.xml

>-      <item name="android:scaleType">centerCrop</item>
>+      <item name="android:scaleType">fitCenter</item>

What's the reason for this change?
Attachment #675193 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #39)
> Comment on attachment 675193 [details] [diff] [review]
> Load top site thumbnails asynchronously in about:home
> 
> >diff --git a/mobile/android/base/resources/values/styles.xml b/mobile/android/base/resources/values/styles.xml
> 
> >-      <item name="android:scaleType">centerCrop</item>
> >+      <item name="android:scaleType">fitCenter</item>
> 
> What's the reason for this change?

Making images load asynchronously exposed this bug. The default image has to be center, no stretched in the thumbnail area. This wasn't visible before because we were replacing it immediately. Now this default image is visible until the DB query returns.
(In reply to Mark Finkle (:mfinkle) from comment #33)
> Comment on attachment 675188 [details] [diff] [review]
> Add a LRU memory cache to Favicons and corresponding API
> 
> >diff --git a/mobile/android/base/MemoryMonitor.java b/mobile/android/base/MemoryMonitor.java
> 
> >+
> >+            GeckoApp.mAppContext.getFavicons().clearMemCache();
> 
> We are trying to reduce usage of GeckoApp.mAppContext so we might want to
> think of other ways to do this in a followup bug.

I could add something like GeckoAppShell.getFavicons() but not sure if this would be an improvement.
Here's another update.

Margaret was using combined_with_images in the query suggested in comment #8 query which explains the performance hit. The query is supposed to be run on the history tablet instead. So, I tried several variations of the ideas in comment #8 but my conclusion is that there's no relevant performance gain from it. It would probably involve a more complex query setup in our ContentProvider which is not worth it (considering the performance results).

Given that we'll be expiring history entries quite aggressively now (see bug 744961), I think should just land these patches here and see how goes. I think the combination of less history entries and a query that doesn't touch images will give as a substantial performance boost both on awesomescreen and about:home.
(In reply to Mark Finkle (:mfinkle) from comment #34)
> Comment on attachment 675190 [details] [diff] [review]
> Load favicon images asynchronously in the All Pages tab
> 
> >+    private List<String> getUrlsWithoutFavicon() {
> 
> >+            Favicons favicons = GeckoApp.mAppContext.getFavicons();
> 
> Will uses of mAppContext fail if the device has "Kill activities" setting on?

Nice catch. Will test this before pushing.
Might I suggest an approach similar to MemoryMonitor.StorageReducer? That is, chain a context through to wherever the calling code or the persistent object are created?

Generally I would advise against pushing logic into GeckoAppShell just because it requires a context.
(In reply to Mark Finkle (:mfinkle) from comment #34)
> Comment on attachment 675190 [details] [diff] [review]
> Load favicon images asynchronously in the All Pages tab
> 
> >+    private List<String> getUrlsWithoutFavicon() {
> 
> >+            Favicons favicons = GeckoApp.mAppContext.getFavicons();
> 
> Will uses of mAppContext fail if the device has "Kill activities" setting on?

FYI: Tested this with "Don't keep activity" on and it works fine. I've filed bug 807279 to add an API to access Favicons that doesn't require mAppContext.
As philor discussed on irc, we're seeing exceptions during robocop tests, which might be caused by this push. Here's a sample:

I/Robocop ( 9062): 5 INFO TEST-PASS | testHistory | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_blank_02.html should equal http://mochi.test:8888/tests/robocop/robocop_blank_02.html
D/Robocop ( 9062): waiting for DOMContentLoaded
D/AndroidRuntime( 9062): Shutting down VM
E/GeckoAppShell( 9062): >>> REPORTING UNCAUGHT EXCEPTION FROM THREAD 1 ("main")
E/GeckoAppShell( 9062): android.database.StaleDataException: Access closed cursor
E/GeckoAppShell( 9062): 	at android.database.AbstractWindowedCursor.checkPosition(AbstractWindowedCursor.java:217)
E/GeckoAppShell( 9062): 	at android.database.AbstractWindowedCursor.getString(AbstractWindowedCursor.java:41)
E/GeckoAppShell( 9062): 	at android.database.CursorWrapper.getString(CursorWrapper.java:135)
E/GeckoAppShell( 9062): 	at android.database.CursorWrapper.getString(CursorWrapper.java:135)
E/GeckoAppShell( 9062): 	at org.mozilla.gecko.AllPagesTab.getUrlsWithoutFavicon(AllPagesTab.java:738)
E/GeckoAppShell( 9062): 	at org.mozilla.gecko.AllPagesTab.loadFaviconsForCurrentResults(AllPagesTab.java:779)
E/GeckoAppShell( 9062): 	at org.mozilla.gecko.AllPagesTab.access$1700(AllPagesTab.java:61)
E/GeckoAppShell( 9062): 	at org.mozilla.gecko.AllPagesTab$AllPagesHandler.handleMessage(AllPagesTab.java:840)
E/GeckoAppShell( 9062): 	at android.os.Handler.dispatchMessage(Handler.java:99)
E/GeckoAppShell( 9062): 	at android.os.Looper.loop(Looper.java:123)
E/GeckoAppShell( 9062): 	at android.app.ActivityThread.main(ActivityThread.java:4627)
E/GeckoAppShell( 9062): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell( 9062): 	at java.lang.reflect.Method.invoke(Method.java:521)
E/GeckoAppShell( 9062): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:868)
E/GeckoAppShell( 9062): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:626)
E/GeckoAppShell( 9062): 	at dalvik.system.NativeStart.main(Native Method)
I/WindowManager( 1019): WIN DEATH: Window{485a22f0 org.mozilla.fennec/org.mozilla.fennec.App paused=false}
I/Process ( 1019): Sending signal. PID: 9130 SIG: 9
My guess: you've got a background task that loads the favicons, but you've already closed the AllPagesTab, and its mCursorAdapter has been destroyed, which closes -- but does not release the reference to! -- its inner cursor.

Basically you're racing the cleanup of the cursor.

You should do one or all of the following:

* Check isClosed before accessing the cursor.
* Discard the handle to the cursor after you close it. (Still subject to a race.)  Now you'll be checking for null.
* Subscribe to the invalidation event in the dataset observer, and recreate the cursor appropriately.
* Kill the background task that would use the cursor when the cursor adapter is 'invalidated'.
Note that the common solution is to c.requery(), but that's deprecated.
Unfortunately, this was backed out due to Android bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2301d07ca953
Ok, this change fixes the issue (cancel any pending task to update favicons when the view is destroyed):

--- a/mobile/android/base/awesomebar/AllPagesTab.java
+++ b/mobile/android/base/awesomebar/AllPagesTab.java
@@ -151,16 +151,19 @@ public class AllPagesTab extends Awesome
         unregisterEventListener("SearchEngines:Data");
         if (adapter == null) {
             return;
         }
 
         Cursor cursor = adapter.getCursor();
         if (cursor != null)
             cursor.close();
+
+        mHandler.removeMessages(MESSAGE_UPDATE_FAVICONS);
+        mHandler.removeMessages(MESSAGE_LOAD_FAVICONS);
     }
 
     public void filter(String searchTerm) {
         AwesomeBarCursorAdapter adapter = getCursorAdapter();
         adapter.filter(searchTerm);
 
         filterSuggestions(searchTerm);
         if (mSuggestionsOptInPrompt != null) {
Depends on: 808027
Depends on: 808029
Duplicate of this bug: 719471
Duplicate of this bug: 708197
Awesomescreen entries are displayed immediately on the latest Nightly. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-12-06)
Device: Galaxy Tab2 7"
OS: Android 4.0.3
Status: RESOLVED → VERIFIED
Blocks: 833757
Depends on: 819973
Depends on: 834536
Depends on: 838440
You need to log in before you can comment on or make changes to this bug.