Skip to content

Commit

Permalink
Merge r179750 - REGRESSION(r179706): Caused memory corruption on some…
Browse files Browse the repository at this point in the history
… tests (Requested by _ap_ on #webkit).

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

Reviewed by Alexey Proskuryakov.

No new tests. This is caught by existing tests under ASAN, and I don't know how to reproduce
it without ASAN.

* rendering/RenderLineBoxList.cpp:
(WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Give up
and just always invalidate the next line. It's too hard to come up
with the condition that catches all needed cases, doesn't itself
cause a crash, and isn't overzealous. And we do this for the
previous line anyway.  Also clean up the code a bit since it
confusingly reuses a variable, and declares it uninitialized, for
no good reason.
  • Loading branch information
othermaciej authored and carlosgcampos committed Feb 28, 2016
1 parent 009cc58 commit 4065c79
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
19 changes: 19 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
2015-02-06 Maciej Stachowiak <mjs@apple.com>

REGRESSION(r179706): Caused memory corruption on some tests (Requested by _ap_ on #webkit).
https://bugs.webkit.org/show_bug.cgi?id=141324

Reviewed by Alexey Proskuryakov.

No new tests. This is caught by existing tests under ASAN, and I don't know how to reproduce
it without ASAN.

* rendering/RenderLineBoxList.cpp:
(WebCore::RenderLineBoxList::dirtyLinesFromChangedChild): Give up
and just always invalidate the next line. It's too hard to come up
with the condition that catches all needed cases, doesn't itself
cause a crash, and isn't overzealous. And we do this for the
previous line anyway. Also clean up the code a bit since it
confusingly reuses a variable, and declares it uninitialized, for
no good reason.

2015-02-05 Maciej Stachowiak <mjs@apple.com>

Crash due to failing to dirty a removed text node's line box
Expand Down
19 changes: 7 additions & 12 deletions Source/WebCore/rendering/RenderLineBoxList.cpp
Expand Up @@ -378,7 +378,6 @@ void RenderLineBoxList::dirtyLinesFromChangedChild(RenderBoxModelObject* contain

// If we found a line box, then dirty it.
if (box) {
RootInlineBox* adjacentBox;
box->markDirty();

// dirty the adjacent lines that might be affected
Expand All @@ -388,17 +387,13 @@ void RenderLineBoxList::dirtyLinesFromChangedChild(RenderBoxModelObject* contain
// calls setLineBreakInfo with the result of findNextLineBreak. findNextLineBreak,
// despite the name, actually returns the first RenderObject after the BR.
// <rdar://problem/3849947> "Typing after pasting line does not appear until after window resize."
adjacentBox = box->prevRootBox();
if (adjacentBox)
adjacentBox->markDirty();
adjacentBox = box->nextRootBox();
// If |child| has been inserted before the first element in the linebox, but after collapsed leading
// space, the search for |child|'s linebox will go past the leading space to the previous linebox and select that
// one as |box|. If we hit that situation here, dirty the |box| actually containing the child too.
bool insertedAfterLeadingSpace = box->lineBreakObj() == child->previousSibling();
if (adjacentBox && (adjacentBox->lineBreakObj()->isDescendantOf(child) || child->isBR() || (curr && curr->isBR())
|| insertedAfterLeadingSpace || isIsolated(container->style().unicodeBidi())))
adjacentBox->markDirty();
if (RootInlineBox* prevBox = box->prevRootBox())
prevBox->markDirty();

// FIXME: We shouldn't need to always dirty the next line. This is only strictly
// necessary some of the time, in situations involving BRs.
if (RootInlineBox* nextBox = box->nextRootBox())
nextBox->markDirty();
}
}

Expand Down

0 comments on commit 4065c79

Please sign in to comment.