1899013 - Potential nsIObserver loop in ActivityStream

Status: NEW

(Firefox :: New Tab Page, defect, P2)

Description

[Shane Hughes [:aminomancer]]

Issue here.

I'm not sure if a loop can actually happen in practice, but it still would be wise to remove it. This isn't a blocking infinite loop, it just means the observer notification callback can add another observer without removing the existing one. On the initial run, if lazy.Region.home is falsy, a browser-region-updated observer will be added, and this.geo will be set to an empty string. If locale or region is changed, this.geo will be set to a non-empty string. If locale or region changes again, and lazy.Region.home is somehow falsy again (not sure if this is possible, but it's not ideal to rely on that expectation), a second observer will be added. Now if that same sequence of steps repeats, two observers will become four. The number of observers will double with each repetition, since each observer callback creates a new observer. These region/locale notifications firing repetitively is obviously quite unlikely outside of rare edge cases, but if there's ever a regression in any of the region/locale code that results in repeating spurious notifications, this might turn an otherwise silent bug into a crash.

It could be crash-proofed by just changing line 739 to this:

if (!this._regionObserverAdded) {
  Services.obs.addObserver(this, lazy.Region.REGION_TOPIC);
  this._regionObserverAdded = true;
}