From 62b8c30d0bd44f9bd995c35eb676e0547a68579d Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Tue, 1 Nov 2016 16:55:28 +0000
Subject: [PATCH] make pollSavedStatus more robust against failure

check last ack timestamp and size of pending op

provide method to compute sharejs op size so we can check if pending
ops get too big
---
 .../public/coffee/ide/editor/Document.coffee  | 25 ++++++++++++++++---
 .../coffee/ide/editor/ShareJsDoc.coffee       | 15 +++++++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee
index 78431b8b6b..7b41efdc12 100644
--- a/services/web/public/coffee/ide/editor/Document.coffee
+++ b/services/web/public/coffee/ide/editor/Document.coffee
@@ -69,6 +69,12 @@ define [
 		getPendingOp: () ->
 			@doc?.getPendingOp()
 
+		getRecentAck: () ->
+			@doc?.getRecentAck()
+
+		getOpSize: (op) ->
+			@doc?.getOpSize(op)
+
 		hasBufferedOps: () ->
 			@doc?.hasBufferedOps()
 
@@ -143,24 +149,35 @@ define [
 		clearChaosMonkey: () ->
 			clearTimeout @_cm
 
+		MAX_PENDING_OP_SIZE: 30 # pending ops bigger than this are always
+														# considered unsaved
+
 		pollSavedStatus: () ->
 			# returns false if doc has ops waiting to be acknowledged or
 			# sent that haven't changed since the last time we checked.
 			# Otherwise returns true.
 			inflightOp = @getInflightOp()
 			pendingOp = @getPendingOp()
+			recentAck = @getRecentAck()
+			pendingOpSize = pendingOp? && @getOpSize(pendingOp)
 			if !inflightOp? and !pendingOp?
-				# there's nothing going on
+				# there's nothing going on, this is ok.
 				saved = true
 				sl_console.log "[pollSavedStatus] no inflight or pending ops"
 			else if inflightOp? and inflightOp == @oldInflightOp
 				# The same inflight op has been sitting unacked since we
-				# last checked.
+				# last checked, this is bad.
 				saved = false
 				sl_console.log "[pollSavedStatus] inflight op is same as before"
-			else
+			else if pendingOp? and recentAck && pendingOpSize < @MAX_PENDING_OP_SIZE
+				# There is an op waiting to go to server but it is small and
+				# within the flushDelay, this is ok for now.
 				saved = true
-				sl_console.log "[pollSavedStatus] assuming saved (inflightOp?: #{inflightOp?}, pendingOp?: #{pendingOp?})"
+				sl_console.log "[pollSavedStatus] pending op (small with recent ack) assume ok", pendingOp, pendingOpSize
+			else
+				# In any other situation, assume the document is unsaved.
+				saved = false
+				sl_console.log "[pollSavedStatus] assuming not saved (inflightOp?: #{inflightOp?}, pendingOp?: #{pendingOp?})"
 
 			@oldInflightOp = inflightOp
 			return saved
diff --git a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee
index 27d325676c..8b9623c270 100644
--- a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee
+++ b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee
@@ -46,6 +46,7 @@ define [
 			@_doc.on "change", () =>
 				@trigger "change"
 			@_doc.on "acknowledge", () =>
+				@lastAcked = new Date() # note time of last ack from server
 				@trigger "acknowledge"
 			@_doc.on "remoteop", () =>
 				# As soon as we're working with a collaborator, start sending
@@ -101,12 +102,26 @@ define [
 			@connection.id = @socket.socket.sessionid
 			@_doc.autoOpen = false
 			@_doc._connectionStateChanged(state)
+			@lastAcked = null # reset the last ack time when connection changes
 
 		hasBufferedOps: () ->
 			@_doc.inflightOp? or @_doc.pendingOp?
 
 		getInflightOp: () -> @_doc.inflightOp
 		getPendingOp: () -> @_doc.pendingOp
+		getRecentAck: () ->
+			# check if we have received an ack recently (within the flush delay)
+			@lastAcked? and new Date() - @lastAcked < @_doc._flushDelay
+		getOpSize: (op) ->
+			# compute size of an op from its components
+			# (total number of characters inserted and deleted)
+			size = 0
+			for component in op or []
+				if component?.i?
+					size += component.i.length
+				if component?.d?
+					size += component.d.length
+			return size
 
 		attachToAce: (ace) -> @_doc.attach_ace(ace, false, window.maxDocLength)
 		detachFromAce: () -> @_doc.detach_ace?()