Specs: Drop @nonnull from dom.md, drop ChildNode from dom.md, drop TreeRoot...

Specs: Drop @nonnull from dom.md, drop ChildNode from dom.md, drop TreeRoot from dom.md, other minor idomatic changes

Review URL: https://codereview.chromium.org/927613002

specs/dom.md → specs/elements.md

@@ -8,44 +8,49 @@ SKY MODULE
 <script>
 // ELEMENT TREE API
 
-abstract class ChildNode {
-  @nonnull external TreeScope get ownerScope; // O(1)
+abstract class Node extends EventTarget {
+  @override
+  external List<EventTarget> getEventDispatchChain(); // O(N) in number of ancestors across shadow trees
+  // implements EventTarget.getEventDispatchChain()
+  // returns the event dispatch chain (including handling shadow trees)
+
+  external Root get owner; // O(1)
 
   external ParentNode get parentNode; // O(1)
   external Element get parentElement; // O(1) // if parentNode isn't an element, returns null
-  external ChildNode get previousSibling; // O(1)
-  external ChildNode get nextSibling; // O(1)
+  external Node get previousSibling; // O(1)
+  external Node get nextSibling; // O(1)
 
   // the following all throw if parentNode is null
-  external void insertBefore(@nonnull List</*@nonnull*/ ChildNode> nodes); // O(N) in number of arguments plus all their descendants
-  external void insertAfter(@nonnull List</*@nonnull*/ ChildNode> nodes); // O(N) in number of arguments plus all their descendants
-  external void replaceWith(@nonnull List</*@nonnull*/ ChildNode> nodes); // O(N) in number of descendants plus arguments plus all their descendants
+  external void insertBefore(List nodes); // O(N) in number of arguments plus all their descendants
+  external void insertAfter(List nodes); // O(N) in number of arguments plus all their descendants
+  external void replaceWith(List nodes); // O(N) in number of descendants plus arguments plus all their descendants
+  // nodes must be String, Text, or Element
+
   external void remove(); // O(N) in number of descendants
 
   // called when parentNode changes
-  external void parentChangeCallback(ParentNode oldParent, ParentNode newParent, ChildNode previousSibling, ChildNode nextSibling); // O(N) in descendants
+  // this is why insertBefore(), append(), et al, are O(N) -- the whole affected subtree is walked
+  // mutating the element tree from within this is strongly discouraged, since it will result in the
+  // callbacks being invoked while the element tree is in a different state than implied by the callbacks
+  external void parentChangeCallback(ParentNode oldParent, ParentNode newParent, Node previousSibling, Node nextSibling); // O(N) in descendants
   // default implementation calls attached/detached
   void attachedCallback() { }
   void detachedCallback() { }
 
   external List<ContentElement> getDestinationInsertionPoints(); // O(N) in number of insertion points the node is in
   // returns the <content> elements to which this element was distributed
-}
-
-abstract class Node extends EventTarget {
-  @override
-  external List</*@nonnull*/ EventTarget> getEventDispatchChain(); // O(N) in number of ancestors across shadow trees
-  // implements EventTarget.getEventDispatchChain()
-  // returns the event dispatch chain (including handling shadow trees)
 
   external Node cloneNode({bool deep: false}); // O(1) if deep=false, O(N) in the number of descendants if deep=true
 
   external ElementStyleDeclarationList get style; // O(1)
-  // for nodes that aren't reachable from the Application Document, returns null
-  // (so in particular orphaned subtrees and nodes in module documents don't have one)
-  //  -- should be updated when the node's parent chain changes (same time as, e.g.,
-  //     the id hashtable is updated)
-  // also always returns null for ContentElement elements and ShadowRoot nodes
+  // for nodes that aren't in the ApplicationRoot's composed tree,
+  // returns null (so in particular orphaned subtrees and nodes in
+  // module Roots don't have one, nor do shadow tree Roots)
+  // also always returns null for ContentElement elements
+  //  -- should be (lazily) updated when the node's parent chain
+  //     changes (same time as, e.g., the id hashtable is marked
+  //     dirty)
 
   external RenderNode get renderNode; // O(1)
   // this will be null until the first time it is rendered
@@ -62,17 +67,19 @@ abstract class Node extends EventTarget {
 }
 
 abstract class ParentNode extends Node {
-  external ChildNode get firstChild; // O(1)
-  external ChildNode get lastChild; // O(1)
+  external Node get firstChild; // O(1)
+  external Node get lastChild; // O(1)
 
   // Returns a new List every time.
-  external List</*nonnull*/ ChildNode> getChildNodes(); // O(N) in number of child nodes
+  external List<Node> getChildren(); // O(N) in number of child nodes
   external List<Element> getChildElements(); // O(N) in number of child nodes
   // TODO(ianh): might not be necessary if we have the parser drop unnecessary whitespace text nodes
 
-  external void append(@nonnull List</*nonnull*/ ChildNode> nodes); // O(N) in number of arguments plus all their descendants
-  external void prepend(@nonnull List</*nonnull*/ ChildNode> nodes); // O(N) in number of arguments plus all their descendants
-  external void replaceChildrenWith(@nonnull List</*nonnull*/ ChildNode> nodes); // O(N) in number of descendants plus arguments plus all their descendants
+  external void append(List nodes); // O(N) in number of arguments plus all their descendants
+  external void appendSingle(Node nodes); // O(N) in number of descandants
+  external void prepend(List nodes); // O(N) in number of arguments plus all their descendants
+  external void replaceChildrenWith(List nodes); // O(N) in number of descendants plus arguments plus all their descendants
+  // nodes must be String, Text, or Element
 }
 
 class Attr {
@@ -85,7 +92,7 @@ class Attr {
 // only useful when placed on classes that inherit from Element
 class tagname extends AutomaticMetadata {
   const tagname(this.name);
-  @nonnull final String name;
+  final String name;
   void init(DeclarationMirror target, Module module) {
     assert(target is ClassMirror);
     if (!target.isSubclassOf(reflectClass(Element)))
@@ -94,32 +101,33 @@ class tagname extends AutomaticMetadata {
   }
 }
 
-abstract class FindRoot { }
-
-abstract class Element extends ParentNode with ChildNode implements FindRoot {
-  external Element({Map</*@nonnull*/ String, /*@nonnull*/ String> attributes: null,
-                   List</*nonnull*/ ChildNode> nodes: null,
-                   Module hostModule: null}); // O(M+N), M = number of attributes, N = number of nodes plus all their descendants
+abstract class Element extends ParentNode with Node {
+  external Element({Map<String, String> attributes: null,
+                   List children: null,
+                   Module hostModule: null}); // O(M+N), M = number of attributes, N = number of children nodes plus all their descendants
   // initialises the internal attributes table
-  // appends the given child nodes
+  // appends the given children nodes
+  // children must be String, Text, or Element
   // if this.needsShadow, creates a shadow tree
 
-  @nonnull String get tagName { // O(N) in number of annotations on the class
+  String get tagName { // O(N) in number of annotations on the class
     // throws a StateError if the class doesn't have an @tagname annotation
     var tagnameClass = reflectClass(tagname);
     return (reflectClass(this.runtimeType).metadata.singleWhere((mirror) => mirror.type == tagnameClass).reflectee as tagname).value;
   }
 
-  @nonnull external bool hasAttribute(@nonnull String name); // O(N) in number of attributes
-  @nonnull external String getAttribute(@nonnull String name); // O(N) in number of attributes
-  external void setAttribute(@nonnull String name, [@nonnull String value = '']); // O(N) in number of attributes
-  external void removeAttribute(@nonnull String name); // O(N) in number of attributes
+  external bool hasAttribute(String name); // O(N) in number of attributes
+  external String getAttribute(String name); // O(N) in number of attributes
+  external void setAttribute(String name, [String value = '']); // O(N) in number of attributes
+  external void removeAttribute(String name); // O(N) in number of attributes
+  // calling setAttribute() with a null value removes the attribute
+  // (calling it without a value sets it to the empty string)
 
   // Returns a new Array and new Attr instances every time.
-  @nonnull external List<Attr> getAttributes(); // O(N) in number of attributes
+  external List<Attr> getAttributes(); // O(N) in number of attributes
 
   get bool needsShadow => false; // O(1)
-  external ShadowRoot get shadowRoot; // O(1)
+  external Root get shadowRoot; // O(1)
   // returns the shadow root
   // TODO(ianh): Should this be mutable? It would help explain how it gets set...
 
@@ -137,44 +145,36 @@ abstract class Element extends ParentNode with ChildNode implements FindRoot {
   }
 }
 
-class Text extends Node with ChildNode {
-  external Text([@nonnull String value = '']); // O(1)
+class Text extends Node with Node {
+  external Text([String value = '']); // O(1)
 
-  @nonnull external String get value; // O(1)
-  external void set (@nonnull String value); // O(1)
+  external String get value; // O(1)
+  external void set (String value); // O(1)
 
-  void valueChangeCallback(@nonnull String oldValue, @nonnull String newValue) { }
+  void valueChangeCallback(String oldValue, String newValue) { }
 
   @override
   Type getLayoutManager() => TextLayoutManager; // O(1)
 }
 
-class DocumentFragment extends ParentNode implements FindRoot {
-  DocumentFragment([List</*nonnull*/ ChildNode> nodes = null]); // O(N) in number of arguments plus all their descendants
+class Fragment extends ParentNode {
+  Fragment({List children}); // O(N) in number of arguments plus all their descendants
+  // children must be String, Text, or Element
 }
 
-abstract class TreeScope extends ParentNode {
-  external Document get ownerDocument; // O(1)
-  external TreeScope get parentScope; // O(1)
+class Root extends ParentNode {
+  external Root ({List children, Element host}); // O(N) in number of children nodes arguments plus all their descendants
+  // children must be String, Text, or Element
+
+  final Element host;
 
   external Element findId(String id); // O(1)
   // throws if id is null
 }
 
-class ShadowRoot extends TreeScope implements FindRoot {
-  ShadowRoot([this._host]); // O(1)
-  // note that there is no way in the API to use a newly created ShadowRoot currently
-
-  Element _host;
-  Element get host => _host; // O(1)
-}
-
-class Document extends TreeScope implements FindRoot {
-  external Document ([List</*@nonnull*/ ChildNode> nodes = null]); // O(N) in number of arguments plus all their descendants
-}
-
-class ApplicationDocument extends Document {
-  external ApplicationDocument ([List</*@nonnull*/ ChildNode> nodes = null]); // O(N) in number of /nodes/ arguments plus all their descendants
+class ApplicationRoot extends Root {
+  external ApplicationRoot ({List children}) : Root(children: children); // O(N) in number of children nodes arguments plus all their descendants
+  // children must be String, Text, or Element
 
   @override
   Type getLayoutManager() => rootLayoutManager; // O(1)
@@ -197,9 +197,9 @@ class ImportElement extends Element {
 class TemplateElement extends Element {
   TemplateElement = Element;
 
-  // TODO(ianh): convert <template> to using a token stream instead of a DocumentFragment
+  // TODO(ianh): convert <template> to using a token stream instead of a Fragment
 
-  @nonnull external DocumentFragment get content; // O(1)
+  external Fragment get content; // O(1)
 
   @override
   Type getLayoutManager() => null; // O(1)
@@ -217,7 +217,7 @@ class ScriptElement extends Element {
 class StyleElement extends Element {
   StyleElement = Element;
 
-  @nonnull external List</*@nonnull*/ Rule> getRules(); // O(N) in rules
+  external List<Rule> getRules(); // O(N) in rules
 
   @override
   Type getLayoutManager() => null; // O(1)
@@ -227,7 +227,7 @@ class StyleElement extends Element {
 class ContentElement extends Element {
   ContentElement = Element;
 
-  @nonnull external List</*@nonnull*/ Node> getDistributedNodes(); // O(N) in distributed nodes
+  external List<Node> getDistributedNodes(); // O(N) in distributed nodes
 
   @override
   Type getLayoutManager() => null; // O(1)
@@ -285,11 +285,15 @@ class _ErrorElement extends Element {
 }
 
 class SelectorQuery {
-  external SelectorQuery(@nonnull String selector); // O(F()) where F() is the complexity of the selector
-
-  @nonnull external bool matches(@nonnull Element element); // O(F())
-  external Element find(@nonnull FindRoot root); // O(N*F())+O(M) where N is the number of descendants and M the average depth of the tree
-  @nonnull external List</*@nonnull*/ Element> findAll(FindRoot root); // O(N*F())+O(N*M) where N is the number of descendants and M the average depth of the tree
+  external SelectorQuery(String selector); // O(F()) where F() is the complexity of the selector
+
+  external bool matches(Element element); // O(F())
+  external Element find(node root); // O(N*F())+O(M) where N is the number of descendants and M the average depth of the tree
+  external List<Element> findAll(Node root); // O(N*F())+O(N*M) where N is the number of descendants and M the average depth of the tree
+  // find() and findAll() throw if the root is not one of the following:
+  //  - Element
+  //  - Fragment
+  //  - Root
 }
 </script>
 ```

specs/events.md

@@ -131,6 +131,9 @@ abstract class Event<ReturnType> {
   // TODO(ianh): abstract API for doing things at shadow tree boundaries 
   // TODO(ianh): do events get blocked at scope boundaries, e.g. focus events when both sides are in the scope?
   // TODO(ianh): do events get retargetted, e.g. focus when leaving a custom element?
+  // e.g. sent from inside a shadow tree, when exiting the shadow tree, focus event should:
+  //  - disappear if we're moving from one to another element
+  //  - be targetted if it's going to another node in a different scope
 }
 
 class EventTarget {