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

third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp

Index: third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
index de725ff62ad418e8ad81b48123a14d602e1e0ba3..fdb53c0f5112a385d8077453834f804e591e5ee0 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
@@ -13,6 +13,8 @@
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "bindings/core/v8/V8ThrowException.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/html/HTMLElement.h"
+#include "core/html/HTMLUnknownElement.h"
 #include "v8.h"
 #include "wtf/Allocator.h"
 
@@ -152,12 +154,96 @@ ScriptCustomElementDefinition::ScriptCustomElementDefinition(
 {
 }
 
+static String errorForConstructorResult(Element* element,
+    Document& document, const QualifiedName& tagName)
+{
+    // https://dom.spec.whatwg.org/#concept-create-element
+    // 6.1.4. If result's attribute list is not empty, then throw a NotSupportedError.
+    if (element->hasAttributes())
+        return "The result must not have attributes";
+    // 6.1.5. If result has children, then throw a NotSupportedError.
+    if (element->hasChildren())
+        return "The result must not have children";
+    // 6.1.6. If result's parent is not null, then throw a NotSupportedError.
+    if (element->parentNode())
+        return "The result must not have a parent";
+    // 6.1.7. If result's node document is not document, then throw a NotSupportedError.
+    if (&element->document() != &document)
+        return "The result must be in the same document";
+    // 6.1.8. If result's namespace is not the HTML namespace, then throw a NotSupportedError.
+    if (element->namespaceURI() != HTMLNames::xhtmlNamespaceURI)
+        return "The result must have HTML namespace";
+    // 6.1.9. If result's local name is not equal to localName, then throw a NotSupportedError.
+    if (element->localName() != tagName.localName())
+        return "The result must have the same localName";
+    return String();
+}
+
+HTMLElement* ScriptCustomElementDefinition::createCustomElement(
+    Document& document, const QualifiedName& tagName)
+{
+    // Create an element
+    // https://dom.spec.whatwg.org/#concept-create-element
+    // 6. If definition is non-null
+    // 6.1. If the synchronous custom elements flag is set:
+    // 6.1.2. Set result to Construct(C). Rethrow any exceptions.
+    Element* element;
+    {
+        v8::TryCatch tryCatch(m_scriptState->isolate());
+        element = runConstructor();
+        if (tryCatch.HasCaught()) {
+            tryCatch.ReThrow();
+            return nullptr;
+        }
+    }
+
+    // 6.1.3. If result does not implement the HTMLElement interface, throw a TypeError.

[dominicc (has gone to gerrit), 2016/06/09 06:39:22]

This might be a helpful reference:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

[kojii, 2016/06/09 16:54:21]

Ah, thanks for the pointer, this looks good.

It looks like Domenic is fine with checking isHTMLElement() for now:
https://github.com/whatwg/html/issues/1402
Hope this is good with you too?

+    // TODO(kojii): how to check interface? Is this correct way to do this? This
+    // can check HTMLXxxElement classes, but not duck-typing.
+    // https://github.com/whatwg/html/issues/1402
+    if (!element || !element->isHTMLElement()) {
+        throwTypeError("The result must implement HTMLElement interface");

[dominicc (has gone to gerrit), 2016/06/09 06:39:22]

Might not be worth adding a helper for one line? BTW, if we switch to
ExceptionState it has es.throwTypeError.

[kojii, 2016/06/09 16:54:21]

Removed helpers as we switched to ExceptionState.

+        return nullptr;
+    }
+
+    // 6.1.4. through 6.1.9.

[dominicc (has gone to gerrit), 2016/06/09 06:39:23]

Could we make this a "template method pattern" (not a *C++* template) where we
keep the checks on attributes, etc. in CustomElementDefinition and just put the
script-specific things in ScriptCustomElementDefinition?

It looks like everything to the end of this method is just dealing with DOM
things.

[kojii, 2016/06/09 16:54:21]

Done.

+    const String message = errorForConstructorResult(element, document, tagName);
+    if (!message.isEmpty()) {
+        throwDOMException(NotSupportedError, message);
+        return nullptr;
+    }
+
+    DCHECK_EQ(element->getCustomElementState(), CustomElementState::Custom);
+    return toHTMLElement(element);
+}
+
+HTMLElement* ScriptCustomElementDefinition::createCustomElementIgnoringErrors(
+    Document& document, const QualifiedName& 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,
+    // with no attributes, namespace set to given namespace, namespace prefix
+    // set to null, custom element state "undefined", and node document set to
+    // document.
+    v8::TryCatch tryCatch(m_scriptState->isolate());
+    tryCatch.SetVerbose(true);
+
+    HTMLElement* element = createCustomElement(document, tagName);
+    if (element && !tryCatch.HasCaught())
+        return element;
+
+    element = HTMLUnknownElement::create(tagName, document);
+    element->setCustomElementState(CustomElementState::Undefined);
+    return element;
+}
+
 // https://html.spec.whatwg.org/multipage/scripting.html#upgrades
 bool ScriptCustomElementDefinition::runConstructor(Element* element)
 {
     if (!m_scriptState->contextIsValid())
         return false;
-    ScriptState::Scope scope(m_scriptState.get());
     v8::Isolate* isolate = m_scriptState->isolate();
 
     // Step 5 says to rethrow the exception; but there is no one to
@@ -165,6 +251,27 @@ bool ScriptCustomElementDefinition::runConstructor(Element* element)
     v8::TryCatch tryCatch(isolate);
     tryCatch.SetVerbose(true);
 
+    Element* result = runConstructor();
+    if (!result)
+        return false;
+
+    if (result != element) {
+        throwDOMException(
+            InvalidStateError,
+            "custom element constructors must call super() first and must "
+            "not return a different object");
+        return false;
+    }
+
+    return true;
+}
+
+Element* ScriptCustomElementDefinition::runConstructor()
+{
+    if (!m_scriptState->contextIsValid())
+        return nullptr;
+    ScriptState::Scope scope(m_scriptState.get());
+    v8::Isolate* isolate = m_scriptState->isolate();
     ExecutionContext* executionContext = m_scriptState->getExecutionContext();
     v8::Local<v8::Value> result;
     if (!v8Call(V8ScriptRunner::callAsConstructor(
@@ -173,22 +280,10 @@ bool ScriptCustomElementDefinition::runConstructor(Element* element)
         executionContext,
         0,
         nullptr),
-        result))
-        return false;
-
-    if (V8Element::toImplWithTypeCheck(isolate, result) != element) {
-        V8ThrowException::throwException(
-            V8ThrowException::createDOMException(
-                m_scriptState->isolate(),
-                InvalidStateError,
-                "custom element constructors must call super() first and must "
-                "not return a different object",
-                constructor()),
-            m_scriptState->isolate());
-        return false;
+        result)) {
+        return nullptr;
     }
-
-    return true;
+    return V8Element::toImplWithTypeCheck(isolate, result);
 }
 
 v8::Local<v8::Object> ScriptCustomElementDefinition::constructor() const
@@ -209,4 +304,20 @@ ScriptValue ScriptCustomElementDefinition::getConstructorForScript()
     return ScriptValue(m_scriptState.get(), constructor());
 }
 
+void ScriptCustomElementDefinition::throwDOMException(int exceptionCode, const String& message)
+{
+    V8ThrowException::throwException(
+        V8ThrowException::createDOMException(
+            m_scriptState->isolate(),
+            exceptionCode,
+            message,
+            constructor()),
+        m_scriptState->isolate());
+}
+
+void ScriptCustomElementDefinition::throwTypeError(const String& message)
+{
+    V8ThrowException::throwTypeError(m_scriptState->isolate(), message);
+}
+
 } // namespace blink