Add callbacks to ScriptCustomElementDefinition

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "bindings/core/v8/ScriptCustomElementDefinitionBuilder.h"
 
 #include "bindings/core/v8/DOMWrapperWorld.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptCustomElementDefinition.h"
 #include "bindings/core/v8/ScriptState.h"
@@ skipping 41 matching lines @@
         m_constructor)) {
 
         // Constructor is already registered.
         m_exceptionState.throwDOMException(
             NotSupportedError,
             "this constructor has already been used with this registry");
         return false;
     }
     return true;
 }
 

[haraken, 2016/06/06 01:39:15]

I'd change 'char*' in this file with 'String'.

[kojii, 2016/06/06 11:12:35]

Done.

+bool ScriptCustomElementDefinitionBuilder::valueForName(
+    const v8::Local<v8::Object>& object, const char* name,
+    v8::Local<v8::Value>& value) const
+{
+    v8::Isolate* isolate = m_scriptState->isolate();
+    v8::Local<v8::Context> context = m_scriptState->context();
+    v8::Local<v8::String> nameString = v8AtomicString(isolate, name);
+    return v8Call(object->Get(context, nameString), value);
+}
+
 bool ScriptCustomElementDefinitionBuilder::checkPrototype()
 {
-    v8::Isolate* isolate = m_scriptState->isolate();
-    v8::Local<v8::Context> context = m_scriptState->context();
-    v8::Local<v8::String> prototypeString =
-        v8AtomicString(isolate, "prototype");
     v8::Local<v8::Value> prototypeValue;
-    if (!v8Call(
-        m_constructor->Get(context, prototypeString), prototypeValue)) {
+    if (!valueForName(m_constructor, "prototype", prototypeValue))
         return false;
-    }
     if (!prototypeValue->IsObject()) {
         m_exceptionState.throwTypeError(
             "constructor prototype is not an object");
         return false;
     }
     m_prototype = prototypeValue.As<v8::Object>();
     // If retrieving the prototype destroyed the context, indicate that
     // defining the element should not proceed.
     return true;
 }
 
+bool ScriptCustomElementDefinitionBuilder::callableForName(const char* name,
+    v8::Local<v8::Object>& callback) const
+{
+    v8::Local<v8::Value> value;
+    if (!valueForName(m_prototype, name, value))
+        return false;
+    if (value->IsUndefined())
+        return true;

[haraken, 2016/06/06 01:39:15]

Just to confirm: You want to return true for undefined but return false for
null, right?

[kojii, 2016/06/06 11:12:34]

Yes:
Undefined or IsCallable -> true (success)
Otherwise -> false (throw error)

Added the comment to clarify that.

+    if (!value->IsObject()) {
+        m_exceptionState.throwTypeError(
+            String::format("\"%s\" is not an object", name));
+        return false;
+    }
+    callback = value.As<v8::Object>();
+    if (!callback->IsCallable()) {
+        m_exceptionState.throwTypeError(
+            String::format("\"%s\" is not callable", name));
+        return false;
+    }
+    return true;
+}
+
+bool ScriptCustomElementDefinitionBuilder::cacheObservedAttributes()
+{
+    const char* kObservedAttributes = "observedAttributes";
+    v8::Local<v8::Value> observedAttributesValue;
+    if (!valueForName(m_constructor, kObservedAttributes, observedAttributesValue))
+        return false;
+    if (observedAttributesValue->IsUndefined())
+        return true;
+    Vector<AtomicString> list = toImplArray<Vector<AtomicString>>(
+        observedAttributesValue, 0, m_scriptState->isolate(), m_exceptionState);
+    if (m_exceptionState.hadException())
+        return false;
+    if (list.isEmpty())
+        return true;
+    m_observedAttributes.reserveCapacityForSize(list.size());
+    for (const auto& attribute : list)
+        m_observedAttributes.add(attribute);
+    return true;
+}
+
+bool ScriptCustomElementDefinitionBuilder::cacheProperties()

[haraken, 2016/06/06 01:39:15]

I'm a bit confused with the name "cacheProperties". This method is just looking
up existing atributes on m_constructor and m_prototype. How is it "caching"
somethings?

[kojii, 2016/06/06 11:12:35]

This method retrieves values and keep it in member field, so that Blink uses the
value at this point even if JS changes them afterwards; i.e.
  class C { connectedCallback() {} }
  customElements.define('a-a', C);
  C.prototype.connectedCallback = function () { doSomethingDifferent(); }
The spec says we must call the function before "define()".

That said, maybe "retrieve" works better?

[haraken, 2016/06/06 11:45:52]

rememberOriginalProperties ?

Also add a comment about the behavior.

+{
+
+    const char* kConnectedCallback = "connectedCallback";
+    const char* kDisconnectedCallback = "disconnectedCallback";
+    const char* kAttributeChangedCallback = "attributeChangedCallback";
+    return cacheObservedAttributes()
+        && callableForName(kConnectedCallback, m_connectedCallback)
+        && callableForName(kDisconnectedCallback, m_disconnectedCallback)
+        && callableForName(kAttributeChangedCallback, m_attributeChangedCallback);
+}
+
 CustomElementDefinition* ScriptCustomElementDefinitionBuilder::build(
     const CustomElementDescriptor& descriptor)
 {
     return ScriptCustomElementDefinition::create(
         m_scriptState.get(),
         m_registry,
         descriptor,
         m_constructor,
-        m_prototype);
+        m_prototype,
+        m_connectedCallback,
+        m_disconnectedCallback,
+        m_attributeChangedCallback,
+        m_observedAttributes);
 }
 
 } // namespace blink