Custom Elements: Check Definition in createElement, Create Customized Built-in Elements Sync

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index 116247e4c2fc00350ce0e72da6c5148563fbbfff..6883ecda343db3801a519d57bd1cf05bf7b577a8 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -44,6 +44,7 @@
 #include "bindings/core/v8/V8PerIsolateData.h"
 #include "bindings/core/v8/WindowProxy.h"
 #include "core/HTMLElementFactory.h"
+#include "core/HTMLElementTypeHelpers.h"
 #include "core/HTMLNames.h"
 #include "core/SVGElementFactory.h"
 #include "core/SVGNames.h"
@@ -112,6 +113,8 @@
 #include "core/dom/VisitedLinkState.h"
 #include "core/dom/XMLDocument.h"
 #include "core/dom/custom/CustomElement.h"
+#include "core/dom/custom/CustomElementDefinition.h"
+#include "core/dom/custom/CustomElementDescriptor.h"
 #include "core/dom/custom/CustomElementRegistry.h"
 #include "core/dom/custom/V0CustomElementMicrotaskRunQueue.h"
 #include "core/dom/custom/V0CustomElementRegistrationContext.h"
@@ -710,9 +713,12 @@ String getTypeExtension(Document* document,
   return emptyString();
 }
 
+// https://dom.spec.whatwg.org/#dom-document-createelement
 Element* Document::createElement(const AtomicString& localName,
                                  const StringOrDictionary& stringOrOptions,
                                  ExceptionState& exceptionState) {
+  bool isV1 = stringOrOptions.isDictionary() || !registrationContext();

[dominicc (has gone to gerrit), 2016/11/15 07:39:53]

This v0, v1 split thing is very tricky.

The way you have it here makes sense--for v0, use createElement(string, string);
for v1, use createElement(string, dictionary).

The downside with that is that authors have to switch from document.register and
createElement(string,string) to customElements.define and createElement(string,
dictionary) in one go, which will be hard.

On the other hand it has many benefits. You can implement the synchronous
exception for missing definitions accurately, for example.

I think you have made the right decision. I've updated a slide deck describing
custom elements stuff to allude to this and when it lands I'll send mail to
devrel and some users.

In hindsight I'm a bit sad with how the spec for customized built-in elements
has turned out; throwing an exception instead of doing upgrade will make them
difficult to use.

+  // 1. If localName does not match Name production, throw InvalidCharacterError
   if (!isValidName(localName)) {
     exceptionState.throwDOMException(
         InvalidCharacterError,
@@ -720,26 +726,54 @@ Element* Document::createElement(const AtomicString& localName,
     return nullptr;
   }
 
+  // 2. localName converted to ASCII lowercase
+  const AtomicString& convertedLocalName = convertLocalName(localName);
+
+  // 3.
+  const AtomicString& is = convertLocalName(
+      AtomicString(getTypeExtension(this, stringOrOptions, exceptionState)));
+  const AtomicString& name = is.isNull() ? convertedLocalName : is;
+
+  // 4. Let definition be result of lookup up custom element definition
+  CustomElementDefinition* definition = nullptr;
+  if (isV1) {
+    // Is the runtime flag enabled for customized builtin elements?
+    const CustomElementDescriptor desc =
+        RuntimeEnabledFeatures::customElementsBuiltinEnabled()
+            ? CustomElementDescriptor(name, convertedLocalName)
+            : CustomElementDescriptor(convertedLocalName, convertedLocalName);
+    definition = CustomElement::registry(*this)->definitionFor(desc);
+    // 5. If 'is' is non-null and definition is null, throw NotFoundError
+    if (!definition && !is.isNull() && stringOrOptions.isDictionary()) {
+      exceptionState.throwDOMException(NotFoundError,
+                                       "Custom element definition not found.");
+      return nullptr;
+    }
+  }
+
+  // 7. Let element be the result of creating an element
   Element* element;
 
-  if (CustomElement::shouldCreateCustomElement(convertLocalName(localName))) {
-    element = CustomElement::createCustomElementSync(
-        *this, convertLocalName(localName));
+  if (definition) {
+    element =
+        CustomElement::createCustomElementSync(*this, convertedLocalName, name);
   } else if (V0CustomElement::isValidName(localName) && registrationContext()) {
     element = registrationContext()->createCustomTagElement(
-        *this, QualifiedName(nullAtom, convertLocalName(localName),
-                             xhtmlNamespaceURI));
+        *this, QualifiedName(nullAtom, convertedLocalName, xhtmlNamespaceURI));
   } else {
     element = createElement(localName, exceptionState);
     if (exceptionState.hadException())
       return nullptr;
   }
 
-  String typeExtention =
-      getTypeExtension(this, stringOrOptions, exceptionState);
-  if (!typeExtention.isEmpty()) {
-    V0CustomElementRegistrationContext::setIsAttributeAndTypeExtension(
-        element, AtomicString(typeExtention));
+  // 8. If 'is' is non-null, set 'is' attribute
+  if (!is.isEmpty()) {
+    if (stringOrOptions.isString()) {
+      V0CustomElementRegistrationContext::setIsAttributeAndTypeExtension(
+          element, is);
+    } else if (stringOrOptions.isDictionary()) {
+      element->setAttribute(HTMLNames::isAttr, is);
+    }
   }
 
   return element;
@@ -780,29 +814,71 @@ Element* Document::createElementNS(const AtomicString& namespaceURI,
   return createElement(qName, CreatedByCreateElement);
 }
 
+// https://dom.spec.whatwg.org/#internal-createelementns-steps
 Element* Document::createElementNS(const AtomicString& namespaceURI,
                                    const AtomicString& qualifiedName,
                                    const StringOrDictionary& stringOrOptions,
                                    ExceptionState& exceptionState) {
+  bool isV1 = stringOrOptions.isDictionary() || !registrationContext();
+  // 1. Validate and extract

[dominicc (has gone to gerrit), 2016/11/15 07:39:54]

These comments seem incomplete?

+
+  // 2.
+  const AtomicString& is =
+      AtomicString(getTypeExtension(this, stringOrOptions, exceptionState));
+  const AtomicString& name = is.isNull() ? qualifiedName : is;
+
+  if (!isValidName(qualifiedName)) {
+    exceptionState.throwDOMException(
+        InvalidCharacterError,
+        "The tag name provided ('" + qualifiedName + "') is not a valid name.");
+    return nullptr;
+  }
+
   QualifiedName qName(
       createQualifiedName(namespaceURI, qualifiedName, exceptionState));
   if (qName == QualifiedName::null())
     return nullptr;
 
+  // 3. Let definition be result of lookup up custom element definition

[dominicc (has gone to gerrit), 2016/11/15 07:39:54]

Transcribe these carefully.

+  CustomElementDefinition* definition = nullptr;
+  if (isV1) {
+    const CustomElementDescriptor desc =
+        RuntimeEnabledFeatures::customElementsBuiltinEnabled()
+            ? CustomElementDescriptor(name, qualifiedName)
+            : CustomElementDescriptor(qualifiedName, qualifiedName);
+    definition = CustomElement::registry(*this)->definitionFor(desc);
+
+    // 4. If 'is' is non-null and definition is null, throw NotFoundError
+    if (!definition && !is.isNull()) {
+      exceptionState.throwDOMException(NotFoundError,
+                                       "Custom element definition not found.");
+      return nullptr;
+    }
+  }
+
+  // 5. Let element be the result of creating an element
   Element* element;
-  if (CustomElement::shouldCreateCustomElement(qName))
-    element = CustomElement::createCustomElementSync(*this, qName);
-  else if (V0CustomElement::isValidName(qName.localName()) &&
-           registrationContext())
+
+  bool shouldCreateBuiltin =
+      isV1 && RuntimeEnabledFeatures::customElementsBuiltinEnabled();
+
+  if (CustomElement::shouldCreateCustomElement(qName) || shouldCreateBuiltin) {
+    element = CustomElement::createCustomElementSync(*this, qName, is);
+  } else if (V0CustomElement::isValidName(qName.localName()) &&
+             registrationContext()) {
     element = registrationContext()->createCustomTagElement(*this, qName);
-  else
+  } else {
     element = createElement(qName, CreatedByCreateElement);
+  }
 
-  String typeExtention =
-      getTypeExtension(this, stringOrOptions, exceptionState);
-  if (!typeExtention.isEmpty()) {
-    V0CustomElementRegistrationContext::setIsAttributeAndTypeExtension(
-        element, AtomicString(typeExtention));
+  // 6. If 'is' is non-null, set 'is' attribute
+  if (!is.isEmpty()) {
+    if (element->getCustomElementState() != CustomElementState::Custom) {
+      V0CustomElementRegistrationContext::setIsAttributeAndTypeExtension(
+          element, is);
+    } else if (stringOrOptions.isDictionary()) {
+      element->setAttribute(HTMLNames::isAttr, is);
+    }
   }
 
   return element;