Implement "create an element" when sync for Custom Element V1

third_party/WebKit/Source/core/dom/custom/CustomElement.cpp

Index: third_party/WebKit/Source/core/dom/custom/CustomElement.cpp
diff --git a/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp b/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp
index f1e231cb1aa15fc374b15bd984f335d3c4c34276..ca01272dadd14e5bbf031523b00f473e5f92a82a 100644
--- a/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp
+++ b/third_party/WebKit/Source/core/dom/custom/CustomElement.cpp
@@ -31,6 +31,13 @@ CustomElementsRegistry* CustomElement::registry(const Document& document)
     return nullptr;
 }
 
+CustomElementDefinition* CustomElement::definitionForName(const Document& document, const AtomicString& name)

[dominicc (has gone to gerrit), 2016/06/13 07:37:24]

This is not the right signature for when we implement customized built-in
elements. Use a const CustomElementDescriptor&.

[kojii, 2016/06/13 13:44:31]

Made this static to this file for now to avoid expanding its use, and see below
for more comments.

+{
+    if (CustomElementsRegistry* registry = CustomElement::registry(document))
+        return registry->definitionForName(name);
+    return nullptr;
+}
+
 bool CustomElement::isValidName(const AtomicString& name)
 {
     if (!name.length() || name[0] < 'a' || name[0] > 'z')
@@ -79,31 +86,61 @@ bool CustomElement::shouldCreateCustomElement(Document& document, const Qualifie
         && tagName.namespaceURI() == HTMLNames::xhtmlNamespaceURI;
 }
 
-HTMLElement* CustomElement::createCustomElement(Document& document, const AtomicString& localName, CreateElementFlags flags)
+HTMLElement* CustomElement::createCustomElementSync(Document& document, const AtomicString& localName, ExceptionState& exceptionState)
 {
-    return createCustomElement(document,
+    return createCustomElementSync(document,
         QualifiedName(nullAtom, localName, HTMLNames::xhtmlNamespaceURI),
-        flags);
+        exceptionState);
 }
 
-HTMLElement* CustomElement::createCustomElement(Document& document, const QualifiedName& tagName, CreateElementFlags flags)
+HTMLElement* CustomElement::createCustomElementSync(Document& document, const QualifiedName& tagName, ExceptionState& exceptionState)
 {
     DCHECK(shouldCreateCustomElement(document, tagName));
 
     // To create an element:
     // https://dom.spec.whatwg.org/#concept-create-element
     // 6. If definition is non-null, then:
-    HTMLElement* element;
-    if (CustomElementsRegistry* registry = CustomElement::registry(document)) {
-        if (CustomElementDefinition* definition = registry->definitionForName(tagName.localName())) {
-            // 6.2. If the synchronous custom elements flag is not set:
-            if (flags & AsynchronousCustomElements)
-                return createCustomElementAsync(document, *definition, tagName);
-
-            // TODO(kojii): Synchronous mode implementation coming after async.
-        }
-    }
+    // 6.1. If the synchronous custom elements flag is set:
+    if (CustomElementDefinition* definition = definitionForName(document, tagName.localName()))

[dominicc (has gone to gerrit), 2016/06/13 07:37:24]

This is error-prone because it throws away the namespace; maybe we should
upgrade the DCHECK to CHECK.

In general, it might be better to have people construct
CustomElementDescriptors, and use those consistently to find definitions. The
only exceptions are:

- The get API.
- Testing if a *name* is defined.
- As an optimization (for example, taking upgrade candidates for one descriptor
implies any other upgrade candidates with that *name* will never be processed
and can be discarded.)

Then we have assertions about what's a valid descriptor in one place. You can't
process an svg:my-button as a custom element because you can't construct that
descriptor.

If we don't use the descriptor now, we will end up changing all of these method
signatures later to take an "is" attribute. It will be simpler if we use the
descriptor, now, instead.

[kojii, 2016/06/13 13:44:31]

I agree with you in general direction we should move forward to, but I'd prefer
to take a refactoring step on this point. Unable to create synchronously limits
us creating test cases for the parallel work we're doing.

To use CustomElementDescriptors properly and to prepare for type extension, I
need to refactor HTMLElementFactory first. That can be done in parallel with
other work while I'm waiting for reviews on them.

DCHECKs changed to CHECKs.

+        return definition->createElementSync(document, tagName, exceptionState);
+
+    return createUndefinedElement(document, tagName);
+}
+
+HTMLElement* CustomElement::createCustomElementSync(Document& document, const QualifiedName& tagName)
+{
+    DCHECK(shouldCreateCustomElement(document, tagName));
+
+    // When invoked from "create an element for a token":
+    // https://html.spec.whatwg.org/multipage/syntax.html#create-an-element-for-the-token
+    // 7. If this step throws an exception, then report the exception,
+    // and let element be instead a new element that implements
+    // HTMLUnknownElement.
+    if (CustomElementDefinition* definition = definitionForName(document, tagName.localName()))
+        return definition->createElementSync(document, tagName);
+
+    return createUndefinedElement(document, tagName);
+}
+
+HTMLElement* CustomElement::createCustomElementAsync(Document& document, const QualifiedName& tagName)
+{
+    DCHECK(shouldCreateCustomElement(document, tagName));
+
+    // To create an element:
+    // https://dom.spec.whatwg.org/#concept-create-element
+    // 6. If definition is non-null, then:
+    // 6.2. If the synchronous custom elements flag is not set:
+    if (CustomElementDefinition* definition = definitionForName(document, tagName.localName()))
+        return definition->createElementAsync(document, tagName);
+
+    return createUndefinedElement(document, tagName);
+}
+
+HTMLElement* CustomElement::createUndefinedElement(Document& document, const QualifiedName& tagName)
+{
+    DCHECK(shouldCreateCustomElement(document, tagName));
 
+    HTMLElement* element;
     if (V0CustomElement::isValidName(tagName.localName()) && document.registrationContext()) {
         Element* v0element = document.registrationContext()->createCustomTagElement(document, tagName);
         SECURITY_DCHECK(v0element->isHTMLElement());
@@ -117,24 +154,6 @@ HTMLElement* CustomElement::createCustomElement(Document& document, const Qualif
     return element;
 }
 
-HTMLElement* CustomElement::createCustomElementAsync(Document& document,
-    CustomElementDefinition& definition, const QualifiedName& tagName)
-{
-    // https://dom.spec.whatwg.org/#concept-create-element
-    // 6. If definition is non-null, then:
-    // 6.2. If the synchronous custom elements flag is not set:
-    // 6.2.1. Set result to a new element that implements the HTMLElement
-    // interface, with no attributes, namespace set to the HTML namespace,
-    // namespace prefix set to prefix, local name set to localName, custom
-    // element state set to "undefined", and node document set to document.
-    HTMLElement* element = HTMLElement::create(tagName, document);
-    element->setCustomElementState(CustomElementState::Undefined);
-    // 6.2.2. Enqueue a custom element upgrade reaction given result and
-    // definition.
-    enqueueUpgradeReaction(element, &definition);
-    return element;
-}
-
 void CustomElement::enqueueUpgradeReaction(Element* element, CustomElementDefinition* definition)
 {
     // CEReactionsScope must be created by [CEReactions] in IDL,