Rollout r147756: performance regression

https://bugs.webkit.org/show_bug.cgi?id=114176

Patch by Mihai Maerean <mmaerean@adobe.com> on 2013-04-08
Reviewed by Alexis Menard.

Source/WebCore:

Rolling out the patch for https://bugs.webkit.org/show_bug.cgi?id=74144 "[CSS Regions] Elements in a region
should be assignable to a named flow" because of the performance regression in Parser/html5-full-render.html .

No new tests (because this is a rollout patch).

* dom/Element.cpp:
* dom/Element.h:
* dom/NodeRenderingContext.cpp:
(WebCore::NodeRenderingContext::parentRenderer):
(WebCore::NodeRenderingContext::shouldCreateRenderer):
(WebCore::NodeRenderingContext::moveToFlowThreadIfNeeded):
* dom/NodeRenderingContext.h:
* dom/PseudoElement.h:
* dom/Text.cpp:
(WebCore::Text::textRendererIsNeeded):
(WebCore::Text::updateTextRenderer):
* dom/Text.h:
* rendering/FlowThreadController.cpp:
* rendering/FlowThreadController.h:
* rendering/RenderObject.h:
* rendering/RenderRegion.h:
* svg/SVGElement.cpp:
* svg/SVGElement.h:

LayoutTests:

* fast/regions/flow-body-in-html-expected.txt: Removed.
* fast/regions/flow-body-in-html.html: Removed.
* fast/regions/region-content-flown-into-region-expected.html: Removed.
* fast/regions/region-content-flown-into-region.html: Removed.
* fast/regions/universal-selector-children-to-the-same-region-expected.txt: Removed.
* fast/regions/universal-selector-children-to-the-same-region.html: Removed.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@147983 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ff7dadc3
2013-04-08 Mihai Maerean <mmaerean@adobe.com>
Rollout r147756: performance regression
https://bugs.webkit.org/show_bug.cgi?id=114176
Reviewed by Alexis Menard.
* fast/regions/flow-body-in-html-expected.txt: Removed.
* fast/regions/flow-body-in-html.html: Removed.
* fast/regions/region-content-flown-into-region-expected.html: Removed.
* fast/regions/region-content-flown-into-region.html: Removed.
* fast/regions/universal-selector-children-to-the-same-region-expected.txt: Removed.
* fast/regions/universal-selector-children-to-the-same-region.html: Removed.
2013-04-08 Benjamin Poulain <benjamin@webkit.org>
Remove HTML Notification
PASS: the body tag has flow-into while the html has flow-from.
Bug 74144 - [CSS Regions] Elements in a region should be assignable to a named flow
<html>
<head>
<title>103685 - [CSS Regions] Universal child selector on region breaks the rendering of its content</title>
<style type="text/css">
body {
-webkit-flow-into: foo;
}
html {
-webkit-flow-from: foo;
}
</style>
<script type="text/javascript">
if (window.testRunner)
testRunner.dumpAsText()
</script>
</head>
<body>
PASS: the body tag has flow-into while the html has flow-from.
<p><a href="https://bugs.webkit.org/show_bug.cgi?id=74144">Bug 74144</a> - [CSS Regions] Elements in a region should be assignable to a named flow</p>
</body>
</html>
\ No newline at end of file
<html>
<head>
<style type="text/css">
div {
padding: 1px;
}
.region {
border:solid 1px #bbb;
}
</style>
</head>
<body>
<div class="region"><template>PASS r0: content that goes in the region.</template></div>
<div class="region">
<div>PASS r1: content in the region that has flow-into. 1.</div>
<div>PASS r1: (enclosed in a div) content in the region that has flow-into. 2. <b>PASS r1: (enclosed in a div) content in the region that has flow-into. 3.</b>.</div>
<div>PASS r1: (enclosed in a div) content in the region that has flow-into. 4 .</div>
<div>PASS r1: content in the region that has flow-into. 5.</div>
</div>
<div class="region"><span>PASS r2: content that is already in a flow goes to another flow .</span></div>
<p><a href="https://bugs.webkit.org/show_bug.cgi?id=74144">Bug 74144</a> - [CSS Regions] Elements in a region should be assignable to a named flow</p>
</body>
</html>
\ No newline at end of file
<html>
<head>
<title>74144 - [CSS Regions] Elements in a region should be assignable to a named flow</title>
<style type="text/css">
div {
padding: 1px;
}
.content {
-webkit-flow-into: flow;
}
.region {
-webkit-flow-from: flow;
border:solid 1px #bbb;
}
.redirectContent {
-webkit-flow-into: redirectFlow;
}
.redirectRegion {
-webkit-flow-from: redirectFlow;
border:solid 1px #bbb;
}
.redirectContent2 {
-webkit-flow-into: redirectFlow2;
}
.redirectRegion2 {
-webkit-flow-from: redirectFlow2;
border:solid 1px #bbb;
}
.redirectContentToNowhere {
-webkit-flow-into: redirectToNowhere;
}
</style>
<template class="content">PASS r0: content that goes in the region.</template>
</head>
<body>
<div class="region">
FAIL: this should not be visible 1.
<span class="redirectContentToNowhere">FAIL: this should not be visible (redirected to nowhere) 1.</span>
<div class="redirectContent">PASS r1: content in the region that has flow-into. 1.</div>
FAIL: this should not be visible 2.
<div>
FAIL: this should not be visible 3.
<div class="redirectContent">PASS r1: (enclosed in a div) content in the region that has flow-into. 2.
<div class="redirectContentToNowhere">FAIL: this should not be visible (redirected to nowhere) 2.</div>
<b>PASS r1: (enclosed in a div) content in the region that has flow-into. 3.</b>.
</div>
FAIL: this should not be visible 4.
</div>
<div>
FAIL: this should not be visible 5.
<div class="redirectContent">PASS r1: (enclosed in a div) content in the region that has flow-into. 4
<span class="redirectContent2">PASS r2: content that is already in a flow goes to another flow
<span class="redirectContentToNowhere">FAIL: this should not be visible (redirected to nowhere) 3.</span>.
</span>
.
</div>
FAIL: this should not be visible 7.
</div>
FAIL: this should not be visible 8.
<span class="redirectContentToNowhere">FAIL: this should not be visible (redirected to nowhere) 4.</span>
<div class="redirectContent">PASS r1: content in the region that has flow-into. 5.</div>
FAIL: this should not be visible 9.
</div>
<div class="redirectRegion">
FAIL: this should not be visible 10.
</div>
<div class="redirectRegion2">
FAIL: this should not be visible 11.
</div>
<p><a href="https://bugs.webkit.org/show_bug.cgi?id=74144">Bug 74144</a> - [CSS Regions] Elements in a region should be assignable to a named flow</p>
</body>
</html>
\ No newline at end of file
PASS: this is inline
PASS: and block content inside a region that is flowed into the same region
PASS: button
Bug 74144 - [CSS Regions] Elements in a region should be assignable to a named flow
<html>
<head>
<title>103685 - [CSS Regions] Universal child selector on region breaks the rendering of its content</title>
<style type="text/css">
.region > * {
-webkit-flow-into: foo;
}
.region {
-webkit-flow-from: foo;
}
</style>
<script type="text/javascript">
if (window.testRunner)
testRunner.dumpAsText()
</script>
</head>
<body>
<div class="region"><span>PASS: this is inline</span><div><p>PASS: and block content inside a region that is flowed into the same region</p></div><button>PASS: button</button></div>
<p><a href="https://bugs.webkit.org/show_bug.cgi?id=74144">Bug 74144</a> - [CSS Regions] Elements in a region should be assignable to a named flow</p>
</body>
</html>
\ No newline at end of file
2013-04-08 Mihai Maerean <mmaerean@adobe.com>
Rollout r147756: performance regression
https://bugs.webkit.org/show_bug.cgi?id=114176
Reviewed by Alexis Menard.
Rolling out the patch for https://bugs.webkit.org/show_bug.cgi?id=74144 "[CSS Regions] Elements in a region
should be assignable to a named flow" because of the performance regression in Parser/html5-full-render.html .
No new tests (because this is a rollout patch).
* dom/Element.cpp:
* dom/Element.h:
* dom/NodeRenderingContext.cpp:
(WebCore::NodeRenderingContext::parentRenderer):
(WebCore::NodeRenderingContext::shouldCreateRenderer):
(WebCore::NodeRenderingContext::moveToFlowThreadIfNeeded):
* dom/NodeRenderingContext.h:
* dom/PseudoElement.h:
* dom/Text.cpp:
(WebCore::Text::textRendererIsNeeded):
(WebCore::Text::updateTextRenderer):
* dom/Text.h:
* rendering/FlowThreadController.cpp:
* rendering/FlowThreadController.h:
* rendering/RenderObject.h:
* rendering/RenderRegion.h:
* svg/SVGElement.cpp:
* svg/SVGElement.h:
2013-04-08 Benjamin Poulain <benjamin@webkit.org>
Remove HTML Notification
......@@ -2558,32 +2558,6 @@ RenderRegion* Element::renderRegion() const
return 0;
}
bool Element::moveToFlowThreadIsNeeded(RefPtr<RenderStyle>& cachedStyle)
{
Document* doc = document();
if (!doc->cssRegionsEnabled())
return false;
#if ENABLE(FULLSCREEN_API)
if (doc->webkitIsFullScreen() && doc->webkitCurrentFullScreenElement() == this)
return false;
#endif
if (isInShadowTree())
return false;
if (!cachedStyle)
cachedStyle = styleForRenderer();
if (!cachedStyle)
return false;
if (cachedStyle->flowThread().isEmpty())
return false;
return !document()->renderView()->flowThreadController()->isContentNodeRegisteredWithAnyNamedFlow(this);
}
#if ENABLE(CSS_REGIONS)
const AtomicString& Element::webkitRegionOverset() const
......
......@@ -609,7 +609,6 @@ public:
PassRefPtr<RenderStyle> styleForRenderer();
RenderRegion* renderRegion() const;
virtual bool moveToFlowThreadIsNeeded(RefPtr<RenderStyle>& cachedStyle);
#if ENABLE(CSS_REGIONS)
const AtomicString& webkitRegionOverset() const;
Vector<RefPtr<Range> > webkitGetRegionFlowRanges() const;
......
......@@ -148,7 +148,7 @@ RenderObject* NodeRenderingContext::previousRenderer() const
return 0;
}
RenderObject* NodeRenderingContext::parentRenderer()
RenderObject* NodeRenderingContext::parentRenderer() const
{
if (RenderObject* renderer = m_node->renderer())
return renderer->parent();
......@@ -170,15 +170,10 @@ RenderObject* NodeRenderingContext::parentRenderer()
if (m_parentFlowRenderer)
return m_parentFlowRenderer;
if (m_node->isElementNode() && toElement(m_node)->moveToFlowThreadIsNeeded(m_style)) {
moveToFlowThread();
return m_parentFlowRenderer;
}
return m_renderingParent ? m_renderingParent->renderer() : 0;
}
bool NodeRenderingContext::shouldCreateRenderer()
bool NodeRenderingContext::shouldCreateRenderer() const
{
if (!m_node->document()->shouldCreateRenderers())
return false;
......@@ -187,16 +182,8 @@ bool NodeRenderingContext::shouldCreateRenderer()
RenderObject* parentRenderer = this->parentRenderer();
if (!parentRenderer)
return false;
if (!parentRenderer->canHaveChildren()) {
if (parentRenderer->canDOMChildrenHaveRenderParent()) {
// In a region, only the children that need to be in a flow thread should have a renderer.
bool shouldBeInNamedFlow = m_node->isElementNode() && toElement(m_node)->moveToFlowThreadIsNeeded(m_style);
if (!shouldBeInNamedFlow)
return false;
} else
return false;
}
if (!parentRenderer->canHaveChildren())
return false;
if (!m_renderingParent->childShouldCreateRenderer(*this))
return false;
return true;
......@@ -205,21 +192,35 @@ bool NodeRenderingContext::shouldCreateRenderer()
void NodeRenderingContext::moveToFlowThreadIfNeeded()
{
ASSERT(m_node->isElementNode());
ASSERT(m_style);
if (!m_node->document()->cssRegionsEnabled())
return;
if (!toElement(m_node)->moveToFlowThreadIsNeeded(m_style))
if (m_style->flowThread().isEmpty())
return;
moveToFlowThread();
}
// As per http://dev.w3.org/csswg/css3-regions/#flow-into, pseudo-elements such as ::first-line, ::first-letter, ::before or ::after
// cannot be directly collected into a named flow.
if (m_node->isPseudoElement())
return;
void NodeRenderingContext::moveToFlowThread()
{
ASSERT(m_node->isElementNode());
ASSERT(toElement(m_node)->moveToFlowThreadIsNeeded(m_style));
// FIXME: Do not collect elements if they are in shadow tree.
if (m_node->isInShadowTree())
return;
#if ENABLE(FULLSCREEN_API)
Document* document = m_node->document();
if (document->webkitIsFullScreen() && document->webkitCurrentFullScreenElement() == m_node)
return;
#endif
#if ENABLE(SVG)
// Allow only svg root elements to be directly collected by a render flow thread.
if (m_node->isSVGElement()
&& (!(m_node->hasTagName(SVGNames::svgTag) && m_node->parentNode() && !m_node->parentNode()->isSVGElement())))
return;
#endif
if (!m_style)
m_style = toElement(m_node)->styleForRenderer();
ASSERT(m_style);
m_flowThread = m_style->flowThread();
ASSERT(m_node->document()->renderView());
FlowThreadController* flowThreadController = m_node->document()->renderView()->flowThreadController();
......
......@@ -55,7 +55,7 @@ public:
Node* node() const;
ContainerNode* parentNodeForRenderingAndStyle() const;
bool resetStyleInheritance() const;
RenderObject* parentRenderer(); // the renderer that will be the parent for this node's renderer. In the case of RenderFlowThreads, it may need to create it.
RenderObject* parentRenderer() const;
RenderObject* nextRenderer() const;
RenderObject* previousRenderer() const;
InsertionPoint* insertionPoint() const;
......@@ -66,8 +66,7 @@ public:
bool isOnEncapsulationBoundary() const;
private:
bool shouldCreateRenderer();
void moveToFlowThread();
bool shouldCreateRenderer() const;
void moveToFlowThreadIfNeeded();
Node* m_node;
......
......@@ -46,14 +46,6 @@ public:
virtual void attach() OVERRIDE;
virtual bool rendererIsNeeded(const NodeRenderingContext&) OVERRIDE;
// As per http://dev.w3.org/csswg/css3-regions/#flow-into, pseudo-elements such as ::first-line, ::first-letter, ::before or ::after
// cannot be directly collected into a named flow.
virtual bool moveToFlowThreadIsNeeded(RefPtr<RenderStyle>& cachedStyle) OVERRIDE
{
UNUSED_PARAM(cachedStyle);
return false;
}
virtual bool canStartSelection() const OVERRIDE { return false; }
virtual bool canContainRangeEndPoint() const OVERRIDE { return false; }
......
......@@ -197,7 +197,7 @@ PassRefPtr<Node> Text::cloneNode(bool /*deep*/)
return create(document(), data());
}
bool Text::textRendererIsNeeded(NodeRenderingContext& context)
bool Text::textRendererIsNeeded(const NodeRenderingContext& context)
{
if (isEditingText())
return true;
......@@ -302,12 +302,7 @@ void Text::updateTextRenderer(unsigned offsetOfReplacedData, unsigned lengthOfRe
if (!attached())
return;
RenderText* textRenderer = toRenderText(renderer());
if (!textRenderer) {
reattach();
return;
}
NodeRenderingContext renderingContext(this, textRenderer->style());
if (!textRendererIsNeeded(renderingContext)) {
if (!textRenderer || !textRendererIsNeeded(NodeRenderingContext(this, textRenderer->style()))) {
reattach();
return;
}
......
......@@ -46,7 +46,7 @@ public:
void recalcTextStyle(StyleChange);
void createTextRendererIfNeeded();
bool textRendererIsNeeded(NodeRenderingContext&);
bool textRendererIsNeeded(const NodeRenderingContext&);
RenderText* createTextRenderer(RenderArena*, RenderStyle*);
void updateTextRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData);
......
......@@ -227,11 +227,6 @@ void FlowThreadController::updateFlowThreadsIntoConstrainedPhase()
}
}
bool FlowThreadController::isContentNodeRegisteredWithAnyNamedFlow(Node* contentNode) const
{
return m_mapNamedFlowContentNodes.contains(contentNode);
}
#ifndef NDEBUG
bool FlowThreadController::isAutoLogicalHeightRegionsCountConsistent() const
{
......
......@@ -66,7 +66,6 @@ public:
void registerNamedFlowContentNode(Node*, RenderNamedFlowThread*);
void unregisterNamedFlowContentNode(Node*);
bool isContentNodeRegisteredWithAnyNamedFlow(Node*) const;
bool hasFlowThreadsWithAutoLogicalHeightRegions() const { return m_flowThreadsWithAutoLogicalHeightRegions; }
void incrementFlowThreadsWithAutoLogicalHeightRegions() { ++m_flowThreadsWithAutoLogicalHeightRegions; }
......
......@@ -251,7 +251,6 @@ public:
// RenderObject tree manipulation
//////////////////////////////////////////
virtual bool canHaveChildren() const { return virtualChildren(); }
virtual bool canDOMChildrenHaveRenderParent() const { return false; } // Even if this render object can't have render children, the children in the DOM tree may have a render parent (that is different from this object).
virtual bool canHaveGeneratedChildren() const;
virtual bool isChildAllowed(RenderObject*, RenderStyle*) const { return true; }
virtual void addChild(RenderObject* newChild, RenderObject* beforeChild = 0);
......
......@@ -145,7 +145,6 @@ private:
virtual const char* renderName() const { return "RenderRegion"; }
virtual bool canHaveChildren() const OVERRIDE { return false; }
virtual bool canDOMChildrenHaveRenderParent() const OVERRIDE { return true; }
virtual void insertedIntoTree() OVERRIDE;
virtual void willBeRemovedFromTree() OVERRIDE;
......
......@@ -452,12 +452,6 @@ static bool hasLoadListener(Element* element)
return false;
}
bool SVGElement::moveToFlowThreadIsNeeded(RefPtr<RenderStyle>& cachedStyle)
{
// Allow only svg root elements to be directly collected by a render flow thread.
return parentNode() && !parentNode()->isSVGElement() && hasTagName(SVGNames::svgTag) && Element::moveToFlowThreadIsNeeded(cachedStyle);
}
void SVGElement::sendSVGLoadEventIfPossible(bool sendParentLoadEvents)
{
RefPtr<SVGElement> currentTarget = this;
......
......@@ -120,8 +120,6 @@ public:
virtual bool addEventListener(const AtomicString& eventType, PassRefPtr<EventListener>, bool useCapture) OVERRIDE;
virtual bool removeEventListener(const AtomicString& eventType, EventListener*, bool useCapture) OVERRIDE;
virtual bool moveToFlowThreadIsNeeded(RefPtr<RenderStyle>& cachedStyle) OVERRIDE;
protected:
SVGElement(const QualifiedName&, Document*, ConstructionType = CreateSVGElement);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment