Skip to content

GitLab

T
tangled
Commit fbdab111 authored by wangxianzhu@chromium.org's avatar wangxianzhu@chromium.org
Browse files
Options

Reattach whitespace siblings only when needed 

When an element's renderer is being reattached, previously we reattach
whitespace siblings, causing full paint invalidation of the parent.

If the whitespace sibling needed renderer and still needs renderer,
don't reattach.

- Renamed Node::reattachWhitespaceSiblings() to
  Node::reattachWhitespaceSlibingsIfNeeded();
- In the function, test if rendererIsNeeded changed before reattaching;
- Modified Text::textRendererIsNeeded() so that it can return correct
  value when the Text has already renderer.

BUG=428997
TEST=fast/repaint/absolute-display-block-to-none.html
TEST=fast/forms/select-listbox-focus-displaynone.html (Existing test
covering the change in Text.cpp).

Review URL: https://codereview.chromium.org/684633006

git-svn-id: svn://svn.chromium.org/blink/trunk@185421 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 6b4d2027
Hide whitespace changes
Inline Side-by-side
Showing with 60 additions and 10 deletions
third_party/WebKit/LayoutTests/fast/repaint/absolute-display-block-to-none-expected.txt 0 → 100644
View file @ fbdab111
{
"bounds": [800, 600],
"children": [
{
"bounds": [800, 600],
"contentsOpaque": true,
"drawsContent": true,
"repaintRects": [
[100, 100, 100, 100]
]
}
]
}
third_party/WebKit/LayoutTests/fast/repaint/absolute-display-block-to-none.html 0 → 100644
View file @ fbdab111
<!DOCTYPE html>
<script src="resources/text-based-repaint.js"></script>
<script>
function repaintTest() {
document.getElementById('absolute').style.display = 'none';
}
onload = runRepaintTest;
</script>
When an absolute positioned element is set display:none, we should not invalidate the whole body.
<div id="absolute" style="position: absolute; width: 100px; height: 100px; top: 100px; left: 100px; background-color: red"></div>
third_party/WebKit/Source/core/dom/Element.cpp
View file @ fbdab111
......@@ -1509,7 +1509,7 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling)
didRecalcStyle(change);
if (change == Reattach)
reattachWhitespaceSiblings(nextTextSibling);
reattachWhitespaceSiblingsIfNeeded(nextTextSibling);
}
StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change)
......
third_party/WebKit/Source/core/dom/Node.cpp
View file @ fbdab111
......@@ -954,16 +954,15 @@ void Node::detach(const AttachContext& context)
#endif
}
void Node::reattachWhitespaceSiblings(Text* start)
void Node::reattachWhitespaceSiblingsIfNeeded(Text* start)
{
for (Node* sibling = start; sibling; sibling = sibling->nextSibling()) {
if (sibling->isTextNode() && toText(sibling)->containsOnlyWhitespace()) {
bool hadRenderer = !!sibling->renderer();
sibling->reattach();
// If the reattach didn't toggle the visibility of the whitespace we don't
// need to continue reattaching siblings since they won't toggle visibility
// either.
if (hadRenderer == !!sibling->renderer())
toText(sibling)->reattachIfNeeded();
// If sibling's renderer status didn't change we don't need to continue checking
// other siblings since their renderer status won't change either.
if (!!sibling->renderer() == hadRenderer)
return;
} else if (sibling->renderer()) {
return;
......
third_party/WebKit/Source/core/dom/Node.h
View file @ fbdab111
......@@ -744,7 +744,7 @@ protected:
virtual void didMoveToNewDocument(Document& oldDocument);
static void reattachWhitespaceSiblings(Text* start);
static void reattachWhitespaceSiblingsIfNeeded(Text* start);
#if !ENABLE(OILPAN)
void willBeDeletedFromDocument();
......
third_party/WebKit/Source/core/dom/Text.cpp
View file @ fbdab111
......@@ -279,7 +279,7 @@ bool Text::textRendererIsNeeded(const RenderStyle& style, const RenderObject& pa
RenderObject* first = parent.slowFirstChild();
while (first && first->isFloatingOrOutOfFlowPositioned() && maxSiblingsToVisit--)
first = first->nextSibling();
if (!first || NodeRenderingTraversal::nextSiblingRenderer(this) == first)
if (!first || first == renderer() || NodeRenderingTraversal::nextSiblingRenderer(this) == first)
// Whitespace at the start of a block just goes away. Don't even
// make a render object for this text.
return false;
......@@ -316,6 +316,32 @@ void Text::attach(const AttachContext& context)
CharacterData::attach(context);
}
void Text::reattachIfNeeded(const AttachContext& context)
{
bool rendererIsNeeded = false;
ContainerNode* renderingParent = NodeRenderingTraversal::parent(this);
if (renderingParent) {
if (RenderObject* parentRenderer = renderingParent->renderer()) {
if (textRendererIsNeeded(*parentRenderer->style(), *parentRenderer))
rendererIsNeeded = true;
}
}
if (rendererIsNeeded == !!renderer())
return;
// The following is almost the same as Node::reattach() except that we create renderer only if needed.
// Not calling reattach() to avoid repeated calls to Text::textRendererIsNeeded().
AttachContext reattachContext(context);
reattachContext.performingReattach = true;
if (styleChangeType() < NeedsReattachStyleChange)
detach(reattachContext);
if (rendererIsNeeded)
RenderTreeBuilderForText(this, renderingParent->renderer()).createRenderer();
CharacterData::attach(reattachContext);
}
void Text::recalcTextStyle(StyleRecalcChange change, Text* nextTextSibling)
{
if (RenderText* renderer = this->renderer()) {
......@@ -327,7 +353,7 @@ void Text::recalcTextStyle(StyleRecalcChange change, Text* nextTextSibling)
} else if (needsStyleRecalc() || needsWhitespaceRenderer()) {
reattach();
if (this->renderer())
reattachWhitespaceSiblings(nextTextSibling);
reattachWhitespaceSiblingsIfNeeded(nextTextSibling);
}
}
......
third_party/WebKit/Source/core/dom/Text.h
View file @ fbdab111
......@@ -56,6 +56,7 @@ public:
void updateTextRenderer(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData, RecalcStyleBehavior = DoNotRecalcStyle);
virtual void attach(const AttachContext& = AttachContext()) override final;
void reattachIfNeeded(const AttachContext& = AttachContext());
virtual bool canContainRangeEndPoint() const override final { return true; }
virtual NodeType nodeType() const override;
......
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