Diff
Modified: trunk/LayoutTests/ChangeLog (101555 => 101556)
--- trunk/LayoutTests/ChangeLog 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/LayoutTests/ChangeLog 2011-11-30 22:33:20 UTC (rev 101556)
@@ -1,3 +1,21 @@
+2011-11-29 Ryosuke Niwa <rn...@webkit.org>
+
+ Assertion failure (m_nestedIsolateCount >= 1) in BidiResolver::exitIsolate()
+ https://bugs.webkit.org/show_bug.cgi?id=69267
+
+ Reviewed by Eric Seidel.
+
+ Add a regression test for the assertion failure. Also add a regression test for a hang
+ found by Levi Weintraub and Jeremy Moskovich.
+
+ This patch also fixes the assertion failure in fast/block/child-not-removed-from-parent-lineboxes-crash.html
+ introduced by r101268.
+
+ * fast/text/bidi-isolate-hang-with-neutral-expected.html: Added.
+ * fast/text/bidi-isolate-hang-with-neutral.html: Added.
+ * fast/text/bidi-isolate-nextlinebreak-failure-expected.txt: Added.
+ * fast/text/bidi-isolate-nextlinebreak-failure.html: Added.
+
2011-11-30 Tim Horton <timothy_hor...@apple.com>
Unreviewed new Snow Leopard baseline after 101537.
Added: trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral-expected.html (0 => 101556)
--- trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral-expected.html 2011-11-30 22:33:20 UTC (rev 101556)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests having exclamation mark inside an isolated run following a line break.
+WebKit should not fall into an infinite loop or hit an assertion.</p>
+<div style="text-align: right"><br><span>!a</span></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral.html (0 => 101556)
--- trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-hang-with-neutral.html 2011-11-30 22:33:20 UTC (rev 101556)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests having exclamation mark inside an isolated run following a line break.
+WebKit should not fall into an infinite loop or hit an assertion.</p>
+<div dir="rtl"><br><span style="unicode-bidi: -webkit-isolate; unicode-bidi: isolate;">a!</span></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure-expected.txt (0 => 101556)
--- trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure-expected.txt 2011-11-30 22:33:20 UTC (rev 101556)
@@ -0,0 +1,2 @@
+This tests having an inline element with -webkit-isolate immediately following BR. WebKit should not hit an assertion.
+
Added: trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure.html (0 => 101556)
--- trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-nextlinebreak-failure.html 2011-11-30 22:33:20 UTC (rev 101556)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+This tests having an inline element with -webkit-isolate immediately following BR.
+WebKit should not hit an assertion.
+<br><span style="unicode-bidi:-webkit-isolate;"></span>
+<script>
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+</script>
Modified: trunk/Source/WebCore/ChangeLog (101555 => 101556)
--- trunk/Source/WebCore/ChangeLog 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/Source/WebCore/ChangeLog 2011-11-30 22:33:20 UTC (rev 101556)
@@ -1,3 +1,34 @@
+2011-11-29 Ryosuke Niwa <rn...@webkit.org>
+
+ Assertion failure (m_nestedIsolateCount >= 1) in BidiResolver::exitIsolate()
+ https://bugs.webkit.org/show_bug.cgi?id=69267
+
+ Reviewed by Eric Seidel.
+
+ The failure was caused by our updating bidi resolver's current position in layoutRunsAndFloatsInRange
+ without updating the number of nested isolated ancestors. Fixed the bug by computing the number of
+ isolated ancestors when setting a new position to the bidi resolver.
+
+ Also renamed the existing BidiResolver::setPosition to setPositionIgnoringNestedIsolates because this
+ version can be used only when we don't have to update the number of nested isolates.
+
+ Tests: fast/text/bidi-isolate-hang-with-neutral-expected.html
+ fast/text/bidi-isolate-hang-with-neutral.html
+ fast/text/bidi-isolate-nextlinebreak-failure.html
+
+ * platform/graphics/GraphicsContext.cpp:
+ (WebCore::GraphicsContext::drawBidiText):
+ * platform/text/BidiResolver.h:
+ (WebCore::BidiResolver::setPositionIgnoringNestedIsolates):
+ (WebCore::BidiResolver::setPosition):
+ * rendering/InlineIterator.h:
+ (WebCore::numberOfIsolateAncestors): Takes InlineIterator instead of object and root.
+ (WebCore::InlineBidiResolver::appendRun):
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::constructBidiRuns):
+ (WebCore::RenderBlock::layoutRunsAndFloatsInRange):
+ (WebCore::RenderBlock::determineStartPosition):
+
2011-11-30 Brent Fulgham <bfulg...@webkit.org>
[WinCairo] Correct SimpleFontData implementation to match Apple results.
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp (101555 => 101556)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2011-11-30 22:33:20 UTC (rev 101556)
@@ -400,7 +400,7 @@
BidiResolver<TextRunIterator, BidiCharacterRun> bidiResolver;
bidiResolver.setStatus(BidiStatus(run.direction(), run.directionalOverride()));
- bidiResolver.setPosition(TextRunIterator(&run, 0));
+ bidiResolver.setPositionIgnoringNestedIsolates(TextRunIterator(&run, 0));
// FIXME: This ownership should be reversed. We should pass BidiRunList
// to BidiResolver in createBidiRunsForLine.
Modified: trunk/Source/WebCore/platform/text/BidiResolver.h (101555 => 101556)
--- trunk/Source/WebCore/platform/text/BidiResolver.h 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/Source/WebCore/platform/text/BidiResolver.h 2011-11-30 22:33:20 UTC (rev 101556)
@@ -177,7 +177,12 @@
#endif
const Iterator& position() const { return m_current; }
- void setPosition(const Iterator& position) { m_current = position; }
+ void setPositionIgnoringNestedIsolates(const Iterator& position) { m_current = position; }
+ void setPosition(const Iterator& position, unsigned nestedIsolatedCount)
+ {
+ m_current = position;
+ m_nestedIsolateCount = nestedIsolatedCount;
+ }
void increment() { m_current.increment(); }
Modified: trunk/Source/WebCore/rendering/InlineIterator.h (101555 => 101556)
--- trunk/Source/WebCore/rendering/InlineIterator.h 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/Source/WebCore/rendering/InlineIterator.h 2011-11-30 22:33:20 UTC (rev 101556)
@@ -406,11 +406,13 @@
return 0;
}
-static inline unsigned numberOfIsolateAncestors(RenderObject* object, RenderObject* root)
+static inline unsigned numberOfIsolateAncestors(const InlineIterator& iter)
{
- ASSERT(object);
+ RenderObject* object = iter.object();
+ if (!object)
+ return 0;
unsigned count = 0;
- while (object && object != root) {
+ while (object && object != iter.root()) {
if (isIsolatedInline(object))
count++;
object = object->parent();
@@ -482,7 +484,7 @@
// Keep track of when we enter/leave "unicode-bidi: isolate" inlines.
// Initialize our state depending on if we're starting in the middle of such an inline.
// FIXME: Could this initialize from this->inIsolate() instead of walking up the render tree?
- IsolateTracker isolateTracker(numberOfIsolateAncestors(m_sor.m_obj, m_sor.root()));
+ IsolateTracker isolateTracker(numberOfIsolateAncestors(m_sor));
int start = m_sor.m_pos;
RenderObject* obj = m_sor.m_obj;
while (obj && obj != m_eor.m_obj && obj != endOfLine.m_obj) {
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (101555 => 101556)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2011-11-30 22:11:41 UTC (rev 101555)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2011-11-30 22:33:20 UTC (rev 101556)
@@ -966,7 +966,7 @@
RenderObject* startObj = bidiFirstSkippingEmptyInlines(isolatedSpan, &isolatedResolver);
if (!startObj)
continue;
- isolatedResolver.setPosition(InlineIterator(isolatedSpan, startObj, 0));
+ isolatedResolver.setPositionIgnoringNestedIsolates(InlineIterator(isolatedSpan, startObj, 0));
// FIXME: isolatedEnd should probably equal end or the last char in isolatedSpan.
InlineIterator isolatedEnd = endOfLine;
@@ -1216,7 +1216,7 @@
layoutState.lineInfo().setEmpty(true);
- InlineIterator oldEnd = end;
+ const InlineIterator oldEnd = end;
bool isNewUBAParagraph = layoutState.lineInfo().previousLineBrokeCleanly();
FloatingObject* lastFloatFromPreviousLine = (m_floatingObjects && !m_floatingObjects->set().isEmpty()) ? m_floatingObjects->set().last() : 0;
end = lineBreaker.nextLineBreak(resolver, layoutState.lineInfo(), lineBreakIteratorInfo, lastFloatFromPreviousLine, consecutiveHyphenatedLines);
@@ -1284,7 +1284,7 @@
lineBox->deleteLine(renderArena());
removeFloatingObjectsBelow(lastFloatFromPreviousLine, oldLogicalHeight);
setLogicalHeight(oldLogicalHeight + adjustment);
- resolver.setPosition(oldEnd);
+ resolver.setPositionIgnoringNestedIsolates(oldEnd);
end = oldEnd;
continue;
}
@@ -1324,7 +1324,7 @@
}
lineMidpointState.reset();
- resolver.setPosition(end);
+ resolver.setPosition(end, numberOfIsolateAncestors(end));
}
}
@@ -1639,7 +1639,8 @@
if (last) {
setLogicalHeight(last->lineBottomWithLeading());
- resolver.setPosition(InlineIterator(this, last->lineBreakObj(), last->lineBreakPos()));
+ InlineIterator iter = InlineIterator(this, last->lineBreakObj(), last->lineBreakPos());
+ resolver.setPosition(iter, numberOfIsolateAncestors(iter));
resolver.setStatus(last->lineBreakBidiStatus());
} else {
TextDirection direction = style()->direction();
@@ -1648,7 +1649,8 @@
determineParagraphDirection(direction, InlineIterator(this, bidiFirstIncludingEmptyInlines(this), 0));
}
resolver.setStatus(BidiStatus(direction, style()->unicodeBidi() == Override));
- resolver.setPosition(InlineIterator(this, bidiFirstSkippingEmptyInlines(this, &resolver), 0));
+ InlineIterator iter = InlineIterator(this, bidiFirstSkippingEmptyInlines(this, &resolver), 0);
+ resolver.setPosition(iter, numberOfIsolateAncestors(iter));
}
return curr;
}
