545487 - Cleanup TabTracker_observe

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

* Should remove the event listener (not necessary but good form)
* Don't need to QI windows

Comment 1

[Ed Lee :Mardak]

(In reply to comment #0)
> * Should remove the event listener (not necessary but good form)
Are you talking about when the tracker is ending? There isn't an existing code structure to tell various things to unload, and related.. things kinda assume they stay around until the app is quitting.

And just to be clear, you're referring to the window watcher registerNotification?

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #1)
> (In reply to comment #0)
> > * Should remove the event listener (not necessary but good form)
> Are you talking about when the tracker is ending? There isn't an existing code
> structure to tell various things to unload, and related.. things kinda assume
> they stay around until the app is quitting.

No, I mean right after the listener for "load" is called.

> And just to be clear, you're referring to the window watcher
> registerNotification?

Yes.

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

Comment 4

[Ed Lee :Mardak]

Comment on attachment 426383 [details] [diff] [review]
Patch v0.1

>   observe: function TabTracker_observe(aSubject, aTopic, aData) {
..
>+      aSubject.addEventListener("load", function(event) {
>+        event.currentTarget.removeEventListener("load", arguments.callee, false);
You can just name the function and do something like..
aSubject.removeEventListener("load", onLoad, false);

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/labs/weave/rev/277046385d47