Fix "report the exception" in Custom Elements 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 8b82b293f3578feebf3cad0a0533e11e1daad497..c14499794faa47db75aea19579990a6ab47ede52 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
@@ -189,6 +189,15 @@ HTMLElement* ScriptCustomElementDefinition::createElementSync(
     return toHTMLElement(element);
 }
 
+static void dispatchErrorEvent(v8::Isolate* isolate,

[haraken, 2016/08/22 06:36:47]

origin => constructor ?

+    v8::Local<v8::Value> exception, v8::Local<v8::Object> origin)
+{
+    v8::TryCatch tryCatch(isolate);
+    tryCatch.SetVerbose(true);
+    V8ScriptRunner::throwException(isolate, exception,
+        origin.As<v8::Function>()->GetScriptOrigin());
+}
+
 HTMLElement* ScriptCustomElementDefinition::createElementSync(
     Document& document, const QualifiedName& tagName)
 {
@@ -202,16 +211,15 @@ HTMLElement* ScriptCustomElementDefinition::createElementSync(
         "CustomElement", constructor(), isolate);
     HTMLElement* element = createElementSync(document, tagName, exceptionState);
 
-    if (exceptionState.hadException() || !element) {
+    if (exceptionState.hadException()) {
+        DCHECK(!element);
         // 7. If this step throws an exception, then report the exception, ...
-        {
-            v8::TryCatch tryCatch(isolate);
-            tryCatch.SetVerbose(true);
-            exceptionState.throwIfNeeded();
-        }
-
+        dispatchErrorEvent(isolate, exceptionState.getException(), constructor());
+        exceptionState.clearException();

[haraken, 2016/08/22 06:36:47]

Do you need to clear the exception here?

[kojii, 2016/08/22 06:41:33]

Not today, but when we remove throwIfNeeded()[1], it will throw but will
probably discarded (?). Since we processed it, clearing makes sense to me, but
I'm ok to remove if this doesn't look good.

[1] https://codereview.chromium.org/2061113002/

[Yuki, 2016/08/22 06:42:08]

I think we'll need to clear the exception once haraken@'s CL to remove
throwIfNeeded() gets landed.

+        // and return HTMLUnknownElement.
         return CustomElement::createFailedElement(document, tagName);
     }
+    DCHECK(element);
     return element;
 }
 
@@ -238,12 +246,11 @@ bool ScriptCustomElementDefinition::runConstructor(Element* element)
     if (result != element) {
         const String& message = "custom element constructors must call super() first and must "
             "not return a different object";
-        std::unique_ptr<SourceLocation> location = SourceLocation::fromFunction(constructor().As<v8::Function>());
         v8::Local<v8::Value> exception =  V8ThrowException::createDOMException(
             m_scriptState->isolate(),
             InvalidStateError,
             message);
-        fireErrorEvent(m_scriptState.get(), message, exception, std::move(location));
+        dispatchErrorEvent(isolate, exception, constructor());
         return false;
     }
 
@@ -268,14 +275,6 @@ Element* ScriptCustomElementDefinition::runConstructor()
     return V8Element::toImplWithTypeCheck(isolate, result);
 }
 
-void ScriptCustomElementDefinition::fireErrorEvent(ScriptState* scriptState, const String& message, v8::Local<v8::Value> exception, std::unique_ptr<SourceLocation> location)
-{
-    ErrorEvent* event = ErrorEvent::create(message, std::move(location), &scriptState->world());
-    V8ErrorHandler::storeExceptionOnErrorEventWrapper(scriptState, event, exception, scriptState->context()->Global());
-    ExecutionContext* executionContext = scriptState->getExecutionContext();
-    executionContext->dispatchErrorEvent(event, NotSharableCrossOrigin);
-}
-
 v8::Local<v8::Object> ScriptCustomElementDefinition::constructor() const
 {
     DCHECK(!m_constructor.isEmpty());