[Refactoring] Get rid of ContentDistribution::Item
authormorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2012 06:30:25 +0000 (06:30 +0000)
committermorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 May 2012 06:30:25 +0000 (06:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86350

This change replaces the linked list on ContentDistribution with a Vector.
We no longer link item class ContentDistribution::Item.
This simplification also allows ContentDistribution to go.
ContentDistribution is now just a typedef of Vector<RefPtr<Node> >.

Reviewed by Dimitri Glazkov.

No new tests. Covered by existing tests.

* dom/ComposedShadowTreeWalker.cpp:
(WebCore::ComposedShadowTreeWalker::traverseNode):
(WebCore::ComposedShadowTreeWalker::traverseSiblingOrBackToInsertionPoint):
* dom/ElementShadow.cpp:
* dom/ElementShadow.h:
(ElementShadow):
* dom/NodeRenderingContext.cpp:
(WebCore::nextRendererOfInsertionPoint):
(WebCore::previousRendererOfInsertionPoint):
(WebCore::firstRendererOfInsertionPoint):
(WebCore::lastRendererOfInsertionPoint):
* html/shadow/ContentDistributor.cpp:
(WebCore::ContentDistributor::distribute):
(WebCore::ContentDistributor::clearDistribution):
(WebCore::ContentDistributor::findInsertionPointFor):
* html/shadow/ContentDistributor.h:
(WebCore):
(ContentDistributor):
* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::InsertionPoint):
(WebCore::InsertionPoint::attachDistributedNode):
(WebCore::InsertionPoint::assignShadowRoot):
(WebCore::InsertionPoint::nextTo):
(WebCore):
(WebCore::InsertionPoint::previousTo):
* html/shadow/InsertionPoint.h: Added a set of delegate method to m_distribution.
(WebCore::InsertionPoint::hasDistribution):
(WebCore::InsertionPoint::indexOf):
(WebCore::InsertionPoint::size):
(WebCore::InsertionPoint::at):
(WebCore::InsertionPoint::first):
(WebCore::InsertionPoint::last):
(InsertionPoint):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@117033 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/ComposedShadowTreeWalker.cpp
Source/WebCore/dom/ElementShadow.cpp
Source/WebCore/dom/ElementShadow.h
Source/WebCore/dom/NodeRenderingContext.cpp
Source/WebCore/html/shadow/ContentDistributor.cpp
Source/WebCore/html/shadow/ContentDistributor.h
Source/WebCore/html/shadow/InsertionPoint.cpp
Source/WebCore/html/shadow/InsertionPoint.h

index b969f60..d1446b0 100644 (file)
@@ -1,3 +1,51 @@
+2012-05-14  MORITA Hajime  <morrita@google.com>
+
+        [Refactoring] Get rid of ContentDistribution::Item
+        https://bugs.webkit.org/show_bug.cgi?id=86350
+
+        This change replaces the linked list on ContentDistribution with a Vector.
+        We no longer link item class ContentDistribution::Item.
+        This simplification also allows ContentDistribution to go.
+        ContentDistribution is now just a typedef of Vector<RefPtr<Node> >.
+
+        Reviewed by Dimitri Glazkov.
+
+        No new tests. Covered by existing tests.
+
+        * dom/ComposedShadowTreeWalker.cpp:
+        (WebCore::ComposedShadowTreeWalker::traverseNode):
+        (WebCore::ComposedShadowTreeWalker::traverseSiblingOrBackToInsertionPoint):
+        * dom/ElementShadow.cpp:
+        * dom/ElementShadow.h:
+        (ElementShadow):
+        * dom/NodeRenderingContext.cpp:
+        (WebCore::nextRendererOfInsertionPoint):
+        (WebCore::previousRendererOfInsertionPoint):
+        (WebCore::firstRendererOfInsertionPoint):
+        (WebCore::lastRendererOfInsertionPoint):
+        * html/shadow/ContentDistributor.cpp:
+        (WebCore::ContentDistributor::distribute):
+        (WebCore::ContentDistributor::clearDistribution):
+        (WebCore::ContentDistributor::findInsertionPointFor):
+        * html/shadow/ContentDistributor.h:
+        (WebCore):
+        (ContentDistributor):
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::InsertionPoint):
+        (WebCore::InsertionPoint::attachDistributedNode):
+        (WebCore::InsertionPoint::assignShadowRoot):
+        (WebCore::InsertionPoint::nextTo):
+        (WebCore):
+        (WebCore::InsertionPoint::previousTo):
+        * html/shadow/InsertionPoint.h: Added a set of delegate method to m_distribution.
+        (WebCore::InsertionPoint::hasDistribution):
+        (WebCore::InsertionPoint::indexOf):
+        (WebCore::InsertionPoint::size):
+        (WebCore::InsertionPoint::at):
+        (WebCore::InsertionPoint::first):
+        (WebCore::InsertionPoint::last):
+        (InsertionPoint):
+
 2012-05-14  Tim Horton  <timothy_horton@apple.com>
 
         RenderLayer::repaintRectIncludingDescendants shouldn't include repaint rects of composited descendants
index 09b9376..235683f 100644 (file)
@@ -113,8 +113,8 @@ Node* ComposedShadowTreeWalker::traverseNode(const Node* node, TraversalDirectio
 {
     ASSERT(node);
     if (isInsertionPoint(node)) {
-        const ContentDistribution* distribution = toInsertionPoint(node)->distribution();
-        if (Node* next = (direction == TraversalDirectionForward ? distribution->firstNode() : distribution->lastNode()))
+        const InsertionPoint* insertionPoint = toInsertionPoint(node);
+        if (Node* next = (direction == TraversalDirectionForward ? insertionPoint->first() : insertionPoint->last()))
             return traverseNode(next, direction);
         return traverseLightChildren(node, direction);
     }
@@ -141,12 +141,12 @@ Node* ComposedShadowTreeWalker::traverseSiblingOrBackToInsertionPoint(const Node
     ElementShadow* shadow = shadowOfParent(node);
     if (!shadow)
         return traverseSiblingInCurrentTree(node, direction);
-    ContentDistribution::Item* item = shadow->distributionItemFor(node);
-    if (!item)
+    InsertionPoint* insertionPoint = shadow->insertionPointFor(node);
+    if (!insertionPoint)
         return traverseSiblingInCurrentTree(node, direction);
-    if (ContentDistribution::Item* nextItem = (direction == TraversalDirectionForward ? item->next() : item->previous()))
-        return traverseNode(nextItem->node(), direction);
-    return traverseSiblingOrBackToInsertionPoint(item->insertionPoint(), direction);
+    if (Node* next = (direction == TraversalDirectionForward ? insertionPoint->nextTo(node) : insertionPoint->previousTo(node)))
+        return traverseNode(next, direction);
+    return traverseSiblingOrBackToInsertionPoint(insertionPoint, direction);
 }
 
 Node* ComposedShadowTreeWalker::traverseSiblingInCurrentTree(const Node* node, TraversalDirection direction)
index ff85198..73aadea 100644 (file)
@@ -167,11 +167,6 @@ InsertionPoint* ElementShadow::insertionPointFor(const Node* node) const
     return distributor().findInsertionPointFor(node);
 }
 
-ContentDistribution::Item* ElementShadow::distributionItemFor(const Node* node) const
-{
-    return m_distributor.findFor(node);
-}
-
 void ElementShadow::reattach()
 {
     detach();
index d4d7d43..e6f860b 100644 (file)
@@ -70,7 +70,6 @@ public:
     void hostChildrenChanged();
 
     InsertionPoint* insertionPointFor(const Node*) const;
-    ContentDistribution::Item* distributionItemFor(const Node*) const;
 
     ContentDistributor& distributor();
     const ContentDistributor& distributor() const;
index 3d98a6f..b3b1219 100644 (file)
@@ -142,12 +142,12 @@ PassRefPtr<RenderStyle> NodeRenderingContext::releaseStyle()
 
 static inline RenderObject* nextRendererOfInsertionPoint(InsertionPoint* parent, Node* current)
 {
-    ContentDistribution::Item* currentItem = parent->distribution()->find(current);
-    if (!currentItem)
+    size_t start = parent->indexOf(current);
+    if (notFound == start)
         return 0;
 
-    for (ContentDistribution::Item* item = currentItem->next(); item; item = item->next()) {
-        if (RenderObject* renderer = item->node()->renderer())
+    for (size_t i = start + 1; i < parent->size(); ++i) {
+        if (RenderObject* renderer = parent->at(i)->renderer())
             return renderer;
     }
 
@@ -158,10 +158,10 @@ static inline RenderObject* previousRendererOfInsertionPoint(InsertionPoint* par
 {
     RenderObject* lastRenderer = 0;
 
-    for (ContentDistribution::Item* item = parent->distribution()->first(); item; item = item->next()) {
-        if (item->node() == current)
+    for (size_t i = 0; i < parent->size(); ++i) {
+        if (parent->at(i) == current)
             break;
-        if (RenderObject* renderer = item->node()->renderer())
+        if (RenderObject* renderer = parent->at(i)->renderer())
             lastRenderer = renderer;
     }
 
@@ -170,13 +170,10 @@ static inline RenderObject* previousRendererOfInsertionPoint(InsertionPoint* par
 
 static inline RenderObject* firstRendererOfInsertionPoint(InsertionPoint* parent)
 {
-    if (parent->hasDistribution()) {
-        for (ContentDistribution::Item* item = parent->distribution()->first(); item; item = item->next()) {
-            if (RenderObject* renderer = item->node()->renderer())
-                return renderer;
-        }
-
-        return 0;
+    size_t size = parent->size();
+    for (size_t i = 0; i < size; ++i) {
+        if (RenderObject* renderer = parent->at(i)->renderer())
+            return renderer;
     }
 
     return firstRendererOf(parent->firstChild());
@@ -184,13 +181,10 @@ static inline RenderObject* firstRendererOfInsertionPoint(InsertionPoint* parent
 
 static inline RenderObject* lastRendererOfInsertionPoint(InsertionPoint* parent)
 {
-    if (parent->hasDistribution()) {
-        for (ContentDistribution::Item* item = parent->distribution()->last(); item; item = item->previous()) {
-            if (RenderObject* renderer = item->node()->renderer())
-                return renderer;
-        }
-
-        return 0;
+    size_t size = parent->size();
+    for (size_t i = 0; i < size; ++i) {
+        if (RenderObject* renderer = parent->at(size - 1 - i)->renderer())
+            return renderer;
     }
 
     return lastRendererOf(parent->lastChild());
index 3720a03..ac5dbf6 100644 (file)
 
 namespace WebCore {
 
-ContentDistribution::ContentDistribution()
-{
-}
-
-ContentDistribution::~ContentDistribution()
-{
-    ASSERT(isEmpty());
-}
-
-ContentDistribution::Item* ContentDistribution::find(Node* node) const
-{
-    for (ContentDistribution::Item* item = first(); item; item = item->next()) {
-        if (node == item->node())
-            return item;
-    }
-    
-    return 0;
-}
-
-void ContentDistribution::clear()
-{
-    if (isEmpty()) {
-        ASSERT(!m_last);
-        return;
-    }
-
-    RefPtr<ContentDistribution::Item> item = m_first;
-    while (item) {
-        ASSERT(!item->previous());
-        RefPtr<ContentDistribution::Item> nextItem = item->m_next;
-        item->m_next.clear();
-        if (nextItem)
-            nextItem->m_previous.clear();
-        item = nextItem;
-    }
-
-    m_first.clear();
-    m_last.clear();
-}
-
-void ContentDistribution::append(InsertionPoint* insertionPoint, Node* node)
-{
-    RefPtr<Item> child = Item::create(insertionPoint, node);
-
-    if (isEmpty()) {
-        ASSERT(!m_last);
-        m_first = m_last = child;
-        return;
-    }
-
-    ASSERT(!m_last->next());
-    ASSERT(!child->previous());
-    m_last->m_next = child;
-    child->m_previous = m_last;
-    m_last = m_last->next();
-}
-
 ContentDistributor::ContentDistributor()
     : m_phase(Prevented)
 {
@@ -115,30 +58,22 @@ void ContentDistributor::distribute(InsertionPoint* insertionPoint, ContentDistr
         if (!query.matches(child))
             continue;
 
-        distribution->append(insertionPoint, child);
-        m_nodeToInsertionPoint.add(distribution->last());
+        distribution->append(child);
+        m_nodeToInsertionPoint.add(child, insertionPoint);
         m_pool[i] = 0;
     }
 }
 
 void ContentDistributor::clearDistribution(ContentDistribution* list)
 {
-    for (ContentDistribution::Item* item = list->first(); item; item = item->next())
-        m_nodeToInsertionPoint.remove(item);
-
+    for (size_t i = 0; i < list->size(); ++i)
+        m_nodeToInsertionPoint.remove(list->at(i).get());
     list->clear();
 }
 
-ContentDistribution::Item* ContentDistributor::findFor(const Node* key) const
-{
-    InvertedTable::iterator found = m_nodeToInsertionPoint.find<const Node*, Translator>(key);
-    return found != m_nodeToInsertionPoint.end() ? *found : 0;
-}
-
 InsertionPoint* ContentDistributor::findInsertionPointFor(const Node* key) const
 {
-    InvertedTable::iterator found = m_nodeToInsertionPoint.find<const Node*, Translator>(key);
-    return found != m_nodeToInsertionPoint.end() ? (*found)->insertionPoint() : 0;
+    return m_nodeToInsertionPoint.get(key);
 }
 
 void ContentDistributor::willDistribute()
index ad5d8b6..d443772 100644 (file)
@@ -32,7 +32,7 @@
 #define ContentDistributor_h
 
 #include <wtf/Forward.h>
-#include <wtf/HashSet.h>
+#include <wtf/HashMap.h>
 #include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
 
@@ -43,51 +43,7 @@ class InsertionPoint;
 class Node;
 class ShadowRoot;
 
-class ContentDistribution {
-public:
-    // TODO: The class should be reduced into simple Node* which is kept in a vector: https://bugs.webkit.org/show_bug.cgi?id=86350
-    class Item : public RefCounted<Item> {
-    public:
-        friend class ContentDistribution;
-
-        InsertionPoint* insertionPoint() const { return m_insertionPoint; }
-        Node* node() const { return m_node.get(); }
-        Item* next() const { return m_next.get(); }
-        Item* previous() const { return m_previous.get(); }
-
-    private:
-        static PassRefPtr<Item> create(InsertionPoint* insertionPoint, Node* node) {  return adoptRef(new Item(insertionPoint, node)); }
-
-        Item(InsertionPoint* insertionPoint, Node* node)
-            : m_insertionPoint(insertionPoint)
-            , m_node(node)
-        { }
-
-        InsertionPoint* m_insertionPoint;
-        RefPtr<Node> m_node;
-        RefPtr<Item> m_next;
-        RefPtr<Item> m_previous;
-    };
-    
-    ContentDistribution();
-    ~ContentDistribution();
-
-    Item* first() const { return m_first.get(); }
-    Item* last() const { return m_last.get(); }
-    Node* firstNode() const { return m_first ? m_first->node() : 0; }
-    Node* lastNode() const { return m_first ? m_last->node() : 0; }
-    
-    Item* find(Node*) const;
-    bool isEmpty() const { return !m_first; }
-
-    void clear();
-    void append(InsertionPoint*, Node*);
-
-private:
-
-    RefPtr<Item> m_first;
-    RefPtr<Item> m_last;
-};
+typedef Vector<RefPtr<Node> > ContentDistribution;
 
 class ContentDistributor {
     WTF_MAKE_NONCOPYABLE(ContentDistributor);
@@ -97,7 +53,6 @@ public:
 
     void distribute(InsertionPoint*, ContentDistribution*);
     void clearDistribution(ContentDistribution*);
-    ContentDistribution::Item* findFor(const Node* key) const;
     InsertionPoint* findInsertionPointFor(const Node* key) const;
 
     void willDistribute();
@@ -108,21 +63,6 @@ public:
     bool poolIsReady() const;
 
 private:
-    struct Translator {
-    public:
-        static unsigned hash(const Node* key) { return PtrHash<const Node*>::hash(key); }
-        static bool equal(const ContentDistribution::Item* item, const Node* node) { return item->node() == node; }
-    };
-
-    struct Hash {
-        static unsigned hash(ContentDistribution::Item* key) { return PtrHash<const Node*>::hash(key->node()); }
-        static bool equal(ContentDistribution::Item* a, ContentDistribution::Item* b) { return a->node() == b->node(); }
-        static const bool safeToCompareToEmptyOrDeleted = false;
-    };
-
-    // Used as a table from Node to InseretionPoint
-    typedef HashSet<ContentDistribution::Item*, Hash> InvertedTable;
-
     enum DistributionPhase {
         Prevented,
         Started,
@@ -131,7 +71,7 @@ private:
 
     Vector<RefPtr<Node> > m_pool;
     DistributionPhase m_phase;
-    InvertedTable m_nodeToInsertionPoint;
+    HashMap<const Node*, InsertionPoint*> m_nodeToInsertionPoint;
 };
 
 inline bool ContentDistributor::inDistribution() const
index 723b459..56c344d 100644 (file)
@@ -38,7 +38,6 @@ namespace WebCore {
 
 InsertionPoint::InsertionPoint(const QualifiedName& tagName, Document* document)
     : HTMLElement(tagName, document)
-    , m_distribution()
 {
 }
 
@@ -138,8 +137,8 @@ inline void InsertionPoint::clearDistribution(ElementShadow* shadow)
 
 inline void InsertionPoint::attachDistributedNode()
 {
-    for (ContentDistribution::Item* item = m_distribution.first(); item; item = item->next())
-        item->node()->attach();
+    for (size_t i = 0; i < m_distribution.size(); ++i)
+        m_distribution.at(i)->attach();
 }
 
 inline void InsertionPoint::assignShadowRoot(ShadowRoot* shadowRoot)
@@ -147,7 +146,7 @@ inline void InsertionPoint::assignShadowRoot(ShadowRoot* shadowRoot)
     shadowRoot->setAssignedTo(this);
     m_distribution.clear();
     for (Node* node = shadowRoot->firstChild(); node; node = node->nextSibling())
-        m_distribution.append(this, node);
+        m_distribution.append(node);
 }
 
 inline void InsertionPoint::clearAssignment(ShadowRoot* shadowRoot)
@@ -156,4 +155,21 @@ inline void InsertionPoint::clearAssignment(ShadowRoot* shadowRoot)
     m_distribution.clear();
 }
 
+Node* InsertionPoint::nextTo(const Node* node) const
+{
+    size_t index = m_distribution.find(node);
+    if (index == notFound || index + 1 == m_distribution.size())
+        return 0;
+    return m_distribution.at(index + 1).get();
+}
+
+Node* InsertionPoint::previousTo(const Node* node) const
+{
+    size_t index = m_distribution.find(node);
+    if (index == notFound || !index)
+        return 0;
+    return m_distribution.at(index - 1).get();
+}
+
+
 } // namespace WebCore
index 7d1dd12..ac8c76c 100644 (file)
@@ -42,8 +42,7 @@ class InsertionPoint : public HTMLElement {
 public:
     virtual ~InsertionPoint();
 
-    const ContentDistribution* distribution() const { return &m_distribution; }
-    bool hasDistribution() const { return m_distribution.first(); }
+    bool hasDistribution() const { return !m_distribution.isEmpty(); }
     bool isShadowBoundary() const;
     bool isActive() const;
 
@@ -57,6 +56,14 @@ public:
     virtual bool isInsertionPoint() const OVERRIDE { return true; }
     ShadowRoot* assignedFrom() const;
 
+    size_t indexOf(Node* node) const { return m_distribution.find(node); }
+    size_t size() const { return m_distribution.size(); }
+    Node* at(size_t index)  const { return m_distribution.at(index).get(); }
+    Node* first() const { return m_distribution.isEmpty() ? 0 : m_distribution.first().get(); }
+    Node* last() const { return m_distribution.isEmpty() ? 0 : m_distribution.last().get(); }
+    Node* nextTo(const Node*) const;
+    Node* previousTo(const Node*) const;
+
 protected:
     InsertionPoint(const QualifiedName&, Document*);
     virtual bool rendererIsNeeded(const NodeRenderingContext&) OVERRIDE;