We can avoid bug 412730 but still do some optimization by only using *finished* results with no possibility of there being more results. This means we haven't hit maxResults and the search finished. We only need to look through the urls from the previous search while making sure not to kill keyword results and stuff.
Created attachment 308943 [details] [diff] [review] v1 Definitely won't be able to apply this without bug 422490.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #308943 - Flags: review?(dietrich)
FYI, this even works nicely with special searches from bug 395161. Adaptive results still do their thing and keyword results keep updating.
Created attachment 309227 [details] [diff] [review] v1.1 Switched to a nsStringArray to keep things sane ;) Too bad.. no more triple pointer, dereference, postincrement, dereference, assign address magic! :p >+ // The void * argument is a pointer to a nsString * array. We post-increment >+ // the contents of the pointer so that it points to the next nsString * >+ // element for the next enumeration. For the current nsString *, assign it >+ // the address of the current hashtable entry's string. >+ *(*(const nsAString ***)aArg)++ = &aKey;
Oh, forgot the main reason for saneness. Apparently compiling on windows didn't support C99 ability for variable length stack arrays.
This will not block the final release of Firefox 3.
Flags: blocking-firefox3? → blocking-firefox3-
Bug 422490 was a step to get this patch in, which is another step towards bug 422177 which is a P2 blocker. I'll unbitrot this patch from moving it to the top of my stack above the special sauce bug 395161 and keyword magic bug 392143.
Flags: blocking-firefox3- → blocking-firefox3?
Priority: -- → P2
Whiteboard: [has patch][need review dietrich][needed for 422177]
Created attachment 311021 [details] [diff] [review] v1.2 Moved to the top of my stack and pulled in some changes from bug 395161 because we need to use that query fragment in multiple places.
Comment on attachment 311021 [details] [diff] [review] v1.2 r=me
Attachment #311021 - Flags: review?(dietrich) → review+
Comment on attachment 311021 [details] [diff] [review] v1.2 Making things go faster and needed for bug 422177 which makes things go really fast.
Whiteboard: [has patch][need review dietrich][needed for 422177] → [has patch][needed for 422177]
Comment on attachment 311021 [details] [diff] [review] v1.2 a=mconnor on behalf of 1.9 drivers
To see this working, you need to type in some search terms that result in ~5 results. Have the search throbber finish spinning, then type a letter that matches in fewer than 5 of those results. It should show the results right away instead of showing nothing and an activity throbber. Checking in toolkit/components/places/src/nsNavHistory.h; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v <-- nsNavHistory.h new revision: 1.151; previous revision: 1.150 done Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v <-- nsNavHistoryAutoComplete.cpp new revision: 1.50; previous revision: 1.49 done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][needed for 422177]
Target Milestone: --- → Firefox 3 beta5
Created attachment 311225 [details] [diff] [review] leak bustage fix Apparently bad stuff happens with a global const nsCString reference. Converted it to a #define. Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v <-- nsNavHistoryAutoComplete.cpp new revision: 1.52; previous revision: 1.51 done We were leaking 8 bytes.. Error: Leak Test Failed: Number of leaks 8 is greater than LeakFailureThreshold 0 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1206254100.1206254570.32461.gz
FYI, the leak was reported here.. |<----------------Class---------------> nsStringBuffer |<-----Bytes------> Per-Inst Leaked 8 8 |<----------------Objects----------------> Total Rem Mean StdDev 32460 1 ( 4330.68 +/- 2367.57) |<--------------References-------------->| Total Rem Mean StdDev 59418 1 ( 4792.61 +/- 2814.15)
This is pretty obfuscated code here, especially the #define above the former const ref. It's also I think doing scary thing with temporaries for the strings, and could break depending on how the final #define is being used. This really just wants to be a nsPrintfCString for the entire thing in some Init function.. maybe with multiple nsPrintfCStrings for the fragments that are now being done via string pasting in the #define.
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: Firefox 3 beta5 → Firefox 3
Target Milestone: Firefox 3 → Firefox 3 beta5
added this expectation to test case https://litmus.mozilla.org/manage_testcases.cgi?testcase_id=8955
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.