<https://webkit.org/b/120013> Tighten up logic in HTMLTableRowsCollection
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Aug 2013 19:11:40 +0000 (19:11 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Aug 2013 19:11:40 +0000 (19:11 +0000)
Reviewed by Andy Estes.

I was looking for incorrect uses of hasLocalName in places where hasTagName should be used.
The use in HTMLTableRowsCollection looked like that kind of mistake, but when I tried to
make a test case to show the mistake, I found I could not. So I wrote assertions to restate
what I learned, and removed an unneeded null check and tightened up the code a bit. This
should make code size slightly smaller.

* html/HTMLTableRowsCollection.cpp:
(WebCore::assertRowIsInTable): Added. Asserts that the row's position in the table is consistent
with the invariants of how the collection class works. A row that is processed here would have
to be an immediate child of the table, or an immediate child of a table section that in turn is
an immediate child of the table.
(WebCore::isInSection): Added. Replaces the three more-specific helper functions. Unlike those,
this does not do a null check.
(WebCore::HTMLTableRowsCollection::rowAfter): Changed to use the two new functions.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLTableRowsCollection.cpp

index 078b34930611044e317282a152f2798208f45924..804bf98df0d631ccfc84c779d47eb397084af8b3 100644 (file)
@@ -1,3 +1,24 @@
+2013-08-19  Darin Adler  <darin@apple.com>
+
+        <https://webkit.org/b/120013> Tighten up logic in HTMLTableRowsCollection
+
+        Reviewed by Andy Estes.
+
+        I was looking for incorrect uses of hasLocalName in places where hasTagName should be used.
+        The use in HTMLTableRowsCollection looked like that kind of mistake, but when I tried to
+        make a test case to show the mistake, I found I could not. So I wrote assertions to restate
+        what I learned, and removed an unneeded null check and tightened up the code a bit. This
+        should make code size slightly smaller.
+
+        * html/HTMLTableRowsCollection.cpp:
+        (WebCore::assertRowIsInTable): Added. Asserts that the row's position in the table is consistent
+        with the invariants of how the collection class works. A row that is processed here would have
+        to be an immediate child of the table, or an immediate child of a table section that in turn is
+        an immediate child of the table.
+        (WebCore::isInSection): Added. Replaces the three more-specific helper functions. Unlike those,
+        this does not do a null check.
+        (WebCore::HTMLTableRowsCollection::rowAfter): Changed to use the two new functions.
+
 2013-08-19  Pratik Solanki  <psolanki@apple.com>
 
         <https://webkit.org/b/119918> Frame::selection() should return a reference
index 5e6d7c732c6091b79f199972bee28eaadfc0cd78..71ddb2519c50589b726a790bbc4337019edda8db 100644 (file)
@@ -37,23 +37,45 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-static bool isInHead(Element* row)
+#if ASSERT_DISABLED
+
+static inline void assertRowIsInTable(HTMLTableRowElement*)
 {
-    return row->parentNode() && toElement(row->parentNode())->hasLocalName(theadTag);
 }
 
-static bool isInBody(Element* row)
+#else
+
+// The HTMLCollection caching mechanism along with the code in this class will
+// guarantee that this row is an immediate child of either the table, a thead,
+// a tbody, or a tfoot.
+static inline void assertRowIsInTable(HTMLTableRowElement* row)
 {
-    return row->parentNode() && toElement(row->parentNode())->hasLocalName(tbodyTag);
+    if (!row)
+        return;
+    ContainerNode* parent = row->parentNode();
+    ASSERT(parent);
+    if (parent->hasTagName(tableTag))
+        return;
+    ASSERT(parent->hasTagName(theadTag) || parent->hasTagName(tbodyTag) || parent->hasTagName(tfootTag));
+    ContainerNode* grandparent = parent->parentNode();
+    ASSERT(grandparent);
+    ASSERT(grandparent->hasTagName(tableTag));
 }
 
-static bool isInFoot(Element* row)
+#endif
+
+static inline bool isInSection(HTMLTableRowElement* row, const QualifiedName& sectionTag)
 {
-    return row->parentNode() && toElement(row->parentNode())->hasLocalName(tfootTag);
+    // Because we know that the parent is a table or a section, all of which are in the HTML
+    // namespace, it's OK to do the faster hasLocalName here instead of the more typical hasTagName,
+    // since we don't need the check for the HTML namespace.
+    return toElement(row->parentNode())->hasLocalName(sectionTag);
 }
 
 HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement* table, HTMLTableRowElement* previous)
 {
+    assertRowIsInTable(previous);
+
     Node* child = 0;
 
     // Start by looking for the next row in this section.
@@ -68,7 +90,7 @@ HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement* table,
     // If still looking at head sections, find the first row in the next head section.
     if (!previous)
         child = table->firstChild();
-    else if (isInHead(previous))
+    else if (isInSection(previous, theadTag))
         child = previous->parentNode()->nextSibling();
     for (; child; child = child->nextSibling()) {
         if (child->hasTagName(theadTag)) {
@@ -80,11 +102,11 @@ HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement* table,
     }
 
     // If still looking at top level and bodies, find the next row in top level or the first in the next body section.
-    if (!previous || isInHead(previous))
+    if (!previous || isInSection(previous, theadTag))
         child = table->firstChild();
     else if (previous->parentNode() == table)
         child = previous->nextSibling();
-    else if (isInBody(previous))
+    else if (isInSection(previous, tbodyTag))
         child = previous->parentNode()->nextSibling();
     for (; child; child = child->nextSibling()) {
         if (child->hasTagName(trTag))
@@ -98,7 +120,7 @@ HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement* table,
     }
 
     // Find the first row in the next foot section.
-    if (!previous || !isInFoot(previous))
+    if (!previous || !isInSection(previous, tfootTag))
         child = table->firstChild();
     else
         child = previous->parentNode()->nextSibling();