Experiments.disableExperiment() currently requires passing an experiment id. We only ever have one active experiment, so instead it should just disable the current experiment.

Part of this try push: https://tbpl.mozilla.org/?tree=Try&rev=003f94f3a765

Comment on attachment 8395750 [details] [diff] [review] Drop id parameter from Experiments.disableExperiment() Review of attachment 8395750 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/experiments/Experiments.jsm @@ +792,4 @@ > > + yield experiment.stop(terminationKind); > + Services.obs.notifyObservers(null, OBSERVER_TOPIC, null); > + this._pendingTasks.disableExperiment = null; shouldn't this be set as a resolved promise? or better, not set anything and let disableExperiment be defined as a resolved promise anyway as Task.spawn finishes the task?

Comment on attachment 8395750 [details] [diff] [review] Drop id parameter from Experiments.disableExperiment() I've got a different patch which touches the same code.

(In reply to :Felipe Gomes from comment #2) > ::: browser/experiments/Experiments.jsm > @@ +792,4 @@ > > > > + yield experiment.stop(terminationKind); > > + Services.obs.notifyObservers(null, OBSERVER_TOPIC, null); > > + this._pendingTasks.disableExperiment = null; > > shouldn't this be set as a resolved promise? or better, not set anything and > let disableExperiment be defined as a resolved promise anyway as Task.spawn > finishes the task? The promise is resolved for those that already wait on it when the task finishes. At the moment all the tasks null themselves out when done, non-null means there is already a pending or active task of this kind. But bsmedberg should currently be switching out how this works. (In reply to Benjamin Smedberg [:bsmedberg] from comment #3) > I've got a different patch which touches the same code. I assume this is blocked by bug 987225 as well then?

The change proposed here is already taken care of by bug 987225.