Closed Bug 1089332 Opened 5 years ago Closed 5 years ago

Add a getObservers API for accessing the history observers list from History.jsm and nsPlacesExpiration

Categories

(Toolkit :: Places, defect)

defect
Not set
Points:
2

Tracking

()

RESOLVED FIXED
mozilla36
Iteration:
36.3

People

(Reporter: mano, Assigned: mak)

References

Details

Attachments

(2 files, 1 obsolete file)

Similar to the bookmarks patch in bug 1081099, we need a way to notify history observers from the new History.jsm module and from nsPlacesExpiration (see bug 937560 comment 15).

Once that's in place, nsPIPlacesHistoryListenersNotifier should be removed. This may be done in a follow up.
Flags: firefox-backlog+
yep, sounds like a nice cleanup to do.
Blocks: asyncHistory
Points: --- → 3
Flags: in-testsuite?
Depends on: 1090308
Depends on: 937560
Blocks: 1090312
No longer blocks: asyncHistory
OK, we've got a "similar to this other patch". If we can get a DXR link in here, this looks like a good mentored but. Mano, feel like mentoring a contributor through this?
Flags: needinfo?(mano)
Of course but we need this fixed in the next few weeks. If it cannot happen by then (and the sooner the better). I'll take it myself, or maybe Marco will.

http://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsINavBookmarksService.idl#550  <- Port that to nsINavHistoryService.idl

http://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsNavBookmarks.cpp#2674  <- Port that to nsNavHistory.cpp

And there's also the test:
http://dxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_bookmark_catobs.js
Flags: needinfo?(mano)
Blocks: 1090308
No longer depends on: 1090308
taking cause it's needed for bug 1090308.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Marco please add this to the current iteration
Points: 3 → 2
Flags: needinfo?(mmucci)
Added to IT 36.3
Iteration: --- → 36.3
Flags: needinfo?(mmucci) → qe-verify?
Flags: qe-verify? → qe-verify-
Blocks: 1101553
Summary: private-API for accessing the history observers list from History.jsm and nsPlacesExpiration → Add a getObservers API for accessing the history observers list from History.jsm and nsPlacesExpiration
Attached file MozReview Request: bz://1089332/mak (obsolete) —
Attachment #8525334 - Flags: review?(mano)
/r/783 - Bug 1089332 - Add a getObservers API for accessing the history observers list. r=Mano

Pull down this commit:

hg pull review -r 5f5fef9378a8dc3ac9efaf297c6208ead39db21d
/r/783 - Bug 1089332 - Add a getObservers API for accessing the history observers list. r=Mano
/r/785 - Bug 1090308 - Invalidate mDaysOfHistory when getObservers is invoked. r=Mano

Pull down these commits:

hg pull review -r 598e4a507f2731d62b60bc13ba00ecc86d4a7608
https://reviewboard.mozilla.org/r/783/#review443

::: toolkit/components/places/tests/unit/test_history_catobs.js
(Diff revision 1)
> +  let notificationsPromised = new Promise((resolve, reject) => {  

trailing spaces
https://hg.mozilla.org/integration/fx-team/rev/dc582009548d
Flags: in-testsuite? → in-testsuite+
Target Milestone: --- → mozilla36
https://hg.mozilla.org/mozilla-central/rev/dc582009548d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #8525334 - Attachment is obsolete: true
Attachment #8618460 - Flags: review+
Attachment #8618461 - Flags: review+
You need to log in before you can comment on or make changes to this bug.