Title: [94938] trunk
- Revision
- 94938
- Author
- [email protected]
- Date
- 2011-09-11 20:25:04 -0700 (Sun, 11 Sep 2011)
Log Message
REGRESSION (r87351): toggling display of lots (thousands) of elements with display:none is very slow
https://bugs.webkit.org/show_bug.cgi?id=67581
Reviewed by Darin Adler.
Source/WebCore:
Test: perf/show-hide-table-rows.html
* dom/NodeRenderingContext.cpp:
(WebCore::NodeRendererFactory::createRendererAndStyle): Moved style-creating code into createRendererIfNeeded, renamed
to createRenderer.
(WebCore::NodeRendererFactory::createRendererIfNeeded): Re-arrange code to avoid unnecessary creation of renderers.
LayoutTests:
* perf/show-hide-table-rows-expected.txt: Added.
* perf/show-hide-table-rows.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (94937 => 94938)
--- trunk/LayoutTests/ChangeLog 2011-09-12 03:21:28 UTC (rev 94937)
+++ trunk/LayoutTests/ChangeLog 2011-09-12 03:25:04 UTC (rev 94938)
@@ -1,3 +1,13 @@
+2011-09-11 Dimitri Glazkov <[email protected]>
+
+ REGRESSION (r87351): toggling display of lots (thousands) of elements with display:none is very slow
+ https://bugs.webkit.org/show_bug.cgi?id=67581
+
+ Reviewed by Darin Adler.
+
+ * perf/show-hide-table-rows-expected.txt: Added.
+ * perf/show-hide-table-rows.html: Added.
+
2011-09-11 Fumitoshi Ukai <[email protected]>
Unreviewed, one more rebaseline for r94912
Added: trunk/LayoutTests/perf/show-hide-table-rows-expected.txt (0 => 94938)
--- trunk/LayoutTests/perf/show-hide-table-rows-expected.txt (rev 0)
+++ trunk/LayoutTests/perf/show-hide-table-rows-expected.txt 2011-09-12 03:25:04 UTC (rev 94938)
@@ -0,0 +1,3 @@
+Tests that hiding/showing of table rows is linear.
+PASS
+
Added: trunk/LayoutTests/perf/show-hide-table-rows.html (0 => 94938)
--- trunk/LayoutTests/perf/show-hide-table-rows.html (rev 0)
+++ trunk/LayoutTests/perf/show-hide-table-rows.html 2011-09-12 03:25:04 UTC (rev 94938)
@@ -0,0 +1,39 @@
+<style>
+.hidden { display: none; }
+</style>
+<script src=""
+<body>
+<div></div>
+<script>
+
+function setupFunction(magnitude)
+{
+ var html = '<table>';
+ for (var i = 0; i < magnitude; ++i)
+ html += '<tr><td>A</td><td>B</td><td>C</td><td>D</td><td>E</td><td>F</td></tr>\n';
+ html += '</table>';
+ document.querySelector('div').innerHTML = html;
+}
+
+function forEachRow(what)
+{
+ Array.prototype.forEach.call(document.querySelectorAll("tr"), what);
+}
+
+function test(magnitude)
+{
+ forEachRow(function(tr) {
+ tr.className = 'hidden';
+ });
+ document.body.offsetWidth;
+ forEachRow(function(tr) {
+ tr.className = '';
+ });
+ document.body.offsetWidth;
+}
+
+Magnitude.description("Tests that hiding/showing of table rows is linear.");
+Magnitude.run(setupFunction, test, Magnitude.LINEAR);
+document.querySelector('div').textContent = '';
+</script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (94937 => 94938)
--- trunk/Source/WebCore/ChangeLog 2011-09-12 03:21:28 UTC (rev 94937)
+++ trunk/Source/WebCore/ChangeLog 2011-09-12 03:25:04 UTC (rev 94938)
@@ -1,3 +1,17 @@
+2011-09-11 Dimitri Glazkov <[email protected]>
+
+ REGRESSION (r87351): toggling display of lots (thousands) of elements with display:none is very slow
+ https://bugs.webkit.org/show_bug.cgi?id=67581
+
+ Reviewed by Darin Adler.
+
+ Test: perf/show-hide-table-rows.html
+
+ * dom/NodeRenderingContext.cpp:
+ (WebCore::NodeRendererFactory::createRendererAndStyle): Moved style-creating code into createRendererIfNeeded, renamed
+ to createRenderer.
+ (WebCore::NodeRendererFactory::createRendererIfNeeded): Re-arrange code to avoid unnecessary creation of renderers.
+
2011-09-11 Jeremy Moskovich <[email protected]>
[Chromium] Change OOP Font loading code to use CGFont*() APIs.
Modified: trunk/Source/WebCore/dom/NodeRenderingContext.cpp (94937 => 94938)
--- trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2011-09-12 03:21:28 UTC (rev 94937)
+++ trunk/Source/WebCore/dom/NodeRenderingContext.cpp 2011-09-12 03:25:04 UTC (rev 94938)
@@ -286,29 +286,10 @@
{
}
-RenderObject* NodeRendererFactory::createRendererAndStyle()
+RenderObject* NodeRendererFactory::createRenderer()
{
Node* node = m_context.node();
- Document* document = node->document();
- ASSERT(!node->renderer());
- ASSERT(document->shouldCreateRenderers());
-
- if (!m_context.shouldCreateRenderer())
- return 0;
-
- Element* element = node->isElementNode() ? toElement(node) : 0;
- if (element)
- m_context.setStyle(element->styleForRenderer());
- else if (RenderObject* parentRenderer = m_context.parentRenderer())
- m_context.setStyle(parentRenderer->style());
-
- if (!node->rendererIsNeeded(m_context)) {
- if (element && m_context.style()->affectedByEmpty())
- element->setStyleAffectedByEmpty();
- return 0;
- }
-
- RenderObject* newRenderer = node->createRenderer(document->renderArena(), m_context.style());
+ RenderObject* newRenderer = node->createRenderer(node->document()->renderArena(), m_context.style());
if (!newRenderer)
return 0;
@@ -345,25 +326,38 @@
if (!document->shouldCreateRenderers())
return;
- RenderObject* parentRenderer = m_context.parentRenderer();
- RenderObject* nextRenderer = m_context.nextRenderer();
- RenderObject* newRenderer = createRendererAndStyle();
+ ASSERT(!node->renderer());
+ ASSERT(document->shouldCreateRenderers());
- if (m_context.hasFlowThreadParent()) {
- parentRenderer = m_context.parentFlowRenderer();
- // Do not call m_context.nextRenderer() here, because it expects to have
- // the renderer added to its parent already.
- nextRenderer = m_context.parentFlowRenderer()->nextRendererForNode(node);
+ // FIXME: This side effect should be visible from attach() code.
+ m_context.hostChildrenChanged();
+
+ if (!m_context.shouldCreateRenderer())
+ return;
+
+ Element* element = node->isElementNode() ? toElement(node) : 0;
+ if (element)
+ m_context.setStyle(element->styleForRenderer());
+ else if (RenderObject* parentRenderer = m_context.parentRenderer())
+ m_context.setStyle(parentRenderer->style());
+
+ if (!node->rendererIsNeeded(m_context)) {
+ if (element && m_context.style()->affectedByEmpty())
+ element->setStyleAffectedByEmpty();
+ return;
}
+ RenderObject* parentRenderer = m_context.hasFlowThreadParent() ? m_context.parentFlowRenderer() : m_context.parentRenderer();
+ // Do not call m_context.nextRenderer() here in the first clause, because it expects to have
+ // the renderer added to its parent already.
+ RenderObject* nextRenderer = m_context.hasFlowThreadParent() ? m_context.parentFlowRenderer()->nextRendererForNode(node) : m_context.nextRenderer();
+ RenderObject* newRenderer = createRenderer();
+
#if ENABLE(FULLSCREEN_API)
if (document->webkitIsFullScreen() && document->webkitCurrentFullScreenElement() == node)
newRenderer = wrapWithRenderFullScreen(newRenderer, document);
#endif
- // FIXME: This side effect should be visible from attach() code.
- m_context.hostChildrenChanged();
-
if (!newRenderer)
return;
Modified: trunk/Source/WebCore/dom/NodeRenderingContext.h (94937 => 94938)
--- trunk/Source/WebCore/dom/NodeRenderingContext.h 2011-09-12 03:21:28 UTC (rev 94937)
+++ trunk/Source/WebCore/dom/NodeRenderingContext.h 2011-09-12 03:25:04 UTC (rev 94938)
@@ -123,7 +123,7 @@
void createRendererIfNeeded();
private:
- RenderObject* createRendererAndStyle();
+ RenderObject* createRenderer();
NodeRenderingContext m_context;
};
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
