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

RESOLVED FIXED in Firefox 21

Status

()

Toolkit
Places
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: emorley, Assigned: mak)

Tracking

(Blocks: 1 bug, {intermittent-failure})

Trunk
mozilla21
x86
Windows 7
intermittent-failure
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox20 affected, firefox21 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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
}
(Assignee)

Comment 3

6 years ago
to be investigated
Assignee: nobody → mak77
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 34

6 years ago
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
(Assignee)

Updated

6 years ago
Blocks: 832616
(Assignee)

Updated

6 years ago
Blocks: 832617
(Assignee)

Comment 35

6 years ago
Created attachment 704220 [details] [diff] [review]
patch v1.0
Attachment #704220 - Flags: review?(mano)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 39

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Can we uplift this to Aurora too?
status-firefox20: --- → affected
status-firefox21: --- → fixed
(Assignee)

Comment 45

6 years ago
(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.
(Assignee)

Comment 46

6 years ago
rs=me on test disable, if you want to do that.
Comment hidden (Treeherder Robot)
You need to log in before you can comment on or make changes to this bug.