Title: [154288] trunk/Source/WebCore
- Revision
- 154288
- Author
- [email protected]
- Date
- 2013-08-19 12:11:40 -0700 (Mon, 19 Aug 2013)
Log Message
<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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (154287 => 154288)
--- trunk/Source/WebCore/ChangeLog 2013-08-19 18:28:02 UTC (rev 154287)
+++ trunk/Source/WebCore/ChangeLog 2013-08-19 19:11:40 UTC (rev 154288)
@@ -1,3 +1,24 @@
+2013-08-19 Darin Adler <[email protected]>
+
+ <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 <[email protected]>
<https://webkit.org/b/119918> Frame::selection() should return a reference
Modified: trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp (154287 => 154288)
--- trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp 2013-08-19 18:28:02 UTC (rev 154287)
+++ trunk/Source/WebCore/html/HTMLTableRowsCollection.cpp 2013-08-19 19:11:40 UTC (rev 154288)
@@ -37,23 +37,45 @@
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 @@
// 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 @@
}
// 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 @@
}
// 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();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
