dom: optimize element size, remove importc hack & dead code

This switches CAtom to uint32; it seems better to use the same size on
all platforms.

4 files changed, 85 insertions, 111 deletions

diff --git a/src/css/stylednode.nim b/src/css/stylednode.nim
index 127ce41c..cf9a8a59 100644
--- a/src/css/stylednode.nim
+++ b/src/css/stylednode.nim
@@ -58,19 +58,19 @@ type
 template textData*(styledNode: StyledNode): string =
   CharacterData(styledNode.node).data
 
-# For debugging
-func `$`*(node: StyledNode): string =
-  if node == nil:
-    return "nil"
-  case node.t
-  of stText:
-    return "#text " & node.textData
-  of stElement:
-    if node.node != nil:
-      return $node.node
-    return $node.pseudo
-  of stReplacement:
-    return "#replacement"
+when defined(debug):
+  func `$`*(node: StyledNode): string =
+    if node == nil:
+      return "nil"
+    case node.t
+    of stText:
+      return "#text " & node.textData
+    of stElement:
+      if node.node != nil:
+        return $node.node
+      return $node.pseudo
+    of stReplacement:
+      return "#replacement"
 
 iterator branch*(node: StyledNode): StyledNode {.inline.} =
   var node = node
diff --git a/src/html/catom.nim b/src/html/catom.nim
index 3590bc63..93fbafcb 100644
--- a/src/html/catom.nim
+++ b/src/html/catom.nim
@@ -124,7 +124,7 @@ static:
   doAssert (CAtomFactoryStrMapLength and (CAtomFactoryStrMapLength - 1)) == 0
 
 type
-  CAtom* = distinct int
+  CAtom* = distinct uint32
 
   CAtomFactoryInit = object
     obj: CAtomFactoryObj
diff --git a/src/html/chadombuilder.nim b/src/html/chadombuilder.nim
index bedd0d66..7b8c2102 100644
--- a/src/html/chadombuilder.nim
+++ b/src/html/chadombuilder.nim
@@ -22,12 +22,10 @@ type CharsetConfidence* = enum
   ccTentative, ccCertain, ccIrrelevant
 
 type
-  HTML5ParserWrapper* = ref object
+  HTML5ParserWrapper* = ref object of RootObj
     parser: HTML5Parser[Node, CAtom]
     builder*: ChaDOMBuilder
     opts: HTML5ParserOpts[Node, CAtom]
-    # hack so we don't have to worry about leaks or the GC deallocating parser
-    refs: seq[Document]
     stoppedFromScript: bool
 
   ChaDOMBuilder = ref object of DOMBuilder[Node, CAtom]
@@ -37,7 +35,6 @@ type
     factory: CAtomFactory
     poppedScript: HTMLScriptElement
 
-type
   DOMBuilderImpl = ChaDOMBuilder
   HandleImpl = Node
   AtomImpl = CAtom
@@ -48,10 +45,8 @@ type DOMParser = ref object # JS interface
 
 jsDestructor(DOMParser)
 
-#TODO this is disgusting and should be removed
 proc setActiveParser(document: Document; wrapper: HTML5ParserWrapper) =
-  document.parser = cast[pointer](wrapper)
-  wrapper.refs.add(document)
+  document.parser = wrapper
 
 proc getDocumentImpl(builder: ChaDOMBuilder): Node =
   return builder.document
@@ -77,7 +72,6 @@ proc restart*(wrapper: HTML5ParserWrapper; charset: Charset) =
   let document = newDocument(builder.factory)
   document.charset = charset
   document.setActiveParser(wrapper)
-  wrapper.refs.add(document)
   document.contentType = "text/html"
   let oldDocument = builder.document
   document.url = oldDocument.url
@@ -262,9 +256,6 @@ proc parseHTMLFragment*(element: Element; s: string): seq[Node] =
   builder.finish()
   return root.childList
 
-# Forward declaration hack
-domParseHTMLFragment = parseHTMLFragment
-
 proc newHTML5ParserWrapper*(window: Window; url: URL; factory: CAtomFactory;
     confidence: CharsetConfidence; charset: Charset): HTML5ParserWrapper =
   let opts = HTML5ParserOpts[Node, CAtom](scripting: window.settings.scripting)
@@ -277,6 +268,9 @@ proc newHTML5ParserWrapper*(window: Window; url: URL; factory: CAtomFactory;
   builder.document.setActiveParser(wrapper)
   return wrapper
 
+template toOA(writeBuffer: DocumentWriteBuffer): openArray[char] =
+  writeBuffer.data.toOpenArray(writeBuffer.i, writeBuffer.data.high)
+
 proc parseBuffer*(wrapper: HTML5ParserWrapper; buffer: openArray[char]):
     ParseResult =
   let builder = wrapper.builder
@@ -312,8 +306,8 @@ proc parseBuffer*(wrapper: HTML5ParserWrapper; buffer: openArray[char]):
 
 # Called from dom whenever document.write is executed.
 # We consume everything pushed into the top buffer.
-proc CDB_parseDocumentWriteChunk(wrapper: pointer) {.exportc.} =
-  let wrapper = cast[HTML5ParserWrapper](wrapper)
+proc parseDocumentWriteChunk(wrapper: RootRef) =
+  let wrapper = HTML5ParserWrapper(wrapper)
   let builder = wrapper.builder
   let document = builder.document
   let buffer = document.writeBuffers[^1]
@@ -345,9 +339,6 @@ proc CDB_parseDocumentWriteChunk(wrapper: pointer) {.exportc.} =
 proc finish*(wrapper: HTML5ParserWrapper) =
   wrapper.parser.finish()
   wrapper.builder.finish()
-  for r in wrapper.refs:
-    r.parser = nil
-  wrapper.refs.setLen(0)
 
 proc newDOMParser*(): DOMParser {.jsctor.} =
   return DOMParser()
@@ -373,5 +364,9 @@ proc parseFromString*(ctx: JSContext; parser: DOMParser; str, t: string):
   else:
     return errTypeError("Invalid mime type")
 
+# Forward declaration hack
+domParseHTMLFragment = parseHTMLFragment
+domParseDocumentWriteChunk = parseDocumentWriteChunk
+
 proc addHTMLModule*(ctx: JSContext) =
   ctx.registerType(DOMParser)
diff --git a/src/html/dom.nim b/src/html/dom.nim
index 2a4e2d97..8462f29a 100644
--- a/src/html/dom.nim
+++ b/src/html/dom.nim
@@ -161,7 +161,7 @@ type
     # Live collection cache: pointers to live collections are saved in all
     # nodes they refer to. These are removed when the collection is destroyed,
     # and invalidated when the owner node's children or attributes change.
-    liveCollections: HashSet[pointer]
+    liveCollections: seq[pointer]
     cachedChildNodes: NodeList
     internalDocument: Document # not nil
 
@@ -211,8 +211,7 @@ type
     cachedSheetsInvalid*: bool
     cachedChildren: HTMLCollection
     cachedForms: HTMLCollection
-    #TODO I hate this but I really don't want to put chadombuilder into dom too
-    parser*: pointer
+    parser*: RootRef
 
   CharacterData* = ref object of Node
     data* {.jsget.}: string
@@ -245,21 +244,19 @@ type
   Element* = ref object of Node
     namespace*: Namespace
     namespacePrefix*: NamespacePrefix
-    prefix*: string
+    internalHover: bool
+    invalid*: bool
+    # The owner StyledNode is marked as invalid when one of these no longer
+    # matches the DOM value.
+    invalidDeps*: set[DependencyType]
     localName*: CAtom
-
     id* {.jsget.}: CAtom
-    name* {.jsget.}: CAtom
+    name {.jsget.}: CAtom
     classList* {.jsget.}: DOMTokenList
     attrs*: seq[AttrData] # sorted by int(qualifiedName)
-    internalAttributes: NamedNodeMap
-    internalHover: bool
-    invalid*: bool
+    cachedAttributes: NamedNodeMap
     cachedStyle*: CSSStyleDeclaration
     cachedChildren: HTMLCollection
-    # The owner StyledNode is marked as invalid when one of these no longer
-    # matches the DOM value.
-    invalidDeps*: set[DependencyType]
 
   AttrDummyElement = ref object of Element
 
@@ -485,6 +482,7 @@ var doqs*: proc (node: Node; q: string): Element {.nimcall.} = nil
 # set in html/chadombuilder
 var domParseHTMLFragment*: proc(element: Element; s: string): seq[Node]
   {.nimcall.}
+var domParseDocumentWriteChunk*: proc(wrapper: RootRef) {.nimcall.}
 # set in html/env
 var windowFetch*: proc(window: Window; input: JSValue;
   init = RequestInit(window: JS_UNDEFINED)): JSResult[FetchPromise]
@@ -519,7 +517,7 @@ proc reset(state: var DrawingState) =
   state.strokeStyle = rgba(0, 0, 0, 255)
   state.path = newPath()
 
-proc create2DContext*(jctx: JSContext; target: HTMLCanvasElement;
+proc create2DContext(jctx: JSContext; target: HTMLCanvasElement;
     options = JS_UNDEFINED) =
   var pipefd: array[2, cint]
   if pipe(pipefd) == -1:
@@ -1112,36 +1110,36 @@ func escapeText(s: string; attribute_mode = false): string =
     else:
       result &= c
 
-func `$`*(node: Node): string =
-  # Note: this function should only be used for debugging.
-  if node == nil:
-    return "null"
-  if node of Element:
-    let element = Element(node)
-    result = "<" & element.localNameStr
-    for attr in element.attrs:
-      let k = element.document.toStr(attr.localName)
-      result &= ' ' & k & "=\"" & attr.value.escapeText(true) & "\""
-    result &= ">\n"
-    for node in element.childList:
-      for line in ($node).split('\n'):
-        result &= "\t" & line & "\n"
-    result &= "</" & element.localNameStr & ">"
-  elif node of Text:
-    let text = Text(node)
-    result = text.data.escapeText()
-  elif node of Comment:
-    result = "<!-- " & Comment(node).data & "-->"
-  elif node of ProcessingInstruction:
-    result = "" #TODO
-  elif node of DocumentType:
-    result = "<!DOCTYPE" & ' ' & DocumentType(node).name & ">"
-  elif node of Document:
-    result = "Node of Document"
-  elif node of DocumentFragment:
-    result = "Node of DocumentFragment"
-  else:
-    result = "Unknown node"
+when defined(debug):
+  func `$`*(node: Node): string =
+    if node == nil:
+      return "null"
+    if node of Element:
+      let element = Element(node)
+      result = "<" & element.localNameStr
+      for attr in element.attrs:
+        let k = element.document.toStr(attr.localName)
+        result &= ' ' & k & "=\"" & attr.value.escapeText(true) & "\""
+      result &= ">\n"
+      for node in element.childList:
+        for line in ($node).split('\n'):
+          result &= "\t" & line & "\n"
+      result &= "</" & element.localNameStr & ">"
+    elif node of Text:
+      let text = Text(node)
+      result = text.data.escapeText()
+    elif node of Comment:
+      result = "<!-- " & Comment(node).data & "-->"
+    elif node of ProcessingInstruction:
+      result = "" #TODO
+    elif node of DocumentType:
+      result = "<!DOCTYPE" & ' ' & DocumentType(node).name & ">"
+    elif node of Document:
+      result = "Node of Document"
+    elif node of DocumentFragment:
+      result = "Node of DocumentFragment"
+    else:
+      result = "Unknown node"
 
 func parentElement*(node: Node): Element {.jsfget.} =
   let p = node.parentNode
@@ -1258,15 +1256,16 @@ proc populateCollection(collection: Collection) =
         collection.snapshot.add(desc)
   if collection.islive:
     for child in collection.snapshot:
-      child.liveCollections.incl(collection.id)
-    collection.root.liveCollections.incl(collection.id)
+      child.liveCollections.add(collection.id)
+    collection.root.liveCollections.add(collection.id)
 
 proc refreshCollection(collection: Collection) =
   let document = collection.root.document
   if collection.id in document.invalidCollections:
     for child in collection.snapshot:
-      assert collection.id in child.liveCollections
-      child.liveCollections.excl(collection.id)
+      let i = child.liveCollections.find(collection.id)
+      assert i != -1
+      child.liveCollections.del(i)
     collection.snapshot.setLen(0)
     collection.populateCollection()
     document.invalidCollections.excl(collection.id)
@@ -1274,8 +1273,9 @@ proc refreshCollection(collection: Collection) =
 proc finalize0(collection: Collection) =
   if collection.islive:
     for child in collection.snapshot:
-      assert collection.id in child.liveCollections
-      child.liveCollections.excl(collection.id)
+      let i = child.liveCollections.find(collection.id)
+      assert i != -1
+      child.liveCollections.del(i)
     collection.root.document.invalidCollections.excl(collection.id)
 
 proc finalize(collection: HTMLCollection) {.jsfin.} =
@@ -1889,17 +1889,17 @@ func hasAttributes(element: Element): bool {.jsfunc.} =
   return element.attrs.len > 0
 
 func attributes(element: Element): NamedNodeMap {.jsfget.} =
-  if element.internalAttributes != nil:
-    return element.internalAttributes
-  element.internalAttributes = NamedNodeMap(element: element)
+  if element.cachedAttributes != nil:
+    return element.cachedAttributes
+  element.cachedAttributes = NamedNodeMap(element: element)
   for i, attr in element.attrs.mpairs:
-    element.internalAttributes.attrlist.add(Attr(
+    element.cachedAttributes.attrlist.add(Attr(
       internalDocument: element.document,
       index: -1,
       dataIdx: i,
       ownerElement: element
     ))
-  return element.internalAttributes
+  return element.cachedAttributes
 
 func findAttr(element: Element; qualifiedName: string): int =
   return element.findAttr(element.normalizeAttrQName(qualifiedName))
@@ -2016,7 +2016,7 @@ func scriptingEnabled*(document: Document): bool =
     return false
   return document.window.settings.scripting
 
-func scriptingEnabled*(element: Element): bool =
+func scriptingEnabled(element: Element): bool =
   return element.document.scriptingEnabled
 
 func isSubmitButton*(element: Element): bool =
@@ -2045,18 +2045,6 @@ func canSubmitImplicitly*(form: HTMLFormElement): bool =
       return false
   return true
 
-func qualifiedName*(element: Element): string =
-  if element.namespacePrefix != NO_PREFIX:
-    $element.namespacePrefix & ':' & element.localNameStr
-  else:
-    element.localNameStr
-
-template toOA*(writeBuffer: DocumentWriteBuffer): openArray[char] =
-  writeBuffer.data.toOpenArray(writeBuffer.i, writeBuffer.data.high)
-
-#TODO :(
-proc CDB_parseDocumentWriteChunk(wrapper: pointer) {.importc.}
-
 # https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-write-steps
 proc write(ctx: JSContext; document: Document; args: varargs[JSValue]):
     Err[DOMException] {.jsfunc.} =
@@ -2080,7 +2068,7 @@ proc write(ctx: JSContext; document: Document; args: varargs[JSValue]):
     text &= s
   buffer.data &= text
   if document.parserBlockingScript == nil:
-    CDB_parseDocumentWriteChunk(document.parser)
+    domParseDocumentWriteChunk(document.parser)
   return ok()
 
 func findFirst*(document: Document; tagType: TagType): HTMLElement =
@@ -2658,15 +2646,6 @@ proc parseColor(element: Element; s: string): ARGBColor =
     return ec
   return color.argbcolor
 
-#TODO ??
-func target0*(element: Element): string =
-  if element.attrb(satTarget):
-    return element.attr(satTarget)
-  for base in element.document.elements(TAG_BASE):
-    if base.attrb(satTarget):
-      return base.attr(satTarget)
-  return ""
-
 # HTMLHyperlinkElementUtils (for <a> and <area>)
 func href0(element: HTMLElement): string =
   if not element.attrb(satHref):
@@ -3023,7 +3002,7 @@ proc setInvalid*(element: Element) =
     element.document.invalid = true
 
 proc delAttr(element: Element; i: int; keep = false) =
-  let map = element.internalAttributes
+  let map = element.cachedAttributes
   let name = element.attrs[i].qualifiedName
   element.attrs.delete(i) # ordering matters
   if map != nil:
@@ -3416,7 +3395,7 @@ func cmpAttrName(a: AttrData; b: CAtom): int =
 # Returns the attr index if found, or the negation - 1 of an upper bound
 # (where a new attr with the passed name may be inserted).
 func findAttrOrNext(element: Element; qualName: CAtom): int =
-  for i, data in element.attrs:
+  for i, data in element.attrs.mpairs:
     if data.qualifiedName == qualName:
       return i
     if int(data.qualifiedName) > int(qualName):
@@ -3954,7 +3933,7 @@ proc reset*(form: HTMLFormElement) =
     control.resetElement()
     control.setInvalid()
 
-proc renderBlocking*(element: Element): bool =
+proc renderBlocking(element: Element): bool =
   if "render" in element.attr(satBlocking).split(AsciiWhitespace):
     return true
   if element of HTMLScriptElement:
@@ -3964,7 +3943,7 @@ proc renderBlocking*(element: Element): bool =
       return true
   return false
 
-proc blockRendering*(element: Element) =
+proc blockRendering(element: Element) =
   let document = element.document
   if document.contentType == "text/html" and document.body == nil:
     element.document.renderBlockingElements.add(element)