Add callbacks to ScriptCustomElementDefinition

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

Index: third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h
index b6d98ab1cee403410831c3e29a54a7eb8af56ff2..a89ede5eb0d45cb43426b45a928f2aae76675a17 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h
@@ -34,15 +34,26 @@ public:
     bool checkConstructorIntrinsics() override;
     bool checkConstructorNotRegistered() override;
     bool checkPrototype() override;
+    bool cacheProperties() override;
     CustomElementDefinition* build(const CustomElementDescriptor&) override;
 
 private:
+    friend class ScriptCustomElementDefinition;

[dominicc (has gone to gerrit), 2016/06/02 00:31:27]

I don't really like the ScriptCustomElementDefinition being a friend of the
builder. It puts the registry within reach of the definition, when really the
registry is the master object and should act on the definition.

If you're worried about the long, repetitive repetitive parameter list to
create, I suggest you encapsulate the builder's state in a parameter object and
pass that object to the ScriptCustomElementDefinition. A future unit test that
wants to create a ScriptCustomElementDefinition can create one without having to
go through a builder.

[yosin_UTC9, 2016/06/02 01:33:38]

When const-ish isn't important in |ScriptCusomElementDefintion|, I suggest to
make |SCEDBuilder| as friend of |SCED|. In this case, ctor of SCED is private
default ctor. Then |SCEDBuilder| set member variables of SCED directly.

Here is a simple example of my editor's ComputedStyle and
ComputedStyle::Builder.

[kojii, 2016/06/02 03:18:08]

Reverted.

+
     RefPtr<ScriptState> m_scriptState;
     Member<CustomElementsRegistry> m_registry;
     v8::Local<v8::Value> m_constructorValue;
     v8::Local<v8::Object> m_constructor;
     v8::Local<v8::Object> m_prototype;
+    v8::Local<v8::Object> m_connectedCallback;
+    v8::Local<v8::Object> m_disconnectedCallback;
+    v8::Local<v8::Object> m_attributeChangedCallback;
+    Vector<AtomicString> m_observedAttributes;

[dominicc (has gone to gerrit), 2016/06/02 00:31:27]

Any reason not to just make this a HashSet and move or copy the HashSet to the
definition?

[kojii, 2016/06/02 03:18:08]

Done. Not really, had a micro-optimization in mind originally but not worth to
worry.

     ExceptionState& m_exceptionState;
+
+    bool getValue(const v8::Local<v8::Object>&, const char*,

[yosin_UTC9, 2016/06/02 01:33:38]

Term "get" is so usual, we should use another name, e.g. |valueFor()|,
|valueFor()|, ...

+        v8::Local<v8::Value>&) const;
+    bool getCallable(const char*, v8::Local<v8::Object>&) const;

[yosin_UTC9, 2016/06/02 01:33:38]

In Blink, getter doesn't have "get", but in other projects, "get" is used for
getter.
Term "get" is so usual, we should use another name, e.g. |calalbleFor()|

[kojii, 2016/06/02 03:18:08]

Done, thought "get" is chromium style but I don't have opinions here.

 };
 
 } // namespace blink