From bb9d1f49c0b7dad45e855e75cb7be455caa653d4 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 09:51:04 -0700
Subject: [PATCH 01/10] Convert DOMElementPool and specs to JS

---
 spec/dom-element-pool-spec.coffee | 60 ---------------------------
 spec/dom-element-pool-spec.js     | 64 +++++++++++++++++++++++++++++
 src/dom-element-pool.coffee       | 55 -------------------------
 src/dom-element-pool.js           | 68 +++++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+), 115 deletions(-)
 delete mode 100644 spec/dom-element-pool-spec.coffee
 create mode 100644 spec/dom-element-pool-spec.js
 delete mode 100644 src/dom-element-pool.coffee
 create mode 100644 src/dom-element-pool.js

diff --git a/spec/dom-element-pool-spec.coffee b/spec/dom-element-pool-spec.coffee
deleted file mode 100644
index 2efe80beb..000000000
--- a/spec/dom-element-pool-spec.coffee
+++ /dev/null
@@ -1,60 +0,0 @@
-DOMElementPool = require '../src/dom-element-pool'
-{contains} = require 'underscore-plus'
-
-describe "DOMElementPool", ->
-  domElementPool = null
-
-  beforeEach ->
-    domElementPool = new DOMElementPool
-
-  it "builds DOM nodes, recycling them when they are freed", ->
-    [div, span1, span2, span3, span4, span5, textNode] = elements = [
-      domElementPool.buildElement("div")
-      domElementPool.buildElement("span")
-      domElementPool.buildElement("span")
-      domElementPool.buildElement("span")
-      domElementPool.buildElement("span")
-      domElementPool.buildElement("span")
-      domElementPool.buildText("Hello world!")
-    ]
-
-    div.appendChild(span1)
-    span1.appendChild(span2)
-    div.appendChild(span3)
-    span3.appendChild(span4)
-    span4.appendChild(textNode)
-
-    domElementPool.freeElementAndDescendants(div)
-    domElementPool.freeElementAndDescendants(span5)
-
-    expect(contains(elements, domElementPool.buildElement("div"))).toBe(true)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(true)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(true)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(true)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(true)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(true)
-    expect(contains(elements, domElementPool.buildText("another text"))).toBe(true)
-
-    expect(contains(elements, domElementPool.buildElement("div"))).toBe(false)
-    expect(contains(elements, domElementPool.buildElement("span"))).toBe(false)
-    expect(contains(elements, domElementPool.buildText("unexisting"))).toBe(false)
-
-  it "forgets free nodes after being cleared", ->
-    span = domElementPool.buildElement("span")
-    div = domElementPool.buildElement("div")
-    domElementPool.freeElementAndDescendants(span)
-    domElementPool.freeElementAndDescendants(div)
-
-    domElementPool.clear()
-
-    expect(domElementPool.buildElement("span")).not.toBe(span)
-    expect(domElementPool.buildElement("div")).not.toBe(div)
-
-  it "throws an error when trying to free the same node twice", ->
-    div = domElementPool.buildElement("div")
-    domElementPool.freeElementAndDescendants(div)
-    expect(-> domElementPool.freeElementAndDescendants(div)).toThrow()
-
-  it "throws an error when trying to free an invalid element", ->
-    expect(-> domElementPool.freeElementAndDescendants(null)).toThrow()
-    expect(-> domElementPool.freeElementAndDescendants(undefined)).toThrow()
diff --git a/spec/dom-element-pool-spec.js b/spec/dom-element-pool-spec.js
new file mode 100644
index 000000000..11c8be0ff
--- /dev/null
+++ b/spec/dom-element-pool-spec.js
@@ -0,0 +1,64 @@
+const DOMElementPool = require ('../src/dom-element-pool')
+
+describe('DOMElementPool', function () {
+  let domElementPool
+
+  beforeEach(() => { domElementPool = new DOMElementPool() })
+
+  it('builds DOM nodes, recycling them when they are freed', function () {
+    let elements
+    const [div, span1, span2, span3, span4, span5, textNode] = Array.from(elements = [
+      domElementPool.buildElement('div'),
+      domElementPool.buildElement('span'),
+      domElementPool.buildElement('span'),
+      domElementPool.buildElement('span'),
+      domElementPool.buildElement('span'),
+      domElementPool.buildElement('span'),
+      domElementPool.buildText('Hello world!')
+    ])
+
+    div.appendChild(span1)
+    span1.appendChild(span2)
+    div.appendChild(span3)
+    span3.appendChild(span4)
+    span4.appendChild(textNode)
+
+    domElementPool.freeElementAndDescendants(div)
+    domElementPool.freeElementAndDescendants(span5)
+
+    expect(elements.includes(domElementPool.buildElement('div'))).toBe(true)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(true)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(true)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(true)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(true)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(true)
+    expect(elements.includes(domElementPool.buildText('another text'))).toBe(true)
+
+    expect(elements.includes(domElementPool.buildElement('div'))).toBe(false)
+    expect(elements.includes(domElementPool.buildElement('span'))).toBe(false)
+    expect(elements.includes(domElementPool.buildText('unexisting'))).toBe(false)
+  })
+
+  it('forgets free nodes after being cleared', function () {
+    const span = domElementPool.buildElement('span')
+    const div = domElementPool.buildElement('div')
+    domElementPool.freeElementAndDescendants(span)
+    domElementPool.freeElementAndDescendants(div)
+
+    domElementPool.clear()
+
+    expect(domElementPool.buildElement('span')).not.toBe(span)
+    expect(domElementPool.buildElement('div')).not.toBe(div)
+  })
+
+  it('throws an error when trying to free the same node twice', function () {
+    const div = domElementPool.buildElement('div')
+    domElementPool.freeElementAndDescendants(div)
+    expect(() => domElementPool.freeElementAndDescendants(div)).toThrow()
+  })
+
+  it('throws an error when trying to free an invalid element', function () {
+    expect(() => domElementPool.freeElementAndDescendants(null)).toThrow()
+    expect(() => domElementPool.freeElementAndDescendants(undefined)).toThrow()
+  })
+})
diff --git a/src/dom-element-pool.coffee b/src/dom-element-pool.coffee
deleted file mode 100644
index f81a537f3..000000000
--- a/src/dom-element-pool.coffee
+++ /dev/null
@@ -1,55 +0,0 @@
-module.exports =
-class DOMElementPool
-  constructor: ->
-    @freeElementsByTagName = {}
-    @freedElements = new Set
-
-  clear: ->
-    @freedElements.clear()
-    for tagName, freeElements of @freeElementsByTagName
-      freeElements.length = 0
-    return
-
-  build: (tagName, factory, reset) ->
-    element = @freeElementsByTagName[tagName]?.pop()
-    element ?= factory()
-    reset(element)
-    @freedElements.delete(element)
-    element
-
-  buildElement: (tagName, className) ->
-    factory = -> document.createElement(tagName)
-    reset = (element) ->
-      delete element.dataset[dataId] for dataId of element.dataset
-      element.removeAttribute("style")
-      if className?
-        element.className = className
-      else
-        element.removeAttribute("class")
-    @build(tagName, factory, reset)
-
-  buildText: (textContent) ->
-    factory = -> document.createTextNode(textContent)
-    reset = (element) -> element.textContent = textContent
-    @build("#text", factory, reset)
-
-  freeElementAndDescendants: (element) ->
-    @free(element)
-    @freeDescendants(element)
-
-  freeDescendants: (element) ->
-    for descendant in element.childNodes by -1
-      @free(descendant)
-      @freeDescendants(descendant)
-    return
-
-  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)
-
-    tagName = element.nodeName.toLowerCase()
-    @freeElementsByTagName[tagName] ?= []
-    @freeElementsByTagName[tagName].push(element)
-    @freedElements.add(element)
-
-    element.remove()
diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
new file mode 100644
index 000000000..7cd5dc53e
--- /dev/null
+++ b/src/dom-element-pool.js
@@ -0,0 +1,68 @@
+module.exports =
+class DOMElementPool {
+  constructor () {
+    this.freeElementsByTagName = {}
+    this.freedElements = new Set()
+  }
+
+  clear () {
+    this.freedElements.clear()
+    for (let tagName in this.freeElementsByTagName) {
+      const freeElements = this.freeElementsByTagName[tagName]
+      freeElements.length = 0
+    }
+  }
+
+  build (tagName, factory, reset) {
+    let element = this.freeElementsByTagName[tagName] ? this.freeElementsByTagName[tagName].pop() : null
+    if (!element) { element = factory() }
+    reset(element)
+    this.freedElements.delete(element)
+    return element
+  }
+
+  buildElement (tagName, className) {
+    const factory = () => document.createElement(tagName)
+    const reset = function (element) {
+      for (let dataId in element.dataset) { delete element.dataset[dataId] }
+      element.removeAttribute('style')
+      if (className != null) {
+        element.className = className
+      } else {
+        element.removeAttribute('class')
+      }
+    }
+    return this.build(tagName, factory, reset)
+  }
+
+  buildText (textContent) {
+    const factory = () => document.createTextNode(textContent)
+    const reset = element => { element.textContent = textContent }
+    return this.build('#text', factory, reset)
+  }
+
+  freeElementAndDescendants (element) {
+    this.free(element)
+    return this.freeDescendants(element)
+  }
+
+  freeDescendants (element) {
+    for (let i = element.childNodes.length - 1; i >= 0; i--) {
+      const descendant = element.childNodes[i]
+      this.free(descendant)
+      this.freeDescendants(descendant)
+    }
+  }
+
+  free (element) {
+    if (element == null) { throw new Error('The element cannot be null or undefined.') }
+    if (this.freedElements.has(element)) { throw new Error('The element has already been freed!') }
+
+    const tagName = element.nodeName.toLowerCase()
+    if (this.freeElementsByTagName[tagName] == null) { this.freeElementsByTagName[tagName] = [] }
+    this.freeElementsByTagName[tagName].push(element)
+    this.freedElements.add(element)
+
+    return element.remove()
+  }
+}

From d7db7af7225e597cf065422fecba03834516bf3d Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 09:56:37 -0700
Subject: [PATCH 02/10] Remove closure allocations from DOMElementPool build
 methods

---
 src/dom-element-pool.js | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index 7cd5dc53e..fa436451c 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -13,17 +13,9 @@ class DOMElementPool {
     }
   }
 
-  build (tagName, factory, reset) {
-    let element = this.freeElementsByTagName[tagName] ? this.freeElementsByTagName[tagName].pop() : null
-    if (!element) { element = factory() }
-    reset(element)
-    this.freedElements.delete(element)
-    return element
-  }
-
   buildElement (tagName, className) {
-    const factory = () => document.createElement(tagName)
-    const reset = function (element) {
+    let element = this.freeElementsByTagName[tagName] ? this.freeElementsByTagName[tagName].pop() : null
+    if (element) {
       for (let dataId in element.dataset) { delete element.dataset[dataId] }
       element.removeAttribute('style')
       if (className != null) {
@@ -31,14 +23,22 @@ class DOMElementPool {
       } else {
         element.removeAttribute('class')
       }
+      this.freedElements.delete(element)
+    } else {
+      element = document.createElement(tagName)
     }
-    return this.build(tagName, factory, reset)
+    return element
   }
 
   buildText (textContent) {
-    const factory = () => document.createTextNode(textContent)
-    const reset = element => { element.textContent = textContent }
-    return this.build('#text', factory, reset)
+    let element = this.freeElementsByTagName['#text'] ? this.freeElementsByTagName['#text'].pop() : null
+    if (element) {
+      element.textContent = textContent
+      this.freedElements.delete(element)
+    } else {
+      element = document.createTextNode(textContent)
+    }
+    return element
   }
 
   freeElementAndDescendants (element) {

From 01f0bd56af95540aa03762236ef3657584b6893a Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 10:03:01 -0700
Subject: [PATCH 03/10] Use a Map instead of an object to store
 freeElementsByTagName

---
 src/dom-element-pool.js | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index fa436451c..d8a5e3bb9 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -1,20 +1,18 @@
 module.exports =
 class DOMElementPool {
   constructor () {
-    this.freeElementsByTagName = {}
+    this.freeElementsByTagName = new Map()
     this.freedElements = new Set()
   }
 
   clear () {
     this.freedElements.clear()
-    for (let tagName in this.freeElementsByTagName) {
-      const freeElements = this.freeElementsByTagName[tagName]
-      freeElements.length = 0
-    }
+    this.freeElementsByTagName.clear()
   }
 
   buildElement (tagName, className) {
-    let element = this.freeElementsByTagName[tagName] ? this.freeElementsByTagName[tagName].pop() : null
+    const elements = this.freeElementsByTagName.get(tagName)
+    let element = elements ? elements.pop() : null
     if (element) {
       for (let dataId in element.dataset) { delete element.dataset[dataId] }
       element.removeAttribute('style')
@@ -31,7 +29,8 @@ class DOMElementPool {
   }
 
   buildText (textContent) {
-    let element = this.freeElementsByTagName['#text'] ? this.freeElementsByTagName['#text'].pop() : null
+    const elements = this.freeElementsByTagName.get('#text')
+    let element = elements ? elements.pop() : null
     if (element) {
       element.textContent = textContent
       this.freedElements.delete(element)
@@ -43,7 +42,7 @@ class DOMElementPool {
 
   freeElementAndDescendants (element) {
     this.free(element)
-    return this.freeDescendants(element)
+    this.freeDescendants(element)
   }
 
   freeDescendants (element) {
@@ -59,10 +58,14 @@ class DOMElementPool {
     if (this.freedElements.has(element)) { throw new Error('The element has already been freed!') }
 
     const tagName = element.nodeName.toLowerCase()
-    if (this.freeElementsByTagName[tagName] == null) { this.freeElementsByTagName[tagName] = [] }
-    this.freeElementsByTagName[tagName].push(element)
+    let elements = this.freeElementsByTagName.get(tagName)
+    if (!elements) {
+      elements = []
+      this.freeElementsByTagName.set(tagName, elements)
+    }
+    elements.push(element)
     this.freedElements.add(element)
 
-    return element.remove()
+    element.remove()
   }
 }

From 2ad6f832394e5160a354407680ee9aa8686d748a Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 10:29:04 -0700
Subject: [PATCH 04/10] Allow metadata to be passed to atom.assert

---
 spec/atom-environment-spec.coffee | 5 +++++
 src/atom-environment.coffee       | 9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/spec/atom-environment-spec.coffee b/spec/atom-environment-spec.coffee
index 9b9715a07..d967fb97b 100644
--- a/spec/atom-environment-spec.coffee
+++ b/spec/atom-environment-spec.coffee
@@ -142,6 +142,11 @@ describe "AtomEnvironment", ->
           atom.assert(false, "a == b", (e) -> error = e)
           expect(error).toBe errors[0]
 
+      describe "if passed metadata", ->
+        it "assigns the metadata on the assertion failure's error object", ->
+          atom.assert(false, "a == b", {foo: 'bar'})
+          expect(errors[0].metadata).toEqual {foo: 'bar'}
+
     describe "if the condition is true", ->
       it "does nothing", ->
         result = atom.assert(true, "a == b")
diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee
index 2c637b0d6..013354028 100644
--- a/src/atom-environment.coffee
+++ b/src/atom-environment.coffee
@@ -821,12 +821,17 @@ class AtomEnvironment extends Model
   Section: Private
   ###
 
-  assert: (condition, message, callback) ->
+  assert: (condition, message, callbackOrMetadata) ->
     return true if condition
 
     error = new Error("Assertion failed: #{message}")
     Error.captureStackTrace(error, @assert)
-    callback?(error)
+
+    if callbackOrMetadata?
+      if typeof callbackOrMetadata is 'function'
+        callbackOrMetadata?(error)
+      else
+        error.metadata = callbackOrMetadata
 
     @emitter.emit 'did-fail-assertion', error
 

From 5753b75f096a52a2dcb874581633fdd3d824e118 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 10:30:51 -0700
Subject: [PATCH 05/10] Fail assertion with content metadata if the same
 element is freed twice

This changes the element pool to only remove elements' children right
before we use an element again in order to preserve the structure of
double-freed elements. This will aid in debugging double-free
occurrences. It's also less work in cases where nodes aren't reused.
---
 spec/dom-element-pool-spec.js | 23 +++++++++++++++++++++--
 src/dom-element-pool.js       | 25 ++++++++++++++-----------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/spec/dom-element-pool-spec.js b/spec/dom-element-pool-spec.js
index 11c8be0ff..52f4d772e 100644
--- a/spec/dom-element-pool-spec.js
+++ b/spec/dom-element-pool-spec.js
@@ -51,10 +51,29 @@ describe('DOMElementPool', function () {
     expect(domElementPool.buildElement('div')).not.toBe(div)
   })
 
-  it('throws an error when trying to free the same node twice', function () {
+  it('fails an assertion when freeing the same element twice', function () {
+    let failure
+    atom.onDidFailAssertion((error) => failure = error)
+
     const div = domElementPool.buildElement('div')
+    div.textContent = 'testing'
     domElementPool.freeElementAndDescendants(div)
-    expect(() => domElementPool.freeElementAndDescendants(div)).toThrow()
+    expect(failure).toBeUndefined()
+    domElementPool.freeElementAndDescendants(div)
+    expect(failure.message).toBe('Assertion failed: The element has already been freed!')
+    expect(failure.metadata.content).toBe('<div>testing</div>')
+  })
+
+  it('fails an assertion when freeing the same text node twice', function () {
+    let failure
+    atom.onDidFailAssertion((error) => failure = error)
+
+    const node = domElementPool.buildText('testing')
+    domElementPool.freeElementAndDescendants(node)
+    expect(failure).toBeUndefined()
+    domElementPool.freeElementAndDescendants(node)
+    expect(failure.message).toBe('Assertion failed: The element has already been freed!')
+    expect(failure.metadata.content).toBe('testing')
   })
 
   it('throws an error when trying to free an invalid element', function () {
diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index d8a5e3bb9..624e0b74f 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -21,6 +21,9 @@ class DOMElementPool {
       } else {
         element.removeAttribute('class')
       }
+      while (element.firstChild) {
+        element.removeChild(element.firstChild)
+      }
       this.freedElements.delete(element)
     } else {
       element = document.createElement(tagName)
@@ -42,20 +45,17 @@ class DOMElementPool {
 
   freeElementAndDescendants (element) {
     this.free(element)
-    this.freeDescendants(element)
-  }
-
-  freeDescendants (element) {
-    for (let i = element.childNodes.length - 1; i >= 0; i--) {
-      const descendant = element.childNodes[i]
-      this.free(descendant)
-      this.freeDescendants(descendant)
-    }
+    element.remove()
   }
 
   free (element) {
     if (element == null) { throw new Error('The element cannot be null or undefined.') }
-    if (this.freedElements.has(element)) { throw new Error('The element has already been freed!') }
+    if (this.freedElements.has(element)) {
+      atom.assert(false, 'The element has already been freed!', {
+        content: element instanceof Text ? element.textContent : element.outerHTML.toString()
+      })
+      return
+    }
 
     const tagName = element.nodeName.toLowerCase()
     let elements = this.freeElementsByTagName.get(tagName)
@@ -66,6 +66,9 @@ class DOMElementPool {
     elements.push(element)
     this.freedElements.add(element)
 
-    element.remove()
+    for (let i = element.childNodes.length - 1; i >= 0; i--) {
+      const descendant = element.childNodes[i]
+      this.free(descendant)
+    }
   }
 }

From 1528561c9bb1bf6a33aa391003b784bcfe50f49f Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 10:46:37 -0700
Subject: [PATCH 06/10] Only free elements that the DOMElementPool created

---
 spec/dom-element-pool-spec.js | 35 +++++++++++++++++++++++++----------
 src/dom-element-pool.js       |  5 +++++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/spec/dom-element-pool-spec.js b/spec/dom-element-pool-spec.js
index 52f4d772e..959003ca8 100644
--- a/spec/dom-element-pool-spec.js
+++ b/spec/dom-element-pool-spec.js
@@ -51,29 +51,44 @@ describe('DOMElementPool', function () {
     expect(domElementPool.buildElement('div')).not.toBe(div)
   })
 
+  it('does not attempt to free nodes that were not created by the pool', () => {
+    let assertionFailure
+    atom.onDidFailAssertion((error) => assertionFailure = error)
+
+    const foreignDiv = document.createElement('div')
+    const div = domElementPool.buildElement('div')
+    div.appendChild(foreignDiv)
+    domElementPool.freeElementAndDescendants(div)
+    const span = domElementPool.buildElement('span')
+    span.appendChild(foreignDiv)
+    domElementPool.freeElementAndDescendants(span)
+
+    expect(assertionFailure).toBeUndefined()
+  })
+
   it('fails an assertion when freeing the same element twice', function () {
-    let failure
-    atom.onDidFailAssertion((error) => failure = error)
+    let assertionFailure
+    atom.onDidFailAssertion((error) => assertionFailure = error)
 
     const div = domElementPool.buildElement('div')
     div.textContent = 'testing'
     domElementPool.freeElementAndDescendants(div)
-    expect(failure).toBeUndefined()
+    expect(assertionFailure).toBeUndefined()
     domElementPool.freeElementAndDescendants(div)
-    expect(failure.message).toBe('Assertion failed: The element has already been freed!')
-    expect(failure.metadata.content).toBe('<div>testing</div>')
+    expect(assertionFailure.message).toBe('Assertion failed: The element has already been freed!')
+    expect(assertionFailure.metadata.content).toBe('<div>testing</div>')
   })
 
   it('fails an assertion when freeing the same text node twice', function () {
-    let failure
-    atom.onDidFailAssertion((error) => failure = error)
+    let assertionFailure
+    atom.onDidFailAssertion((error) => assertionFailure = error)
 
     const node = domElementPool.buildText('testing')
     domElementPool.freeElementAndDescendants(node)
-    expect(failure).toBeUndefined()
+    expect(assertionFailure).toBeUndefined()
     domElementPool.freeElementAndDescendants(node)
-    expect(failure.message).toBe('Assertion failed: The element has already been freed!')
-    expect(failure.metadata.content).toBe('testing')
+    expect(assertionFailure.message).toBe('Assertion failed: The element has already been freed!')
+    expect(assertionFailure.metadata.content).toBe('testing')
   })
 
   it('throws an error when trying to free an invalid element', function () {
diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index 624e0b74f..683a1d247 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -1,11 +1,13 @@
 module.exports =
 class DOMElementPool {
   constructor () {
+    this.managedElements = new Set()
     this.freeElementsByTagName = new Map()
     this.freedElements = new Set()
   }
 
   clear () {
+    this.managedElements.clear()
     this.freedElements.clear()
     this.freeElementsByTagName.clear()
   }
@@ -27,6 +29,7 @@ class DOMElementPool {
       this.freedElements.delete(element)
     } else {
       element = document.createElement(tagName)
+      this.managedElements.add(element)
     }
     return element
   }
@@ -39,6 +42,7 @@ class DOMElementPool {
       this.freedElements.delete(element)
     } else {
       element = document.createTextNode(textContent)
+      this.managedElements.add(element)
     }
     return element
   }
@@ -50,6 +54,7 @@ class DOMElementPool {
 
   free (element) {
     if (element == null) { throw new Error('The element cannot be null or undefined.') }
+    if (!this.managedElements.has(element)) return
     if (this.freedElements.has(element)) {
       atom.assert(false, 'The element has already been freed!', {
         content: element instanceof Text ? element.textContent : element.outerHTML.toString()

From 6fa5c17cfbcc31038230826278c3ba6a7eb6d4dc Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 10:58:58 -0700
Subject: [PATCH 07/10] Add back DOMElementPool.freeDescendants

---
 src/dom-element-pool.js | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index 683a1d247..0486280fb 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -52,6 +52,13 @@ class DOMElementPool {
     element.remove()
   }
 
+  freeDescendants (element) {
+    while (element.firstChild) {
+      this.free(element.firstChild)
+      element.removeChild(element.firstChild)
+    }
+  }
+
   free (element) {
     if (element == null) { throw new Error('The element cannot be null or undefined.') }
     if (!this.managedElements.has(element)) return

From ee749bf2862cdf9e4bf834e8bd86be81b0b7fda6 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 11:27:44 -0700
Subject: [PATCH 08/10] Assign className in DOMElementPool when building new
 elements

Also, improve test coverage
---
 spec/dom-element-pool-spec.js | 16 +++++++++++++++-
 src/dom-element-pool.js       |  5 ++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/spec/dom-element-pool-spec.js b/spec/dom-element-pool-spec.js
index 959003ca8..9de932e27 100644
--- a/spec/dom-element-pool-spec.js
+++ b/spec/dom-element-pool-spec.js
@@ -8,7 +8,7 @@ describe('DOMElementPool', function () {
   it('builds DOM nodes, recycling them when they are freed', function () {
     let elements
     const [div, span1, span2, span3, span4, span5, textNode] = Array.from(elements = [
-      domElementPool.buildElement('div'),
+      domElementPool.buildElement('div', 'foo'),
       domElementPool.buildElement('span'),
       domElementPool.buildElement('span'),
       domElementPool.buildElement('span'),
@@ -17,6 +17,13 @@ describe('DOMElementPool', function () {
       domElementPool.buildText('Hello world!')
     ])
 
+    expect(div.className).toBe('foo')
+    div.textContent = 'testing'
+    div.style.backgroundColor = 'red'
+    div.dataset.foo = 'bar'
+
+    expect(textNode.textContent).toBe('Hello world!')
+
     div.appendChild(span1)
     span1.appendChild(span2)
     div.appendChild(span3)
@@ -37,6 +44,13 @@ describe('DOMElementPool', function () {
     expect(elements.includes(domElementPool.buildElement('div'))).toBe(false)
     expect(elements.includes(domElementPool.buildElement('span'))).toBe(false)
     expect(elements.includes(domElementPool.buildText('unexisting'))).toBe(false)
+
+    expect(div.className).toBe('')
+    expect(div.textContent).toBe('')
+    expect(div.style.backgroundColor).toBe('')
+    expect(div.dataset.foo).toBeUndefined()
+
+    expect(textNode.textContent).toBe('another text')
   })
 
   it('forgets free nodes after being cleared', function () {
diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index 0486280fb..808e71ab5 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -18,7 +18,7 @@ class DOMElementPool {
     if (element) {
       for (let dataId in element.dataset) { delete element.dataset[dataId] }
       element.removeAttribute('style')
-      if (className != null) {
+      if (className) {
         element.className = className
       } else {
         element.removeAttribute('class')
@@ -29,6 +29,9 @@ class DOMElementPool {
       this.freedElements.delete(element)
     } else {
       element = document.createElement(tagName)
+      if (className) {
+        element.className = className
+      }
       this.managedElements.add(element)
     }
     return element

From 3c525d98a2bb7b4eb37993fc5c59c94c7473bba3 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 11:49:21 -0700
Subject: [PATCH 09/10] Quality Text global

---
 src/dom-element-pool.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index 808e71ab5..e4f815c05 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -67,7 +67,7 @@ class DOMElementPool {
     if (!this.managedElements.has(element)) return
     if (this.freedElements.has(element)) {
       atom.assert(false, 'The element has already been freed!', {
-        content: element instanceof Text ? element.textContent : element.outerHTML.toString()
+        content: element instanceof window.Text ? element.textContent : element.outerHTML.toString()
       })
       return
     }

From e5c0dd1695062f463b86313cb35d051a0af1a168 Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Mon, 13 Feb 2017 11:49:46 -0700
Subject: [PATCH 10/10] Remove toString call

---
 src/dom-element-pool.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dom-element-pool.js b/src/dom-element-pool.js
index e4f815c05..0fef02dee 100644
--- a/src/dom-element-pool.js
+++ b/src/dom-element-pool.js
@@ -67,7 +67,7 @@ class DOMElementPool {
     if (!this.managedElements.has(element)) return
     if (this.freedElements.has(element)) {
       atom.assert(false, 'The element has already been freed!', {
-        content: element instanceof window.Text ? element.textContent : element.outerHTML.toString()
+        content: element instanceof window.Text ? element.textContent : element.outerHTML
       })
       return
     }