Closed Bug 1436966 Opened 3 years ago Closed 3 years ago

Remove the bookmark-observers & history-observers category listeners

Categories

(Toolkit :: Places, enhancement, P1)

Product:
Toolkit
Component:
Places
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Whiteboard: [fxsearch])

Attachments

(1 file)

Bug 1436966 - Remove the bookmark-observers & history-observers category listeners.
59 bytes, text/x-review-board-request
mak
: review+
Details 
Since bug 1371677, the bookmark-observers category listener is no longer used except for one test.

We should remove support for it as we no longer need it, especially with notification rework on the horizon.
Turns out from a c++ perspective it is slightly easier to remove the history-observer as well at the same time.
Summary: Remove the bookmark-observers category listener → Remove the bookmark-observers & history-observers category listeners
Comment on attachment 8949690 [details]
Bug 1436966 - Remove the bookmark-observers & history-observers category listeners.

https://reviewboard.mozilla.org/r/219030/#review224776

Thanks, this may help with observers refactoring (and likely should block that bug)

::: toolkit/components/places/nsNavBookmarks.cpp:2847
(Diff revision 1)
>    nsCOMArray<nsINavBookmarkObserver> observers;
>  
> -  // First add the category cache observers.
> -  mCacheObservers.GetEntries(observers);
> -
>    // Then add the other observers.

this comment should likely go away

::: toolkit/components/places/nsPlacesExpiration.js:442
(Diff revision 1)
>        return;
>      }
>      this._shuttingDown = true;
>      this.expireOnIdle = false;
> +
> +    PlacesUtils.history.removeObserver(this);

Let's just add this as a weak observer, we don't need the observer service to keep us alive, and the history service will stop notifying by itself on shutdown.
so we can avoid this removal.
Attachment #8949690 - Flags: review?(mak77) → review+
Blocks: 1340498
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s cbe0c2cf72778c660c3d0ec7b070b062396605f4 -d 1f07c08daa41: rebasing 446568:cbe0c2cf7277 "Bug 1436966 - Remove the bookmark-observers & history-observers category listeners. r=mak" (tip)
merging toolkit/components/places/nsNavBookmarks.cpp
merging toolkit/components/places/nsNavBookmarks.h
warning: conflicts while merging toolkit/components/places/nsNavBookmarks.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/64ec4273a0d7
Remove the bookmark-observers & history-observers category listeners. r=mak
https://hg.mozilla.org/mozilla-central/rev/64ec4273a0d7
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.