[Code Review] Trying to wrap my head around promises. Here's my attempt at rewriting one somewhat nested function. I'm not sure how to stop propagation from within a promise callback so, for the rewritten version, I changed `this.controller.isActive` to reject the promise if `isActive` is `false`. I don't really know if this code even works, I w…

original.js

ProfilerPanel.prototype = {
  stopProfiling: function () {
    this.controller.isActive(function (err, isActive) {
      if (err) {
        Cu.reportError("ProfilerController.isActive: " + err.message);
        return;
      }

      if (!isActive) {
        return;
      }

      this.controller.stop(function (err, data) {
        if (err) {
          Cu.reportError("ProfilerController.stop: " + err.message);
          return;
        }

        this.activeProfile.parse(data, function onParsed() {
          this.emit("parsed");
        }.bind(this));

        onStop();
        this.emit("stopped");
      }.bind(this));
    }.bind(this));
  }
};

rewrite.js

ProfilerPanel.prototype = {
  stopProfiling: function () {
    let {emit, controller, activeProfile} = this;
    let isActive = controller.isActive();

    let stop = function () {
      return controller.stop().then(function emit("stopped"));
    };

    let parse = function (data) {
      activeProfile.parse(data, function emit("parsed"));
    }
    
    isActive.then(stop).then(parse).then(onStop, function (err) {
      if (err) {
        Cu.reportError("ProfilerController.stopProfiling: " + err.message);
      }
    });
  }
};

Kind of off-topic, but I'm a bit confused by this:

    let stop = function () {
      return controller.stop().then(function emit("stopped"));
    };

I'm not sure if that's just a typo or some ES6 syntax that I'm not aware of... I'm thinking what you mean here is:

    let stop = function () {
      return controller.stop().then(emit.bind(this, "stopped"));
    };

@joewalker, this is true—isActive is bizarre. Thanks!
@jugglinmike, that's a Firefox specific shortcut syntax for one-statement functions. Don't know if its in ES6 or not.

The if (err) should not be necessary, unless one of your steps is intentionally doing throw undefined or similar.

@domenic if (err) is to distinguish between the case when isActive fails and the case when it simply returns false. It's unnecessary if I move async isActive into the controller.stop.