Closed Bug 337058 Opened 14 years ago Closed 14 years ago

Composite calendar .createInstance doesn't give empty observer list

Categories

(Calendar :: Internal Components, defect, major)

x86
All
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jminta, Assigned: jminta)

Details

Attachments

(1 file)

I added the line dump("Observers:"+this.mObservers.length+'\n'); to the function calCompositeCalendar() at http://lxr.mozilla.org/mozilla/source/calendar/providers/composite/calCompositeCalendar.js#103

With this, I see:
js> var cc1 = Components.classes["@mozilla.org/calendar/calendar;1?type=composite"].createInstance(Components.interfaces.calICompositeCalendar);
Observers:0
js> var compositeObserver = {
    onStartBatch: function onStartBatch() {},
    onEndBatch: function onEndBatch() {},
    onLoad: function onLoad() {},
    onAddItem: function onAddItem(aItem){},
    onDeleteItem: function onDeleteItem(aItem) {},
    onModifyItem: function onModifyItem(aItem) {},
    onCalendarAdded: function onCalendarAdded(aCalendar) {},
    onCalendarRemoved: function onCalendarRemoved(aCalendar) {},
    onDefaultCalendarChanged: function onDefaultCalendarChanged(aCalendar) {},

    QueryInterface: function QueryInterface(aIID) {
        if (!aIID.equals(Components.interfaces.calIObserver) &&
           !aIID.equals(Components.interfaces.calICompositeObserver) &&
           !aIID.equals(Components.interfaces.calIOperationListener) &&
           !aIID.equals(Components.interfaces.nsISupports)) {
               throw Components.results.NS_ERROR_NO_INTERFACE;
           }
           return this;
       }
    }
}
js> cc1.addObserver(compositeObserver);
js> cc1.removeObserver(compositeObserver);
js> var cc2 = Components.classes["@mozilla.org/calendar/calendar;1?type=composite"].createInstance(Components.interfaces.calICompositeCalendar);
Observers:1

Notice that cc2, supposedly a new XPCOM object, already has an observer!  Do we have a closure somewhere?  This is currently the biggest obstacle to loading Lightning in a separate window.
In xpcshell I commented out everything in calCompositeCalendar.js except for the add/remove observer methods, and still saw this behavior, so my guess is that it has something to do with the registration rather than with the js.
The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!
Assignee: base → nobody
Talked with dbaron about this.  He pointed out that member properties of a prototype aren't refreshed just because |new foo()| is called.  We need to explicitly set these to blank arrays in the constructor.

I'd bet there are other places in the code similarly abusing .prototype, and we should track those down to avoid other nasty bugs like this one.

Note that recent patches actually show that printing depended on this bug being around.  Landing this patch could easily expose other areas where we run into scoping problems with getCompositeCalendar() because of similar reliance on the .createInstance object not being clean.  As such, I'd like to push this forward at a high priority, so that we have time to track down and tidy up those areas.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #230837 - Flags: first-review?(dmose)
Severity: normal → major
Comment on attachment 230837 [details] [diff] [review]
initialize member properties in constructor

r=dmose
Attachment #230837 - Flags: first-review?(dmose) → first-review+
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.