Diff
Modified: trunk/Source/WebCore/ChangeLog (99019 => 99020)
--- trunk/Source/WebCore/ChangeLog 2011-11-02 00:55:49 UTC (rev 99019)
+++ trunk/Source/WebCore/ChangeLog 2011-11-02 01:12:01 UTC (rev 99020)
@@ -1,3 +1,27 @@
+2011-11-01 Julien Chaffraix <[email protected]>
+
+ Pack RenderTableCell bits
+ https://bugs.webkit.org/show_bug.cgi?id=71135
+
+ Reviewed by Darin Adler.
+
+ Tested by RenderTableCellTest unit test.
+ (unfortunately Chromium specific...)
+
+ This saves another 8 bytes on RenderTableCell on x86-64.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::RenderTableCell):
+ * rendering/RenderTableCell.h:
+ Changed the field order to use more strict packing.
+
+ (WebCore::RenderTableCell::setCol):
+ (WebCore::RenderTableCell::setRow):
+ Added overflow checks to the 2 previous methods. We
+ CRASH even in release to avoid potential badness
+ (the limit is currently above 2 billions rows or columns
+ which is high enough to prevent it being hit by accident)
+
2011-11-01 Emil A Eklund <[email protected]>
Switch background/border image back to Int
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (99019 => 99020)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-02 00:55:49 UTC (rev 99019)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-02 01:12:01 UTC (rev 99020)
@@ -44,11 +44,11 @@
RenderTableCell::RenderTableCell(Node* node)
: RenderBlock(node)
, m_row(unsetRowIndex)
+ , m_cellWidthChanged(false)
, m_column(unsetColumnIndex)
+ , m_hasAssociatedTableCellElement(node && (node->hasTagName(tdTag) || node->hasTagName(thTag)))
, m_intrinsicPaddingBefore(0)
, m_intrinsicPaddingAfter(0)
- , m_cellWidthChanged(false)
- , m_hasAssociatedTableCellElement(node && (node->hasTagName(tdTag) || node->hasTagName(thTag)))
{
}
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (99019 => 99020)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-02 00:55:49 UTC (rev 99019)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-02 01:12:01 UTC (rev 99020)
@@ -29,11 +29,12 @@
namespace WebCore {
-// FIXME: It is possible for these indexes to be reached for a table big enough.
-// We would need to enforce a maximal index on both rows and columns.
-static const unsigned unsetColumnIndex = 0xFFFFFFFF;
-static const unsigned unsetRowIndex = 0xFFFFFFFF;
+static const unsigned unsetColumnIndex = 0x7FFFFFFF;
+static const unsigned maxColumnIndex = 0x7FFFFFFE; // 2,147,483,646
+static const unsigned unsetRowIndex = 0x7FFFFFFF;
+static const unsigned maxRowIndex = 0x7FFFFFFE; // 2,147,483,646
+
class RenderTableCell : public RenderBlock {
public:
explicit RenderTableCell(Node*);
@@ -48,14 +49,28 @@
// Called from HTMLTableCellElement.
void colSpanOrRowSpanChanged();
- void setCol(unsigned column) { m_column = column; }
+ void setCol(unsigned column)
+ {
+ if (UNLIKELY(column > maxColumnIndex))
+ CRASH();
+
+ m_column = column;
+ }
+
unsigned col() const
{
ASSERT(m_column != unsetColumnIndex);
return m_column;
}
- void setRow(unsigned row) { m_row = row; }
+ void setRow(unsigned row)
+ {
+ if (UNLIKELY(row > maxRowIndex))
+ CRASH();
+
+ m_row = row;
+ }
+
unsigned row() const
{
ASSERT(m_row != unsetRowIndex);
@@ -159,14 +174,12 @@
void paintCollapsedBorder(GraphicsContext*, const LayoutRect&);
- unsigned m_row;
- unsigned m_column;
+ unsigned m_row : 31;
+ bool m_cellWidthChanged : 1;
+ unsigned m_column : 31;
+ bool m_hasAssociatedTableCellElement : 1;
int m_intrinsicPaddingBefore;
int m_intrinsicPaddingAfter;
-
- // FIXME: It would be nice to pack these 2 bits into some of the previous fields.
- bool m_cellWidthChanged : 1;
- bool m_hasAssociatedTableCellElement : 1;
};
inline RenderTableCell* toRenderTableCell(RenderObject* object)
Modified: trunk/Source/WebKit/chromium/ChangeLog (99019 => 99020)
--- trunk/Source/WebKit/chromium/ChangeLog 2011-11-02 00:55:49 UTC (rev 99019)
+++ trunk/Source/WebKit/chromium/ChangeLog 2011-11-02 01:12:01 UTC (rev 99020)
@@ -1,3 +1,15 @@
+2011-11-01 Julien Chaffraix <[email protected]>
+
+ Pack RenderTableCell bits
+ https://bugs.webkit.org/show_bug.cgi?id=71135
+
+ Reviewed by Darin Adler.
+
+ * WebKit.gypi:
+ * tests/RenderTableCellTest.cpp: Added.
+ Added some tests for the column/row index limit
+ that was implemented in RenderTableCell.
+
2011-11-01 Nate Chapin <[email protected]>
Add 2 new TargetTypes (to match ResourceRequest::TargetType)
Modified: trunk/Source/WebKit/chromium/WebKit.gypi (99019 => 99020)
--- trunk/Source/WebKit/chromium/WebKit.gypi 2011-11-02 00:55:49 UTC (rev 99019)
+++ trunk/Source/WebKit/chromium/WebKit.gypi 2011-11-02 01:12:01 UTC (rev 99020)
@@ -80,6 +80,7 @@
'tests/PODArenaTest.cpp',
'tests/PODIntervalTreeTest.cpp',
'tests/PODRedBlackTreeTest.cpp',
+ 'tests/RenderTableCellTest.cpp',
'tests/TilingDataTest.cpp',
'tests/TreeSynchronizerTest.cpp',
'tests/TreeTestHelpers.cpp',
Added: trunk/Source/WebKit/chromium/tests/RenderTableCellTest.cpp (0 => 99020)
--- trunk/Source/WebKit/chromium/tests/RenderTableCellTest.cpp (rev 0)
+++ trunk/Source/WebKit/chromium/tests/RenderTableCellTest.cpp 2011-11-02 01:12:01 UTC (rev 99020)
@@ -0,0 +1,135 @@
+/*
+ * 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:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+// FIXME: If we get the TestWebKitAPI framework to bring a full Frame + DOM stack
+// in a portable way, this test should be shared with all ports!
+
+#include "config.h"
+
+#include "RenderTableCell.h"
+
+#include "Document.h"
+#include "Frame.h"
+#include "FrameTestHelpers.h"
+#include "RenderArena.h"
+#include "WebFrame.h"
+#include "WebFrameImpl.h"
+#include "WebView.h"
+
+#include <gtest/gtest.h>
+
+using namespace WebKit;
+
+namespace WebCore {
+
+namespace {
+
+class RenderTableCellDeathTest : public testing::Test {
+ // It's unfortunate that we have to get the whole browser stack to test one RenderObject
+ // but the code needs it.
+ static Frame* frame()
+ {
+ static WebView* webView;
+
+ if (webView)
+ return static_cast<WebFrameImpl*>(webView->mainFrame())->frame();
+
+ webView = FrameTestHelpers::createWebViewAndLoad("about:blank");
+ webView->setFocus(true);
+ return static_cast<WebFrameImpl*>(webView->mainFrame())->frame();
+ }
+
+ static Document* document()
+ {
+ return frame()->document();
+ }
+
+ static RenderArena* arena()
+ {
+ return document()->renderArena();
+ }
+
+ virtual void SetUp()
+ {
+ m_cell = new (arena()) RenderTableCell(document());
+ }
+
+ virtual void TearDown()
+ {
+ m_cell->destroy();
+ }
+
+protected:
+ RenderTableCell* m_cell;
+};
+
+TEST_F(RenderTableCellDeathTest, CanSetColumn)
+{
+ static const unsigned columnIndex = 10;
+ m_cell->setCol(columnIndex);
+ EXPECT_EQ(columnIndex, m_cell->col());
+}
+
+TEST_F(RenderTableCellDeathTest, CanSetRow)
+{
+ static const unsigned rowIndex = 10;
+ m_cell->setRow(rowIndex);
+ EXPECT_EQ(rowIndex, m_cell->row());
+}
+
+TEST_F(RenderTableCellDeathTest, CanSetColumnToMaxColumnIndex)
+{
+ m_cell->setCol(maxColumnIndex);
+ EXPECT_EQ(maxColumnIndex, m_cell->col());
+}
+
+TEST_F(RenderTableCellDeathTest, CanSetRowToMaxRowIndex)
+{
+ m_cell->setRow(maxRowIndex);
+ EXPECT_EQ(maxRowIndex, m_cell->row());
+}
+
+TEST_F(RenderTableCellDeathTest, CrashIfColumnOverflowOnSetting)
+{
+ ASSERT_DEATH(m_cell->setCol(maxColumnIndex + 1), "");
+}
+
+TEST_F(RenderTableCellDeathTest, CrashIfRowOverflowOnSetting)
+{
+ ASSERT_DEATH(m_cell->setRow(maxRowIndex + 1), "");
+}
+
+TEST_F(RenderTableCellDeathTest, CrashIfSettingUnsetColumnIndex)
+{
+ ASSERT_DEATH(m_cell->setCol(unsetColumnIndex), "");
+}
+
+TEST_F(RenderTableCellDeathTest, CrashIfSettingUnsetRowIndex)
+{
+ ASSERT_DEATH(m_cell->setRow(unsetRowIndex), "");
+}
+
+}
+
+} // namespace WebCore
