Defer starting connection monitor until a connection is opened

3 files changed, 52 insertions, 41 deletions

diff --git a/actioncable/app/assets/javascripts/action_cable/connection.coffee b/actioncable/app/assets/javascripts/action_cable/connection.coffee
index 9be5cdf5fb..bd63f5bb57 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee
@@ -1,3 +1,5 @@
+#= require ./connection_monitor
+
 # Encapsulate the cable connection held by the consumer. This is an internal class not intended for direct user manipulation.
 
 {message_types} = ActionCable.INTERNAL
@@ -6,6 +8,7 @@ class ActionCable.Connection
   @reopenDelay: 500
 
   constructor: (@consumer) ->
+    @monitor = new ActionCable.ConnectionMonitor this
 
   send: (data) ->
     if @isOpen()
@@ -23,6 +26,7 @@ class ActionCable.Connection
       @uninstallEventHandlers() if @webSocket?
       @webSocket = new WebSocket(@consumer.url)
       @installEventHandlers()
+      @monitor.start()
       true
 
   close: ->
@@ -72,9 +76,9 @@ class ActionCable.Connection
       {identifier, message, type} = JSON.parse(event.data)
       switch type
         when message_types.welcome
-          @consumer.connectionMonitor.connected()
+          @monitor.recordConnect()
         when message_types.ping
-          @consumer.connectionMonitor.ping()
+          @monitor.recordPing()
         when message_types.confirmation
           @consumer.subscriptions.notify(identifier, "connected")
         when message_types.rejection
@@ -98,5 +102,5 @@ class ActionCable.Connection
   disconnect: ->
     return if @disconnected
     @disconnected = true
-    @consumer.connectionMonitor.disconnected()
     @consumer.subscriptions.notifyAll("disconnected")
+    @monitor.recordDisconnect()
diff --git a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
index 904a426644..0cc675fa94 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
@@ -7,60 +7,69 @@ class ActionCable.ConnectionMonitor
 
   @staleThreshold: 6 # Server::Connections::BEAT_INTERVAL * 2 (missed two pings)
 
-  constructor: (@consumer) ->
-    @start()
+  constructor: (@connection) ->
+    @reconnectAttempts = 0
 
-  connected: ->
-    @reset()
-    @pingedAt = now()
-    delete @disconnectedAt
-    ActionCable.log("ConnectionMonitor connected")
+  start: ->
+    unless @isRunning()
+      @startedAt = now()
+      delete @stoppedAt
+      @startPolling()
+      document.addEventListener("visibilitychange", @visibilityDidChange)
+      ActionCable.log("ConnectionMonitor started. pollInterval = #{@getPollInterval()} ms")
 
-  disconnected: ->
-    @disconnectedAt = now()
-    ActionCable.log("ConnectionMonitor disconnected")
+  stop: ->
+    if @isRunning()
+      @stoppedAt = now()
+      @stopPolling()
+      document.removeEventListener("visibilitychange", @visibilityDidChange)
+      ActionCable.log("ConnectionMonitor stopped")
+
+  isRunning: ->
+    @startedAt? and not @stoppedAt?
 
-  ping: ->
+  recordPing: ->
     @pingedAt = now()
 
-  reset: ->
+  recordConnect: ->
     @reconnectAttempts = 0
-    @consumer.connection.isOpen()
+    @recordPing()
+    delete @disconnectedAt
+    ActionCable.log("ConnectionMonitor recorded connect")
 
-  start: ->
-    @reset()
-    delete @stoppedAt
-    @startedAt = now()
+  recordDisconnect: ->
+    @disconnectedAt = now()
+    ActionCable.log("ConnectionMonitor recorded disconnect")
+
+  # Private
+
+  startPolling: ->
+    @stopPolling()
     @poll()
-    document.addEventListener("visibilitychange", @visibilityDidChange)
-    ActionCable.log("ConnectionMonitor started, pollInterval is #{@getInterval()}ms")
 
-  stop: ->
-    @stoppedAt = now()
-    document.removeEventListener("visibilitychange", @visibilityDidChange)
-    ActionCable.log("ConnectionMonitor stopped")
+  stopPolling: ->
+    clearTimeout(@pollTimeout)
 
   poll: ->
-    setTimeout =>
-      unless @stoppedAt
-        @reconnectIfStale()
-        @poll()
-    , @getInterval()
+    @pollTimeout = setTimeout =>
+      @reconnectIfStale()
+      @poll()
+    , @getPollInterval()
 
-  getInterval: ->
+  getPollInterval: ->
     {min, max} = @constructor.pollInterval
     interval = 5 * Math.log(@reconnectAttempts + 1)
-    clamp(interval, min, max) * 1000
+    Math.round(clamp(interval, min, max) * 1000)
 
   reconnectIfStale: ->
     if @connectionIsStale()
-      ActionCable.log("ConnectionMonitor detected stale connection, reconnectAttempts = #{@reconnectAttempts}")
+      ActionCable.log("ConnectionMonitor detected stale connection. reconnectAttempts = #{@reconnectAttempts}, pollInterval = #{@getPollInterval()} ms, time disconnected = #{secondsSince(@disconnectedAt)} s, stale threshold = #{@constructor.staleThreshold} s")
       @reconnectAttempts++
       if @disconnectedRecently()
-        ActionCable.log("ConnectionMonitor skipping reopen because recently disconnected at #{@disconnectedAt}")
+        ActionCable.log("ConnectionMonitor skipping reopening recent disconnect")
       else
         ActionCable.log("ConnectionMonitor reopening")
-        @consumer.connection.reopen()
+        @connection.reopen()
 
   connectionIsStale: ->
     secondsSince(@pingedAt ? @startedAt) > @constructor.staleThreshold
@@ -71,9 +80,9 @@ class ActionCable.ConnectionMonitor
   visibilityDidChange: =>
     if document.visibilityState is "visible"
       setTimeout =>
-        if @connectionIsStale() or not @consumer.connection.isOpen()
-          ActionCable.log("ConnectionMonitor reopening stale connection after visibilitychange to #{document.visibilityState}")
-          @consumer.connection.reopen()
+        if @connectionIsStale() or not @connection.isOpen()
+          ActionCable.log("ConnectionMonitor reopening stale connection on visibilitychange. visbilityState = #{document.visibilityState}")
+          @connection.reopen()
       , 200
 
   now = ->
diff --git a/actioncable/app/assets/javascripts/action_cable/consumer.coffee b/actioncable/app/assets/javascripts/action_cable/consumer.coffee
index 3d93d40b99..7aae1ed8ed 100644
--- a/actioncable/app/assets/javascripts/action_cable/consumer.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/consumer.coffee
@@ -1,5 +1,4 @@
 #= require ./connection
-#= require ./connection_monitor
 #= require ./subscriptions
 #= require ./subscription
 
@@ -19,7 +18,6 @@ class ActionCable.Consumer
   constructor: (@url) ->
     @subscriptions = new ActionCable.Subscriptions this
     @connection = new ActionCable.Connection this
-    @connectionMonitor = new ActionCable.ConnectionMonitor this
 
   send: (data) ->
     @connection.send(data)