[webkit-reviews] review denied: [Bug 112538] Custom Elements: "created" lifecycle callback should be called. : [Attachment 194214] Patch

[bugzilla-daemon at webkit.org]

Elliott Sprehn <esprehn at chromium.org> has denied Hajime Morrita
<morrita at google.com>'s request for review:
Bug 112538: Custom Elements: "created" lifecycle callback should be called.
https://bugs.webkit.org/show_bug.cgi?id=112538

Attachment 194214: Patch
https://bugs.webkit.org/attachment.cgi?id=194214&action=review

------- Additional Comments from Elliott Sprehn <esprehn at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=194214&action=review


Pretty sure it's called readyCallback now. Also we need a generic queue that
tracks invocations of all the lifecycle methods in order. We can't do all ready
callbacks and then all inserted callbacks, they need to happen in sequence. I
think you want CustomElementInvocation to a type() field, and then loop over
and execute the type of callback for each one.

> Source/WebCore/ChangeLog:39
> +	   instnace are marked. See
CustomElementRegistry::CallbackDeliveryScope

instances

> Source/WebCore/bindings/v8/CustomElementHelpers.cpp:153
> +    v8::Handle<v8::Value> functionValue =
prototype->Get(v8::String::NewSymbol("created"));

Didn't we rename this readyCallback ?

> Source/WebCore/dom/CustomElementRegistry.cpp:236
> +	   s_needsDeliverLifecycleCallbacks = true;

Can we just check isEmpty on the registries and remove this static state bool?

> Source/WebCore/dom/CustomElementRegistry.cpp:277
> +    s_needsDeliverLifecycleCallbacks = false;

Lets eliminate this bool.

> Source/WebCore/dom/CustomElementRegistry.h:65
> +    RefPtr<Element> m_element;

this needs an m_type which is one of Ready, Inserted, Removed or
AttributeChanged I think.

> Source/WebCore/dom/CustomElementRegistry.h:107
> +    static bool s_needsDeliverLifecycleCallbacks;

Make activeCustomElementRegistries() inline and put it here and you don't need
this anymore.

> Source/WebCore/dom/Document.cpp:900
> +	   m_registry->didCreateElement(element);

How is it possible to not have a registry and still get here?