Stop re-signaling a slotchange event

Stop re-signaling a slotchange event

The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status [2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
Committed: https://crrev.com/d7af35614d3a6f1d8f0d6f4e37ea921b89446002
Cr-Commit-Position: refs/heads/master@{#428718}

[hayato, 2016-10-14 08:08:55]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 08:09:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 08:11:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 08:11:46]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hayato, 2016-10-14 08:36:07]

New slotchange behavior

[hayato, 2016-10-14 08:36:08]

The CQ bit was checked by hayato@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 08:36:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 09:53:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 09:53:20]

Dry run: This issue passed the CQ dry run.

[hayato, 2016-10-31 06:19:49]

Description was changed from

==========
New slotchange behavior

BUG=
==========

to

==========
The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).


Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

[hayato, 2016-10-31 06:20:08]

Description was changed from

==========
The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).


Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

to

==========
The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

[hayato, 2016-10-31 06:20:42]

hayato@chromium.org changed reviewers:
+ kochi@chromium.org, tkent@chromium.org

[hayato, 2016-10-31 06:20:42]

PTAL

[kochi, 2016-10-31 08:39:14]

LGTM

You should include the subject as the first line in
the description, as the CQ commits message with the
description as commit message, IIUC.

Do you want to mention this change makes the behavior
compatible with Safari/WebKit in the description?

https://codereview.chromium.org/2420043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2420043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:2377: if (root && root->isV1() &&
!parentSlot.hasAssignedNodesSlow())
BTW (this is not for this review) this |root| is guaranteed
non-null (L2376), so the first check is useless.

https://codereview.chromium.org/2420043002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.h (right):

https://codereview.chromium.org/2420043002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.h:118: };
nit: clang-format seems to prefer fitting in one line,
enum class SlotChangeType { Initial, Chained };
but if presubmit check didn't complain this, I think this is okay.
(also for the trailing comma for |Chained|).

[hayato, 2016-10-31 10:19:52]

Description was changed from

==========
The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

to

==========
Stop re-signaling a slotchange event

The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

[hayato, 2016-10-31 10:23:45]

> You should include the subject as the first line in
> the description, as the CQ commits message with the
> description as commit message, IIUC.

Done.

> 
> Do you want to mention this change makes the behavior
> compatible with Safari/WebKit in the description?

Safari's implementation is incomplete. They are aware of it, and we agreed that
it is okay to update the behavior.


>
https://codereview.chromium.org/2420043002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/Node.h:118: };
> nit: clang-format seems to prefer fitting in one line,
> enum class SlotChangeType { Initial, Chained };
> but if presubmit check didn't complain this, I think this is okay.
> (also for the trailing comma for |Chained|).

presubmit did not complain this. "git cl format" did nothing for this part, in
my environment.

[hayato, 2016-10-31 10:23:57]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2016-10-31 10:24:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 11:37:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-31 11:37:39]

Failed to apply patch for third_party/WebKit/Source/core/html/HTMLSlotElement.h:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/core/html/HTMLSlotElement.h:88
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/core/html/HTMLSlotElement.h' with
conflicts.
  U third_party/WebKit/Source/core/html/HTMLSlotElement.h

Patch:       third_party/WebKit/Source/core/html/HTMLSlotElement.h
Index: third_party/WebKit/Source/core/html/HTMLSlotElement.h
diff --git a/third_party/WebKit/Source/core/html/HTMLSlotElement.h
b/third_party/WebKit/Source/core/html/HTMLSlotElement.h
index
7b6e44a21bd7e6845d14ddc00d44a68e31c36efa..41cf631b1fdd2b9e6e5efc42c04a2e915f17d8fc
100644
--- a/third_party/WebKit/Source/core/html/HTMLSlotElement.h
+++ b/third_party/WebKit/Source/core/html/HTMLSlotElement.h
@@ -88,13 +88,12 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement
{
   bool hasAssignedNodesSlow() const;
   bool findHostChildWithSameSlotName() const;
 
-  void enqueueSlotChangeEvent();
-
   void clearDistribution();
   void saveAndClearDistribution();
 
   bool supportsDistribution() const { return isInShadowTree(); }
   void updateDistributedNodesManually();
+  void didSlotChange(SlotChangeType);
 
   static AtomicString normalizeSlotName(const AtomicString&);
 
@@ -107,6 +106,7 @@ class CORE_EXPORT HTMLSlotElement final : public HTMLElement
{
   void removedFrom(ContainerNode*) final;
   void willRecalcStyle(StyleRecalcChange) final;
 
+  void enqueueSlotChangeEvent();
   void dispatchSlotChangeEvent();
 
   HeapVector<Member<Node>> m_assignedNodes;

[hayato, 2016-10-31 12:09:40]

rebased

[hayato, 2016-10-31 12:09:42]

The CQ bit was checked by hayato@chromium.org

[hayato, 2016-10-31 12:09:43]

The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org
Link to the patchset: https://codereview.chromium.org/2420043002/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2016-10-31 12:09:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hayato, 2016-10-31 12:10:11]

The CQ bit was unchecked by hayato@chromium.org

[hayato, 2016-10-31 12:12:03]

rebased

[hayato, 2016-10-31 12:12:05]

The CQ bit was checked by hayato@chromium.org

[hayato, 2016-10-31 12:12:06]

The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org
Link to the patchset: https://codereview.chromium.org/2420043002/#ps60001
(title: "rebased")

[commit-bot: I haz the power, 2016-10-31 12:12:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 15:39:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-31 15:44:51]

Description was changed from

==========
Stop re-signaling a slotchange event

The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777
==========

to

==========
Stop re-signaling a slotchange event

The spec change is: https://github.com/whatwg/dom/pull/353
Spec issue is: https://github.com/w3c/webcomponents/issues/571

Blink no longer re-fires a slotchange event at a slot's assignedSlot (or parent
slot).

Given that the usage of the current slotchange is low enough [1],
it would be okay to change the behavior, just having the entry on chrome status
[2].

- [1] https://www.chromestatus.com/metrics/feature/timeline/popularity/1468
- [2] https://www.chromestatus.com/feature/5666302446272512

BUG=660777

Committed: https://crrev.com/d7af35614d3a6f1d8f0d6f4e37ea921b89446002
Cr-Commit-Position: refs/heads/master@{#428718}
==========

[commit-bot: I haz the power, 2016-10-31 15:44:52]

Patchset 4 (id:??) landed as
https://crrev.com/d7af35614d3a6f1d8f0d6f4e37ea921b89446002
Cr-Commit-Position: refs/heads/master@{#428718}