[webkit-reviews] review granted: [Bug 183397] Implement customElements.upgrade() : [Attachment 346361] Implements the feature

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has granted	review:
Bug 183397: Implement customElements.upgrade()
https://bugs.webkit.org/show_bug.cgi?id=183397

Attachment 346361: Implements the feature

https://bugs.webkit.org/attachment.cgi?id=346361&action=review




--- Comment #5 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 346361
  --> https://bugs.webkit.org/attachment.cgi?id=346361
Implements the feature

View in context: https://bugs.webkit.org/attachment.cgi?id=346361&action=review

>> Source/WebCore/dom/CustomElementReactionQueue.cpp:-125
>> -	ASSERT(element.isConnected());
> 
> We need to remove this since upgrade can be called on a disconnected tree,
and that's sort of the whole point of this API.

Right. It seems Element::insertedIntoAncestor is the only place where
enqueueElementUpgradeIfDefined was called.

> Source/WebCore/dom/CustomElementRegistry.cpp:117
> +static void upgradeElementsInShadowIncludingdescendants(ContainerNode& root)

nit: 'D' of descendants should be uppercase.

> Source/WebCore/dom/CustomElementRegistry.h:44
> +class DeferredPromise;

The DeferredPromise include / forward declaration change seems unrelated to
this patch, right? Maybe mention in the ChangeLog whether it is just a fix
passing by.