Closed Bug 824074 Opened 8 years ago Closed 8 years ago

Properly update frecency sorting in bookmarks queries. (was: Intermittent places/tests/unit/test_result_sort.js | 9 == 8)

Categories

(Toolkit :: Places, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21
Tracking Status
firefox20 --- affected
firefox21 --- fixed

People

(Reporter: emorley, Assigned: mak)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Rev3 WINNT 6.1 mozilla-inbound debug test xpcshell on 2012-12-21 03:30:11 PST for push b323378d9d27

slave: talos-r3-w7-053

https://tbpl.mozilla.org/php/getParsedLog.php?id=18161074&tree=Mozilla-Inbound

{
TEST-PASS | c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js | [checkOrder : 60] 8 == 8

TEST-PASS | c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js | [checkOrder : 58] 7 == 7

TEST-PASS | c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js | [checkOrder : 59] 9 == 9

TEST-PASS | c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js | [checkOrder : 60] 8 == 8

TEST-INFO | (xpcshell/head.js) | test 2 finished

TEST-INFO | (xpcshell/head.js) | running event loop

TEST-UNEXPECTED-FAIL | c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js | 9 == 8 - See following stack:
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: do_throw :: line 461
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: _do_check_eq :: line 555
JS frame :: c:\talos-slave\test\build\xpcshell\head.js :: do_check_eq :: line 576
JS frame :: c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js :: checkOrder :: line 58
JS frame :: c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/test_result_sort.js :: <TOP_LEVEL> :: line 117
JS frame :: resource://gre/modules/commonjs/promise/core.js :: effort :: line 53
JS frame :: resource://gre/modules/commonjs/promise/core.js :: resolveDeferred :: line 125
JS frame :: resource://gre/modules/commonjs/promise/core.js :: then :: line 34
JS frame :: resource://gre/modules/commonjs/promise/core.js :: resolve :: line 167
JS frame :: c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/unit/head_bookmarks.js -> file:///c:/talos-slave/test/build/xpcshell/tests/toolkit/components/places/tests/head_common.js :: .handleCompletion :: line 654
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
}
to be investigated
Assignee: nobody → mak77
There is a real bug here, we don't live update correctly the frecency in bookmarks queries. I have a patch that has a couple defects yet, though I'm not willing to handle those here, so I will file follow-ups:
1. onDeleteVisits we still don't do the right thing
2. if we'd pass frecency along with onItemVisited we may save a query, but then I wonder if we may have to do the same in onVisit and if this enforces us to do an additional query to fetch frecency after we set it.

There is an alternative that would be to consider a bookmarks query sorted by frecency a complex query, thus forcing a refresh(), but I'm less prone to whole refreshes nowadays.
Status: NEW → ASSIGNED
Blocks: 832616
Blocks: 832617
Attached patch patch v1.0Splinter Review
Attachment #704220 - Flags: review?(mano)
https://hg.mozilla.org/integration/mozilla-inbound/rev/f67a97dda10d
Flags: in-testsuite+
Summary: Intermittent places/tests/unit/test_result_sort.js | 9 == 8 → Properly update frecency sorting in bookmarks queries. (was: Intermittent places/tests/unit/test_result_sort.js | 9 == 8)
Target Milestone: --- → mozilla21
https://hg.mozilla.org/mozilla-central/rev/f67a97dda10d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Can we uplift this to Aurora too?
(In reply to Ryan VanderMeulen from comment #44)
> Can we uplift this to Aurora too?

This also touches Places results code.  While the change is not risky, it doesn't bring specific wins to the users, thus I'm not sure there are the requirements for an uplift.  I think would be safer to just disable the test in Aurora if it's too noisy.
rs=me on test disable, if you want to do that.
You need to log in before you can comment on or make changes to this bug.