[webkit-reviews] review granted: [Bug 189144] slotchange event doesn't get fired when inserting, removing, or renaming slot elements : [Attachment 348839] Fixes the bug

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 189144: slotchange event doesn't get fired when inserting, removing, or
renaming slot elements
https://bugs.webkit.org/show_bug.cgi?id=189144

Attachment 348839: Fixes the bug

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




--- Comment #19 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 348839
  --> https://bugs.webkit.org/attachment.cgi?id=348839
Fixes the bug

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

r=me

> Source/WebCore/ChangeLog:10
> +	   Let us consider each scenairo separately.

'scenario'

> Source/WebCore/ChangeLog:51
> +	   node insertion, removal, or rename but no longer has after the
mutation. This patch also adds a slot mutation
> +	   version number, m_slotMutationVersion, which is incremented whenever
a node is about to be inserted or removed,
> +	   and slot resolution version, m_slotResolutionVersion, which is set
to the current slot mutation version number
> +	   when the full slot resolution is triggered during slot mutations.
They are used to avoid redundant tree traversals

I like the versioning approach.

> Source/WebCore/ChangeLog:74
> +	   While the DOM specification currently specifies the former beavhior,
this patch implements the latter behavior

'behavior'

> Source/WebCore/dom/SlotAssignment.cpp:181
> +    if (!slot->element) // A previous invocation to
resolveSlotsAfterSlotMutation during this removal has updated this slot.
> +	   ASSERT(m_slotResolutionVersion == m_slotMutationVersion &&
!findSlotElement(shadowRoot, name));
> +    else {

Maybe just 

ASSERT(slot->element || (m_slotResolutionVersion == m_slotMutationVersion &&
!findSlotElement(shadowRoot, name)));

A branch with nothing but assert doesn't look nice.