789947 - Setting up search engines creates an extra top sites filter query

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[:Margaret Leibovic]

Spawning this off from bug 785945...

(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.

I added some logging, and I found that the FilterQueryProvider is the one running runQuery() twice when first opening the awesome screen. I found that this is because we're calling AllPagesTab.filter() twice. The first time we're calling it from AwesomeBarTabs.filter(), and then we're calling it again from inside setSearchEngines:
http://hg.mozilla.org/mozilla-central/annotate/1cb30394aa56/mobile/android/base/awesomebar/AllPagesTab.java#l484

I understand that we want to update the list with the search engines, but I think we should factor out that logic from kicking off a new db query.

Comment 1

[:Margaret Leibovic]

Attachment: patch

Comment 2

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a71b19fafcbe

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a71b19fafcbe

Comment 4

[Cristian Nicolae (:xti)]

Changes had landed on the latest Nightly. Closing bug as verified fixed.

--
Firefox 18.0a1 (2012-09-18)
Device: Galaxy Note
OS: Android 4.0.4

Comment 5

[:Margaret Leibovic]

Comment on attachment 660374 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: unnecessary perf cost (run extra top sites query when the awesome screen is opened)
Testing completed (on m-c, etc.): landed on m-c 9/13
Risk to taking this patch (and alternatives if risky): low risk refactoring
String or UUID changes made by this patch: n/a

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 660374 [details] [diff] [review]
patch

Looks like this would be painful enough to users that we should uplift, approving.

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6a76216bc0ff