In chipper-startup, prevent further attempts after success

[samreid]

From #764 I said:

(6) It looks like there is a little interference between the setTimeout and the addEventListener( 'resize'. For instance, if attemptStart is initially false, then a timeout is scheduled for 1000ms later, then resize triggers at 500ms, then attemptStart is called later unnecessarily. I know at that point it becomes a no-op, but it seems asymmetrical to not have the events types cancel each other. Not sure how much more complicated this would make it though. Something like:

  // We can call this anytime to attempt to start things (if they haven't been started already).
  function attemptStart() {
    if ( !started && isReady() ) {
      started = true;
      timeoutStillRunning && clearTimeout();
      windowListenerStillRunning && removeWindowListener();

      phet.chipper.runRequireJS();
    }
  }

For instance, with this patch:

Index: templates/chipper-startup.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- templates/chipper-startup.js	(revision 0e1cf7b7f51776bca20379e777bc36200b9b3757)
+++ templates/chipper-startup.js	(date 1562348254000)
@@ -62,7 +62,10 @@
 
   // We can call this anytime to attempt to start things (if they haven't been started already).
   function attemptStart() {
+    console.log( 'attempt start' );
     if ( !started && isReady() ) {
+
+      console.log( 'started' );
       started = true;
 
       phet.chipper.runRequireJS();
@@ -71,6 +74,7 @@
 
   // Attempt to start based on timeouts (in case other methods don't work). This will call attemptStart() at least once.
   ( function timeoutListener() {
+    console.log( 'timeoutListener' );
     attemptStart();
 
     if ( !started ) {
@@ -82,10 +86,12 @@
 
     // Attempt to start on window resizes
     window.addEventListener( 'resize', function resizeListener() {
+      console.log( 'resize' );
       attemptStart();
 
       if ( started ) {
         window.removeEventListener( 'resize', resizeListener );
+        console.log( 'resize.started' );
       }
     } );
   }

I saw this console.log trace:

timeoutListener
attempt start
timeoutListener
attempt start
timeoutListener
attempt start
timeoutListener
attempt start
timeoutListener
attempt start
resize
attempt start
started
resize.started
timeoutListener
attempt start

Notice that even after resize and started, there is still another attempt to start.

[samreid]

@jonathanolson replied:

It seems like more effort to independently track the state of all listeners to be able to clear them. Is there an advantage?

[samreid]

On slack, I said:

I also think it would be nice to remove listeners on a fine-grained basis so we don’t end up with the pattern where “attempt start” is called again after it is already started.

Would this be prohibitively expensive to implement?

[jonathanolson]

Would this be prohibitively expensive to implement?

It's not prohibitive, but I find the current approach cleaner and easier to maintain. What is an advantage to the other approach, as it would have more code and more moving parts?

[samreid]

Leaving self-assigned to implement the fine-grained pattern for comparison. Not sure when I'll get to this, so let me know if timing becomes an issue.

[samreid]

This is not a priority for me at the moment, unassigning.