From e9bf04b50b9ed4763bd8f82f32dfb37d0dedf27e Mon Sep 17 00:00:00 2001
From: Antonio Scandurra <me@as-cii.com>
Date: Mon, 14 Sep 2015 20:21:37 +0200
Subject: [PATCH] Guard against some edge cases

---
 spec/dom-elements-pool-spec.coffee | 49 +++++++++++++++---------------
 src/dom-elements-pool.coffee       | 20 ++++++++----
 2 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/spec/dom-elements-pool-spec.coffee b/spec/dom-elements-pool-spec.coffee
index 2b7364a8c..56cf15761 100644
--- a/spec/dom-elements-pool-spec.coffee
+++ b/spec/dom-elements-pool-spec.coffee
@@ -6,26 +6,14 @@ describe "DomElementsPool", ->
   beforeEach ->
     domElementsPool = new DomElementsPool
 
-  it "creates new nodes until some of them are freed", ->
-    span1 = domElementsPool.build("span")
-    span2 = domElementsPool.build("span")
-    span3 = domElementsPool.build("span")
-
-    expect(span1).not.toBe(span2)
-    expect(span2).not.toBe(span3)
-
-    domElementsPool.free(span1)
-    domElementsPool.free(span2)
-
-    expect(domElementsPool.build("span")).toBe(span2)
-    expect(domElementsPool.build("span")).toBe(span1)
-
-  it "recursively frees a dom tree", ->
-    div = domElementsPool.build("div")
-    span1 = domElementsPool.build("span")
-    span2 = domElementsPool.build("span")
-    span3 = domElementsPool.build("span")
-    span4 = domElementsPool.build("span")
+  it "builds DOM nodes, recycling them when they are freed", ->
+    [div, span1, span2, span3, span4] = elements = [
+      domElementsPool.build("div")
+      domElementsPool.build("span")
+      domElementsPool.build("span")
+      domElementsPool.build("span")
+      domElementsPool.build("span")
+    ]
 
     div.appendChild(span1)
     span1.appendChild(span2)
@@ -34,8 +22,19 @@ describe "DomElementsPool", ->
 
     domElementsPool.freeElementAndDescendants(div)
 
-    expect(domElementsPool.build("div")).toBe(div)
-    expect(domElementsPool.build("span")).toBe(span2)
-    expect(domElementsPool.build("span")).toBe(span1)
-    expect(domElementsPool.build("span")).toBe(span4)
-    expect(domElementsPool.build("span")).toBe(span3)
+    expect(elements).toContain(domElementsPool.build("div"))
+    expect(elements).toContain(domElementsPool.build("span"))
+    expect(elements).toContain(domElementsPool.build("span"))
+    expect(elements).toContain(domElementsPool.build("span"))
+    expect(elements).toContain(domElementsPool.build("span"))
+
+    expect(elements).not.toContain(domElementsPool.build("span"))
+
+  it "throws an error when trying to free the same node twice", ->
+    div = domElementsPool.build("div")
+    domElementsPool.freeElementAndDescendants(div)
+    expect(-> domElementsPool.freeElementAndDescendants(div)).toThrow()
+
+  it "throws an error when trying to free an invalid element", ->
+    expect(-> domElementsPool.freeElementAndDescendants(null)).toThrow()
+    expect(-> domElementsPool.freeElementAndDescendants(undefined)).toThrow()
diff --git a/src/dom-elements-pool.coffee b/src/dom-elements-pool.coffee
index d0c653b02..f75c904aa 100644
--- a/src/dom-elements-pool.coffee
+++ b/src/dom-elements-pool.coffee
@@ -2,6 +2,7 @@ module.exports =
 class DomElementsPool
   constructor: ->
     @freeElementsByTagName = {}
+    @freedElements = new Set
 
   build: (tagName, className, textContent) ->
     element = @freeElementsByTagName[tagName]?.pop()
@@ -9,16 +10,23 @@ class DomElementsPool
     element.className = className
     element.textContent = textContent
     element.removeAttribute("style")
-    element
 
-  free: (element) ->
-    element.remove()
-    @freeElementsByTagName[element.tagName.toLowerCase()] ?= []
-    @freeElementsByTagName[element.tagName.toLowerCase()].push(element)
+    @freedElements.delete(element)
+
+    element
 
   freeElementAndDescendants: (element) ->
     @free(element)
-
     for index in [element.children.length - 1..0] by -1
       child = element.children[index]
       @freeElementAndDescendants(child)
+
+  free: (element) ->
+    throw new Error("The element cannot be null or undefined.") unless element?
+    throw new Error("The element has already been freed!") if @freedElements.has(element)
+
+    @freeElementsByTagName[element.tagName.toLowerCase()] ?= []
+    @freeElementsByTagName[element.tagName.toLowerCase()].push(element)
+    @freedElements.add(element)
+
+    element.remove()