Closed Bug 789947 Opened 7 years ago Closed 7 years ago

Setting up search engines creates an extra top sites filter query

Categories

(Firefox for Android :: Awesomescreen, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 18
Tracking Status
firefox17 --- verified
firefox18 --- verified

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file)

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.
Attached patch patchSplinter Review
Assignee: nobody → margaret.leibovic
Attachment #660374 - Flags: review?(bnicholson)
Attachment #660374 - Flags: review?(bnicholson) → review+
https://hg.mozilla.org/mozilla-central/rev/a71b19fafcbe
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
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
Attachment #660374 - Flags: approval-mozilla-aurora?
Comment on attachment 660374 [details] [diff] [review]
patch

Looks like this would be painful enough to users that we should uplift, approving.
Attachment #660374 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.