Closed Bug 1678619 Opened 4 years ago Closed 4 years ago

Convert frecency notifications to a PlacesEvent

Categories

(Toolkit :: Places, task, P3)

Product:
Toolkit
Component:
Places
Type:
task
Points:
3

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Iteration:
85.2 - Nov 30 - Dec 13
Tracking Flags:
Tracking Status
firefox86 --- fixed

People

(Reporter: mak, Assigned: daisuke)

References

Details

Attachments

(6 files, 6 obsolete files)

Bug 1678619: Replace onFrecencyChanged with onManyFrecenciesChanged.
(deleted), text/x-phabricator-request
Details
Bug 1678619: Remove onFrecencyChanged interface from nsINavHistoryService.
(deleted), text/x-phabricator-request
Details
Bug 1678619: Implement a mechanism to fire pages-rank-changed event.
(deleted), text/x-phabricator-request
Details
Bug 1678619: Apply pages-rank-changed event instead of onManyFrecenciesChanged.
(deleted), text/x-phabricator-request
Details
Bug 1678619: Remove onManyFrecenciesChanged interface from nsINavHistoryService.
(deleted), text/x-phabricator-request
Details
Bug 1678619: Release UDateFormat instance by udat_close after using it.
(deleted), text/x-phabricator-request
Details

We have onFrecencyChanged and onManyFrecenciesChanged, the latter is used when it would not make sense to notify changes one by one.

It looks like the single call is only used by NewTabUtils, but considered the small amount of links it's handling it could just use the batched call.
Thus, it looks like we could just introduce frecencies-changed notification, also because we plan to move towards a system where frecency is not calculated immediately, but in chunks on idle.

Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Iteration: --- → 85.2 - Nov 30 - Dec 13
Points: --- → 3

Depends on D99419

Depends on D99420

Depends on D99421

Depends on D99423

Attachment #9192552 - Attachment is obsolete: true
Attachment #9192553 - Attachment is obsolete: true
Attachment #9192554 - Attachment is obsolete: true
Attachment #9192555 - Attachment is obsolete: true
Attachment #9192556 - Attachment is obsolete: true
Attachment #9192557 - Attachment is obsolete: true
Attachment #9194702 - Attachment description: Bug 1678619: Imlement a mechanism to fire pages-rank-changed event. → Bug 1678619: Implement a mechanism to fire pages-rank-changed event.
Pushed by dakatsuka.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2f22d6d2725 Replace onFrecencyChanged with onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/d26f45eac0b9 Remove onFrecencyChanged interface from nsINavHistoryService. r=mak https://hg.mozilla.org/integration/autoland/rev/efb9bdef89b8 Implement a mechanism to fire pages-rank-changed event. r=mak https://hg.mozilla.org/integration/autoland/rev/308fda30c166 Apply pages-rank-changed event instead of onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/64be4bea09dd Remove onManyFrecenciesChanged interface from nsINavHistoryService. r=mak
Backout by abutkovits@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/076223795fca Backed out 5 changesets for causing memory leaks. CLOSED TREE
Flags: needinfo?(daisuke)
Pushed by dakatsuka.birchill@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9a1d599b4060 Replace onFrecencyChanged with onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/53e385030d67 Remove onFrecencyChanged interface from nsINavHistoryService. r=mak https://hg.mozilla.org/integration/autoland/rev/2043a28e668e Implement a mechanism to fire pages-rank-changed event. r=mak https://hg.mozilla.org/integration/autoland/rev/0a92dfd99dac Apply pages-rank-changed event instead of onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/37b2f438bd2b Remove onManyFrecenciesChanged interface from nsINavHistoryService. r=mak https://hg.mozilla.org/integration/autoland/rev/c8b691b8e793 Release UDateFormat instance by udat_close after using it. r=zbraniecki

Backed out 6 changesets (bug 1678619) on suspicion of causing crashes (bug 1687914). a=backout DONTBUILD

Backout link: https://hg.mozilla.org/mozilla-central/rev/8e996166e64e30320578a0980baf62eb6e2247d5

Status: RESOLVED → REOPENED
Flags: needinfo?(daisuke)
Resolution: FIXED → ---

Figuring out the crash may not be trivial.
First of all, we should verify the crash reports go away after the backout.
I'd suggest to split this bug into depdencies.
In the first dependency we can land the conversion from onFrecencyChanged => onManyFrecenciesChanged.
In the second one we can land all the boilerplate code for the new notification, but don't change consumers.
Ideally that allows us to land the biggest chunk of these changes, without hitting the crash, if we hit the crash we know that part is culprit.
The last part will stay in this bug, that is the actual conversion of consumers.
In the meanwhile we can speak with some crash experts (Gabriele Svelto maybe) and some threads magician (asuth or some other core experts?) to figure out where the problem may lay.

Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/d275db0c1f78 Replace onFrecencyChanged with onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/6ad0b0a4c2d4 Remove onFrecencyChanged interface from nsINavHistoryService. r=mak https://hg.mozilla.org/integration/autoland/rev/405847d2ed21 Implement a mechanism to fire pages-rank-changed event. r=mak https://hg.mozilla.org/integration/autoland/rev/bb388b59a6dd Apply pages-rank-changed event instead of onManyFrecenciesChanged. r=mak https://hg.mozilla.org/integration/autoland/rev/d8ab5dc64a22 Remove onManyFrecenciesChanged interface from nsINavHistoryService. r=mak https://hg.mozilla.org/integration/autoland/rev/7c1829b47919 Release UDateFormat instance by udat_close after using it. r=zbraniecki
Flags: needinfo?(daisuke)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: