From 0ddad54e882ea1333b0fa35a5869c9b6672791b1 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Gerardy <py@py.gy>
Date: Wed, 17 Aug 2016 00:05:08 +0200
Subject: [PATCH 1/2] Tests for `remove` phase Zalgo flakiness.

---
 render/tests/test-onbeforeremove.js | 30 +++++++++++++++++++++++++++++
 render/tests/test-onremove.js       | 11 +++++++++++
 2 files changed, 41 insertions(+)

diff --git a/render/tests/test-onbeforeremove.js b/render/tests/test-onbeforeremove.js
index a6797c66..15e6a511 100644
--- a/render/tests/test-onbeforeremove.js
+++ b/render/tests/test-onbeforeremove.js
@@ -188,4 +188,34 @@ o.spec("onbeforeremove", function() {
 		o(root.childNodes.length).equals(1)
 		o(root.firstChild.firstChild.nodeValue).equals("2")
 	})
+	o("finalizes the remove phase only once when `done()` is called synchronously from both attrs- and tag.onbeforeremove", function() {
+		var onremove = o.spy()
+		var component = {
+			view: function(){return {tag:'br'}},
+			onbeforeremove: function(vnode, done){done()},
+			onremove: onremove
+		}
+		render(root, [{tag: component, attrs: component}])
+		render(root, [])
+		o(onremove.callCount).equals(2) // once for `tag`, once for `attrs`
+	})
+	o("doesn't finalize prematurely if `done` is called twice in the `tag` hook", function(done) {
+		var async = false
+		var component = {
+			view: function(){return {tag:'br'}},
+			onbeforeremove: function(vnode, doneRemoving){
+				doneRemoving()
+				doneRemoving()
+			},
+			onremove: function(){
+				o(async).equals(true)("onremove should be called asynchronously")
+				done()
+			}
+		}
+		render(root, [{tag:component, attrs: {onbeforeremove: function(vnode, doneRemoving){
+			callAsync(doneRemoving)
+		}}}])
+		render(root, [])
+		async = true
+	})
 })
diff --git a/render/tests/test-onremove.js b/render/tests/test-onremove.js
index d3f423fc..43ed200d 100644
--- a/render/tests/test-onremove.js
+++ b/render/tests/test-onremove.js
@@ -145,4 +145,15 @@ o.spec("onremove", function() {
 
 		o(vnode.dom).notEquals(updated.dom)
 	})
+	o("The remove phase is finalized only once when `done()` is called synchronously from both attrs- and tag.onbeforeremove", function () {
+		var onremove = o.spy()
+		var component = {
+			view: function(){return m('br')},
+			onbeforeremove: function(vnode, done){done()},
+			onremove: onremove
+		}
+		render(root, [{tag: component, attrs: component}])
+		render(root, [])
+		o(onremove.callCount).equals(2)
+	})
 })

From 0b5800d09a0d23e3bda0f1cb363c40db2178457c Mon Sep 17 00:00:00 2001
From: Pierre-Yves Gerardy <py@py.gy>
Date: Tue, 16 Aug 2016 22:48:58 +0200
Subject: [PATCH 2/2] Make `onbeforeremove` `done()` handlers more robust.

---
 render/render.js | 67 ++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/render/render.js b/render/render.js
index f0fad6a1..40874142 100644
--- a/render/render.js
+++ b/render/render.js
@@ -203,7 +203,7 @@ module.exports = function($window) {
 			else updateComponent(parent, old, vnode, hooks, nextSibling, recycling, ns)
 		}
 		else {
-			removeNode(parent, old, null, false)
+			removeNode(parent, old, null)
 			insertNode(parent, createNode(vnode, hooks, undefined), nextSibling)
 		}
 	}
@@ -265,7 +265,7 @@ module.exports = function($window) {
 			vnode.domSize = vnode.instance.domSize
 		}
 		else if (old.instance != null) {
-			removeNode(parent, old.instance, null, false)
+			removeNode(parent, old.instance, null)
 			vnode.dom = undefined
 			vnode.domSize = 0
 		}
@@ -327,41 +327,48 @@ module.exports = function($window) {
 			var vnode = vnodes[i]
 			if (vnode != null) {
 				if (vnode.skip) vnode.skip = false
-				else removeNode(parent, vnode, context, false)
+				else removeNode(parent, vnode, context)
 			}
 		}
 	}
-	function removeNode(parent, vnode, context, deferred) {
-		if (deferred === false) {
-			var expected = 0, called = 0
-			var callback = function() {
-				if (++called === expected) removeNode(parent, vnode, context, true)
+	function once(f) {
+		var called = false
+		return function() {
+			if (!called) {
+				called = true
+				f()
 			}
-			if (vnode.attrs && vnode.attrs.onbeforeremove) {
-				expected++
-				vnode.attrs.onbeforeremove.call(vnode.state, vnode, callback)
-			}
-			if (typeof vnode.tag !== "string" && vnode.tag.onbeforeremove) {
-				expected++
-				vnode.tag.onbeforeremove.call(vnode.state, vnode, callback)
-			}
-			if (expected > 0) return
 		}
-
-		onremove(vnode)
-		if (vnode.dom) {
-			var count = vnode.domSize || 1
-			if (count > 1) {
-				var dom = vnode.dom
-				while (--count) {
-					parent.removeChild(dom.nextSibling)
+	}
+	function removeNode(parent, vnode, context) {
+		var expected = 1, called = 0
+		if (vnode.attrs && vnode.attrs.onbeforeremove) {
+			expected++
+			vnode.attrs.onbeforeremove.call(vnode.state, vnode, once(continuation))
+		}
+		if (typeof vnode.tag !== "string" && vnode.tag.onbeforeremove) {
+			expected++
+			vnode.tag.onbeforeremove.call(vnode.state, vnode, once(continuation))
+		}
+		continuation()
+		function continuation() {
+			if (++called === expected) {
+				onremove(vnode)
+				if (vnode.dom) {
+					var count = vnode.domSize || 1
+					if (count > 1) {
+						var dom = vnode.dom
+						while (--count) {
+							parent.removeChild(dom.nextSibling)
+						}
+					}
+					if (vnode.dom.parentNode != null) parent.removeChild(vnode.dom)
+					if (context != null && vnode.domSize == null && !hasIntegrationMethods(vnode.attrs) && typeof vnode.tag === "string") { //TODO test custom elements
+						if (!context.pool) context.pool = [vnode]
+						else context.pool.push(vnode)
+					}
 				}
 			}
-			if (vnode.dom.parentNode != null) parent.removeChild(vnode.dom)
-			if (context != null && vnode.domSize == null && !hasIntegrationMethods(vnode.attrs) && typeof vnode.tag === "string") { //TODO test custom elements
-				if (!context.pool) context.pool = [vnode]
-				else context.pool.push(vnode)
-			}
 		}
 	}
 	function onremove(vnode) {