Add callbacks to ScriptCustomElementDefinition

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

 // 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.
 
 #ifndef ScriptCustomElementDefinitionBuilder_h
 #define ScriptCustomElementDefinitionBuilder_h
 
 #include "core/CoreExport.h"
 #include "core/dom/custom/CustomElementDefinitionBuilder.h"
 #include "platform/heap/Handle.h"
@@ skipping 16 matching lines @@
 public:
     ScriptCustomElementDefinitionBuilder(
         ScriptState*,
         CustomElementsRegistry*,
         const ScriptValue& constructorScriptValue,
         ExceptionState&);
 
     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
 
 #endif // ScriptCustomElementDefinitionBuilder_h