Node dispose not called on SnapshotsDialog

[jonathanolson]

Presumably due to #10 and https://github.com/phetsims/joist/issues/366:

// @public @override
dispose: function() {
  this.disposeSnapshotsDialog();
  //TODO Dialog.dispose fails, see https://github.com/phetsims/models-of-the-hydrogen-atom/issues/10 and https://github.com/phetsims/joist/issues/366
  //JO: Not calling super dispose here will fail an assertion.
  // Dialog.prototype.dispose.call( this );
},

currently fails assertions with:

models-of-the-hydrogen-atom : fuzz : require.js : run
Uncaught Error: Assertion failed: Node.dispose() call is missing from an overridden dispose method
Error: Assertion failed: Node.dispose() call is missing from an overridden dispose method
    at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/assert/js/assert.js:21:13)
    at SnapshotsDialog.Node.dispose (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/nodes/Node.js?bust=1487783605061:505:19)
    at SnapshotsDialog.hide (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/models-of-the-hydrogen-atom/js/spectra/view/SnapshotsDialog.js?bust=1487783605061:94:12)
    at Object.fire (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/joist/js/BarrierRectangle.js?bust=1487783605061:60:57)
    at ButtonListener.setButtonState (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/ButtonListener.js?bust=1487783605061:131:31)
    at Object.up (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/ButtonListener.js?bust=1487783605061:89:14)
    at ButtonListener.buttonUp (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/DownUpListener.js?bust=1487783605061:123:22)
    at Object.up (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/DownUpListener.js?bust=1487783605061:56:16)
    at Input.dispatchToPointer (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/Input.js?bust=1487783605061:773:27)
    at Input.dispatchEvent (https://bayes.colorado.edu/continuous-testing/snapshot-1487782506879/scenery/js/input/Input.js?bust=1487783605061:746:12)
Approximately 2/22/2017, 9:55:06 AM

[jonathanolson]

Found as part of https://github.com/phetsims/scenery/issues/601.

[phet-steele]

@pixelzoom, if this isn't too difficult, can you address this sooner rather than later? I only ask with the hope of cleaning up weekly automated testing (in which this error has been repeatedly occurring).

[pixelzoom]

@phet-steele fixed (?) in above commit. I'm traveling and did this rapidly, so apologies if it doesn't address the problem for some reason. Please verify in master.

[phet-steele]

@pixelzoom this did not fix the problem. It can wait until you return from traveling. Thank you for your time anyway!

[pixelzoom]

@phetsteel It did indeed fix the problem, but introduced a new problem (different error message):

Assertion failed: This Node has already been disposed, and cannot be disposed again

[pixelzoom]

Note to self: reproduce this by running in requirejs mode with ?ea&fuzzMouse.

[pixelzoom]

To reproduce without fuzzMouse, run with ?ea:

1. Go to Specta screen
2. expand Spectrometer accordion box
3. press snapshot (camera) button
4. press "View snapshots" button
5. Click outside of the snapshots dialog to dismiss it
6. press "View snapshots" button
7. Click on trash can button

This will cause the assertion failure. Looks like it's because SnapshotDialog.hide also calls dispose, so dispose is getting called twice.

[pixelzoom]

Here's what happens in the steps above:

4. press "View snapshots" button - creates an instance of SnapshotDialog (instance 1)
5. Click outside of the snapshots dialog to dismiss it - calls dispose on instance 1
6. press "View snapshots" button - creates a new instance of SnapshotDialog (instance 2)
7. Click on trash can button - calls dispose on instance 1 again.

So apparently something is keeping a reference to the the first SnapshotDialog that it shouldn't be keeping. Now I need to investigate whether that something is joist or the sim.

[pixelzoom]

There was a missing numberOfSnapshotsProperty.unlink in SnapshotDialog that was causing a reference to the dialog to be kept. Fixed in above commit.

@phet-steele please verify in master.

[phet-steele]

Fixed!