Convert autocomplete feedback increase to an async query

VERIFIED FIXED in Firefox 3.1b3

Status

()

P2
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

({perf, verified1.9.1})

Trunk
Firefox 3.1b3
perf, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Feedback increase on autocomplete selection can be done async, the only draawback is that we need to change the test to poll the db for the actual input change. so in 
  nsNavHistory::AutoCompleteFeedback
do
  nsCOMPtr<mozIStoragePendingStatement> canceler;
  rv = statement->ExecuteAsync(nsnull, getter_AddRefs(canceler));
(Assignee)

Updated

10 years ago
Assignee: nobody → mak77
Summary: Do feedback increase async in autocomplete → Do autocomplete feedback increase an async query
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

10 years ago
Created attachment 343405 [details] [diff] [review]
patch

the biggest change is the test, as discussed on IRC i'm polling the db every 100ms to see if all data have been written. This is somehow complex but should work, i did not get any test fail with this patch. We will most likely have to update the test if the rank calculation algo changes.
Attachment #343405 - Flags: review?(dietrich)
(Assignee)

Comment 2

10 years ago
oh, the scope is not locking the UI when a user chooses an item from the autocomplete dropdown.

patch built upon fsync stuff, will not apply cleanly on actual tree
Attachment #343405 - Flags: review?(dietrich) → review+
Comment on attachment 343405 [details] [diff] [review]
patch

looks good, r=me. however, it looks like the only dependency on fsync code is the query to confirm use-count updates. if that's so, then please change that so we can land this now. and then update the test in the fsync patch.
(Assignee)

Comment 4

10 years ago
(In reply to comment #3)
> (From update of attachment 343405 [details] [diff] [review])
> looks good, r=me. however, it looks like the only dependency on fsync code is
> the query to confirm use-count updates. if that's so, then please change that
> so we can land this now. and then update the test in the fsync patch.

since fsync is near i'll wait to see, if that is backed out again i'll push this without the temp table, and change it later.
(Assignee)

Comment 5

10 years ago
hwv there is another dependancy on frecency stuff, the feedback increase function has changes as part of the Ts cleanup
(Assignee)

Updated

10 years ago
Whiteboard: [has patch][has review]
Keywords: perf
Priority: -- → P2
Target Milestone: --- → Firefox 3.1
(Assignee)

Updated

10 years ago
Summary: Do autocomplete feedback increase an async query → Convert autocomplete feedback increase to an async query
(Assignee)

Comment 6

10 years ago
pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/405b440b26d9
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Comment 7

10 years ago
Comment on attachment 343405 [details] [diff] [review]
patch

asking approval 1.9.1

This is another move in the direction of "try to not lock the UI after a user action", patch comes with updated test, medium risk.
Attachment #343405 - Flags: approval1.9.1?
(Assignee)

Updated

10 years ago
Depends on: 468590
Attachment #343405 - Flags: approval1.9.1? → approval1.9.1+
(Assignee)

Comment 9

10 years ago
Created attachment 352299 [details] [diff] [review]
unbitrot

this is the patch that was pushed (unbitrotted)
Attachment #343405 - Attachment is obsolete: true
(Assignee)

Updated

10 years ago
Depends on: 468400
(Assignee)

Comment 10

10 years ago
pushed to 1.9.1 branch:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9c5c023fbd98
Keywords: fixed1.9.1
Target Milestone: Firefox 3.1 → Firefox 3.1b3
Whiteboard: [has patch][has review]
verified per checkin and in-testsuite+
Status: RESOLVED → VERIFIED

Updated

10 years ago
Keywords: fixed1.9.1 → verified1.9.1
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.