Element::recalcStyle() overly reattach()-es InsertionPoints.

Source/core/dom/shadow/ContentDistributor.cpp

 /*
  * Copyright (C) 2011 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.
  *     * Neither the name of Google Inc. nor the names of its
@@ skipping 247 matching lines @@
             distributeNodeChildrenTo(shadowElement, root->olderShadowRoot());
             root->olderShadowRoot()->ensureScopeDistribution()->setInsertionPointAssignedTo(shadowElement);
         } else {
             distributeSelectionsTo(shadowElement, pool, distributed);
             if (ElementShadow* shadow = shadowElement->parentNode()->isElementNode() ? toElement(shadowElement->parentNode())->shadow() : 0)
                 shadow->invalidateDistribution();
         }
     }
 }
 
-bool ContentDistributor::invalidate(Element* host)
+bool ContentDistributor::invalidate(Element* host, Vector<RefPtr<Node> >& nodesNeedingReattach)
 {
     ASSERT(needsInvalidation());
     bool needsReattach = (m_validity == Undetermined) || !m_nodeToInsertionPoint.isEmpty();
 
     for (ShadowRoot* root = host->youngestShadowRoot(); root; root = root->olderShadowRoot()) {
         if (ScopeContentDistribution* scope = root->scopeDistribution()) {
             scope->setInsertionPointAssignedTo(0);
             const Vector<RefPtr<InsertionPoint> >& insertionPoints = scope->ensureInsertionPointList(root);
             for (size_t i = 0; i < insertionPoints.size(); ++i) {
-                needsReattach = needsReattach || true;
+                needsReattach = true;
+                for (Node* child = insertionPoints[i]->firstChild(); child; child = child->nextSibling())
+                    nodesNeedingReattach.append(child);
+
                 insertionPoints[i]->clearDistribution();
 
                 // After insertionPoint's distribution is invalidated, its reprojection should also be invalidated.
                 if (!insertionPoints[i]->isActive())
                     continue;
 
                 if (Element* parent = insertionPoints[i]->parentElement()) {
                     if (ElementShadow* shadow = parent->shadow())
                         shadow->invalidateDistribution();
                 }
@@ skipping 49 matching lines @@
             m_nodeToInsertionPoint.add(node, insertionPoint);
         }
     }
 
     insertionPoint->setDistribution(distribution);
 }
 
 void ContentDistributor::ensureDistribution(ShadowRoot* shadowRoot)
 {
     ASSERT(shadowRoot);
+    return ensureDistribution(shadowRoot->host());
+}
 
+void ContentDistributor::ensureDistribution(ElementShadow* shadow)

[esprehn, 2013/06/14 08:32:29]

I'd prefer if this was just a method of Element itself.

element->ensureDistribtion(). Internally it can check for shadow() and such. All
the overloads make this confusing.

[Hajime Morrita, 2013/06/14 09:59:14]

makes sense. will do.

+{
+    ASSERT(shadow);
+    return ensureDistribution(shadow->host());
+}
+
+void ContentDistributor::ensureDistribution(Element* host)
+{
     Vector<ElementShadow*, 8> elementShadows;
-    for (Element* current = shadowRoot->host(); current; current = current->shadowHost()) {
+    for (Element* current = host; current; current = current->shadowHost()) {
         ElementShadow* elementShadow = current->shadow();
         if (!elementShadow->distributor().needsDistribution())
             break;
 
         elementShadows.append(elementShadow);
     }
 
     for (size_t i = elementShadows.size(); i > 0; --i)
         elementShadows[i - 1]->distributor().distribute(elementShadows[i - 1]->host());
 }
 
 
 void ContentDistributor::invalidateDistribution(Element* host)
 {
+    Vector<RefPtr<Node> > nodesNeedingReattach;

[esprehn, 2013/06/14 08:32:29]

These shouldn't need to be RefPtr. I don't think anything below can run script?
If it can then then Node* isn't safe.

[Hajime Morrita, 2013/06/14 09:59:14]

It shouldn't run any script. WIll turn to raw pointer.

     bool didNeedInvalidation = needsInvalidation();
-    bool needsReattach = didNeedInvalidation ? invalidate(host) : false;
+    bool needsReattach = didNeedInvalidation ? invalidate(host, nodesNeedingReattach) : false;
 
     if (needsReattach && host->attached()) {
         for (Node* n = host->firstChild(); n; n = n->nextSibling())
-            n->lazyReattach();
+            n->lazyReattachIfAttached();
+        for (size_t i = 0; i < nodesNeedingReattach.size(); ++i)
+            nodesNeedingReattach[i]->lazyReattachIfAttached();
         host->setNeedsStyleRecalc();
     }
 
     if (didNeedInvalidation) {
         ASSERT(m_validity == Invalidating);
         m_validity = Invalidated;
     }
 }
 
 const SelectRuleFeatureSet& ContentDistributor::ensureSelectFeatureSet(ElementShadow* shadow)
@@ skipping 40 matching lines @@
 {
     for (ElementShadow* shadow = host->shadow(); shadow; shadow = shadow->containingShadow()) {
         if (shadow->distributor().needsSelectFeatureSet())
             break;
         shadow->distributor().setNeedsSelectFeatureSet();
     }
 
     invalidateDistribution(host);
 }
 
+void ContentDistributor::setNeedsStyleRecalcIfDistributedTo(Element* host, InsertionPoint* insertionPoint)
+{
+    for (NodeInsertionPointMap::iterator i = m_nodeToInsertionPoint.begin(); i != m_nodeToInsertionPoint.end(); ++i) {
+        if (i->value == insertionPoint)
+            const_cast<Node*>(i->key)->setNeedsStyleRecalc(SyntheticStyleChange);

[esprehn, 2013/06/14 08:32:29]

Can you just remove the const from the key in the NodeInsertionPointMap type? It
seems unnecessarily restrictive.

Also lets try InlineStyleChange instead of SyntheticStyleChange. I don't think
you need to trigger RenderLayer updates for identical styles.

+    }
+}
+
 void ContentDistributor::didShadowBoundaryChange(Element* host)
 {
     setValidity(Undetermined);
     invalidateDistribution(host);
 }
 
 }