609603 - Search providers in the awesomescreen results are slow to update

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Steps to reproduce:
1) Open the awesomescreen and type something with no matches (like "hghghghg").
2) Wait for the search providers to show up.
3) Type another letter (so the new query is e.g. "hghghghga").

Each time you type a new letter, it takes up to 2-3 seconds for the search provider rows to update.  The delay might depend on your profile size; I tested this on Android with a fully-synced profile (around 13MB of data).

Based on logging, all of the delay is spent waiting for PACS.startSearch to finish returning results.

Maybe we can improve this by assuming that if the old query returned zero results *and* is a prefix of the new query, then the new query will return zero results too.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch v1

This implements what is mentioned in comment 1, and cleans up a few other things. We compare the current query to the previous query. If they match except for characters at the end, and if the number of results in the previous query equals the number of search engines we have, we just reuse the previous query (PACS should be doing something like this anyway?).

I wasn't seeing the slowness on device Matt, but my profile is smaller than the one you are using. I'll try to reproduce and see if this helps, but if you want to apply the patch and try too, would be appreciated.

Comment 2

[Matt Brubeck (:mbrubeck)]

Comment on attachment 488228 [details] [diff] [review]
Patch v1

Yep, this fixes the slowness for me.  There's still a delay before the search engines first appear (while the PACS is still looking for matches), but afterwards they update quickly as I type.  (There's still a delay when I backspace, but that's expected and would take more work to fix.)

But when I actually tap on the awesomescreen row, it searches for the first query that I typed (without any additional characters I typed after the results appeared).

One other note:

>diff --git a/components/AutoCompleteCache.js b/components/AutoCompleteCache.js
>+      let prevRegEx = new RegExp("^" + prev.searchString);
>+      if (prev.matchCount == this.searchEngines.length && query.match(prevRegEx)) {

Building a regexp like this will not work right if the searchString contains special characters like [*+?].  I would use "query.indexOf(prev.searchString) == 0" instead.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Thanks for catching that Matt. I want to stare at this for a bit longer, but here's a revised patch that WFM.

We have to regenerate this list of search provider results in order to update the attached urls. To fix it, I moved all of object used for cachedResults into a new object too...

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

I don't see this and I don't to add code for something that is not a real problem.

Reopen if we see the slowdown

Comment 5

[Matt Brubeck (:mbrubeck)]

I made a video using today's nightly build: http://vimeo.com/16706473

Sorry for the blurrycam and background noise, but I think you can get the idea.  Basically, this feature is not usable at all after I've synced my normal profile.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Another approach would be to always add have the search providers in the list, not added every time, but semi-permanent. If the search engines change in any way, we get notified and could update the semi-fixed rows.

Comment 7

[Wesley Johnston (:wesj)]

The autocomplete controller handles both keypresses, and clicks in the list. Adding richlistitems with adding them to the result set will lead to unpredictable results.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

I am having second thoughts about 2.0-, so I am marking it 2.0+

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Patch v_

Not changed to much, but works fine here.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - I don't think we landed this yet

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Actually, we did:
http://hg.mozilla.org/mobile-browser/rev/c6499027bb47

Comment 12

[Aakash Desai [:aakashd]]

verified FIXED On build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20100104 Namoroka/4.0b9pre Fennec/4.0b4pre