Check slot's distribution state more strictly

third_party/WebKit/Source/core/html/HTMLSlotElement.cpp

 /*
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 27 matching lines @@
 #include "core/dom/shadow/InsertionPoint.h"
 #include "core/events/Event.h"
 #include "core/html/AssignedNodesOptions.h"
 
 namespace blink {
 
 using namespace HTMLNames;
 
 inline HTMLSlotElement::HTMLSlotElement(Document& document)
     : HTMLElement(slotTag, document)
-    , m_distributionState(DistributionUnchanged)
+    , m_distributionState(DistributionDone)
 {
     setHasCustomStyleCallbacks();
 }
 
 DEFINE_NODE_FACTORY(HTMLSlotElement);
 
 const WillBeHeapVector<RefPtrWillBeMember<Node>> HTMLSlotElement::getAssignedNodesForBinding(const AssignedNodesOptions& options)
 {
     updateDistribution();
     if (options.hasFlatten() && options.flatten())
         return getDistributedNodes();
     return m_assignedNodes;
 }
 
 const WillBeHeapVector<RefPtrWillBeMember<Node>>& HTMLSlotElement::getDistributedNodes()
 {
     ASSERT(!needsDistributionRecalc());
     if (isInShadowTree())
         return m_distributedNodes;
 
     // A slot is unlikely to be used outside of a shadow tree.
     // We do not need to optimize this case in most cases.
     // TODO(hayato): If this path causes a performance issue, we should move
     // ShadowRootRaraDate::m_descendantSlots into TreeScopreRareData-ish and
     // update the distribution code so it considers a document tree too.
-    clearDistribution();
+    willUpdateDistribution();
     for (Node& child : NodeTraversal::childrenOf(*this)) {
         if (!child.isSlotAssignable())
             continue;
         if (isHTMLSlotElement(child))
             m_distributedNodes.appendVector(toHTMLSlotElement(child).getDistributedNodes());
         else
             m_distributedNodes.append(&child);
     }
+    didUpdateDistribution();
     return m_distributedNodes;
 }
 
 void HTMLSlotElement::appendAssignedNode(Node& node)
 {
+    ASSERT(m_distributionState == DistributionOnGoing);
     m_assignedNodes.append(&node);
 }
 
 void HTMLSlotElement::appendDistributedNode(Node& node)
 {
+    ASSERT(m_distributionState == DistributionOnGoing);
     size_t size = m_distributedNodes.size();
     m_distributedNodes.append(&node);
     m_distributedIndices.set(&node, size);
 }
 
 void HTMLSlotElement::appendDistributedNodesFrom(const HTMLSlotElement& other)
 {
+    ASSERT(m_distributionState == DistributionOnGoing);
     size_t index = m_distributedNodes.size();
     m_distributedNodes.appendVector(other.m_distributedNodes);
     for (const auto& node : other.m_distributedNodes)
         m_distributedIndices.set(node.get(), index++);
 }
 
-void HTMLSlotElement::clearDistribution()
+void HTMLSlotElement::willUpdateDistribution()
 {
+    ASSERT(m_distributionState != DistributionOnGoing);
+    m_distributionState = DistributionOnGoing;
     m_assignedNodes.clear();
     m_oldDistributedNodes.swap(m_distributedNodes);
     m_distributedNodes.clear();
     m_distributedIndices.clear();
-    m_distributionState = DistributionReset;
 }
 
 bool HTMLSlotElement::hasSlotChangeEventListener()
 {
     return eventTargetData() && eventTargetData()->eventListenerMap.find(EventTypeNames::slotchange);
 }
 
 void HTMLSlotElement::dispatchSlotChangeEvent()
 {
     RefPtrWillBeRawPtr<Event> event = Event::create(EventTypeNames::slotchange);
@@ skipping 116 matching lines @@
             continue;
         if (isHTMLSlotElement(child))
             appendDistributedNodesFrom(toHTMLSlotElement(child));
         else
             appendDistributedNode(child);
     }
 }
 
 bool HTMLSlotElement::distributionChanged()
 {
-    if (m_distributionState == DistributionReset)
+    ASSERT(m_distributionState != DistributionOnGoing);
+    if (m_distributionState == DistributionDone)
         m_distributionState = m_oldDistributedNodes == m_distributedNodes ? DistributionUnchanged : DistributionChanged;
     return m_distributionState == DistributionChanged;
 }
 
 void HTMLSlotElement::didUpdateDistribution()
 {
+    ASSERT(m_distributionState == DistributionOnGoing);
+    m_distributionState = DistributionDone;
     if (isChildOfV1ShadowHost()) {
         ElementShadow* shadow = parentElementShadow();
         ASSERT(shadow);
         if (!shadow->needsDistributionRecalc() && distributionChanged())
             shadow->setNeedsDistributionRecalc();
     }
     if (hasSlotChangeEventListener() && distributionChanged()) {
         // TODO(hayato): Do not enqueue a slotchange event for the same slot twice in the microtask queue
         Microtask::enqueueMicrotask(WTF::bind(&HTMLSlotElement::dispatchSlotChangeEvent, PassRefPtrWillBeRawPtr<HTMLSlotElement>(this)));
     }
 }
 
+void HTMLSlotElement::clearDistribution()

[kochi, 2016/03/04 03:49:07]

Why this is not updateDistribution()?

insertedInto()/removedFrom() will always update the distribution,
as opposed to just clear distribution and calculate lazily later?

[hayato, 2016/03/04 05:15:14]

Could you elaborate?
Yes.

[kochi, 2016/03/04 08:17:15]

The previous one was obvious that it "clears" m_distributedNodes etc.
But now they are cleared, then completes update.

So to me, "clear"ing "distribution" sounds strange.
I thought this function does the full update process of distribution.
 

+{
+    willUpdateDistribution();
+    didUpdateDistribution();
+}
+
 DEFINE_TRACE(HTMLSlotElement)
 {
 #if ENABLE(OILPAN)
     visitor->trace(m_assignedNodes);
     visitor->trace(m_distributedNodes);
     visitor->trace(m_distributedIndices);
     visitor->trace(m_oldDistributedNodes);
 #endif
     HTMLElement::trace(visitor);
 }
 
 } // namespace blink