reward node had no dispose function, leaks memory

[jbphet]

While inspecting code changes for https://github.com/phetsims/build-an-atom/issues/101, I noticed that no dispose function was being called on BAARewardNode, which is a sub-type of the common code type RewardNode. I did some testing and found that reward nodes are not being garbage collected, see the screenshot of a heap profile below. In this profile, I'd gotten the reward node three times, and all three are still around. Since the reward node often creates a lot of sub-nodes, it is fairly memory intensive, and in this profile it's consuming a little more than 2% of all memory for the sim. As an experiment, I commented out the two lines of code in the following excerpt from the RewardNode and the leak went away.

        // When the scene is resized, update the bounds
        scene.addEventListener( 'resize', updateBounds );

        // When the ScreenView transform changes, update the bounds.  This prevents a "behind by one" problem, see https://github.com/phetsims/vegas/issues/4
        this.getScreenView().on( 'transform', updateBounds );

A dispose function should probably be added that removes these listeners.

[jbphet]

Assigning to @samreid to start with, since he is listed as the author of RewardNode. @samreid - I know you've got a lot on your plate at the moment, so feel free to delegate this to @aadish if you like. @aadish and I have been working on plugging memory leaks, so he's probably in a good position to do the work if needed.

[samreid]

@aadish can you please try the following?

1. Add a dispose function to RewardNode
2. The dispose function should unlink all of the listeners. I see 3 added, one in the constructor and one in init.
3. Call dispose from the appropriate site

If this does not resolve the problem, please commit and reassign to me.

[jbphet]

Note that the 3rd item in @samreid's list above may need to be done in all simulations where RewardNode is used.

[aadish]

added the dispose function for rewardNode and called its dispose function in the simulations using it. Profiles on all these simulations and the reward node is not retained @jbphet can you please review this and assign me back for any changes

[jbphet]

I just tested this on the current master, and it doesn't seem to be working. I ran using the 'reward' flag on Build an Atom, and when I got the the screen where the reward node was showing and then pressed the "Continue" button on the dialog, it hit an assertion that said, "Assertion failed: Node.off was called but no listener was removed". This came from the attempt to unhook the resize listener, currently on line 255 of RewardNode.js. I did a little digging, and the method getScene does not appear to be returning the same value when called in the dispose function as it does in the init function. Assigning back to @aadish for investigation.

[jbphet]

@aadish said that this was fixed this morn, and after pulling the problem is no longer happening.

I reviewed the commits, and they look good. I tested on Build and Atom and Reactants, Products, and Leftovers by getting a reward node, taking a heap snapshot, seeing that there was one instance of the node (it's a subclass for both of these sims), then continuing back to the level selection screen, taking another heap snapshot, and verifying that there are no instances of the reward node. All tests passed. Closing.