Title: [117658] trunk
- Revision
- 117658
- Author
- [email protected]
- Date
- 2012-05-18 16:47:08 -0700 (Fri, 18 May 2012)
Log Message
Assertion failure in BidiResolver::commitExplicitEmbedding() (!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty()) at wikipedia.org
https://bugs.webkit.org/show_bug.cgi?id=76574
Reviewed by Levi Weintraub.
Source/WebCore:
Consider this example:
<span style="unicode-bidi: embed"><span style="unicode-bidi: isolate">a</span></span>
Before this patch, we would ASSERT when computing the text runs, as we would have encountered
the "embed LTR" directive from the outer span, but not try to "commit" this embedding until
we encountered the first charater (an optimization to avoid creating unnecessary bidi embedding contexts).
The ASSERT we were hitting was to ensure that embedding directives inside an isolated span
did not bleed out and effect the surrounding text.
bidi "isolate" support uses a multi-pass Unicode Bidi Algorithm (UBA), which when encountering
"isolated" sections of text ignores them in the first pass, and then goes back and runs
a separate instance of the UBA on those isolated sections.
Once we scan into an "isolate" section (during an outer UBA application) we should not respect
any embedding directives inside that "isolate" section.
However, in the above example, our "don't commit embeddings until we need them" and
"assert we don't respect embeddings inside isolated spans" were conflicting.
The fix is to make sure we always commit any pending embedding directives *before*
we enter an isolate section.
Luckly there was no functional bug here as we were still respecting
the embedding directives we were belatedly committing. After this change we're commiting
those directives at a more appropriate time, thus avoiding the ASSERT.
Test: fast/text/bidi-isolate-embedding-crash.html
* platform/text/BidiResolver.h:
(WebCore::::commitExplicitEmbedding):
* rendering/InlineIterator.h:
(WebCore::notifyObserverEnteredObject):
(WebCore::IsolateTracker::commitExplicitEmbedding):
LayoutTests:
* fast/text/bidi-isolate-embedding-crash-expected.txt: Added.
* fast/text/bidi-isolate-embedding-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (117657 => 117658)
--- trunk/LayoutTests/ChangeLog 2012-05-18 23:44:05 UTC (rev 117657)
+++ trunk/LayoutTests/ChangeLog 2012-05-18 23:47:08 UTC (rev 117658)
@@ -1,3 +1,13 @@
+2012-05-18 Eric Seidel <[email protected]>
+
+ Assertion failure in BidiResolver::commitExplicitEmbedding() (!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty()) at wikipedia.org
+ https://bugs.webkit.org/show_bug.cgi?id=76574
+
+ Reviewed by Levi Weintraub.
+
+ * fast/text/bidi-isolate-embedding-crash-expected.txt: Added.
+ * fast/text/bidi-isolate-embedding-crash.html: Added.
+
2012-05-18 Levi Weintraub <[email protected]>
Unreviewed. Updating non-mac platform expectations after r117640.
Added: trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash-expected.txt (0 => 117658)
--- trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash-expected.txt 2012-05-18 23:47:08 UTC (rev 117658)
@@ -0,0 +1 @@
+PASS if did not crash.
Added: trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash.html (0 => 117658)
--- trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-embedding-crash.html 2012-05-18 23:47:08 UTC (rev 117658)
@@ -0,0 +1,12 @@
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+</script>
+<!-- In debug builds, this would ASSERT while trying to find the first bidi-participating
+object on the line. We'd scan, find the explicit LTR embed char implied by the
+outer span, add that to the explicit-embeding stack, then walk into the isolate
+and find our first bidi object on the line (the PASS... text node). But, right before
+returning that we'd try to commit the open embedings, and ASSERT, as the PASS is
+inside the isolate and is not affected by the outer embeddings.
+https://bugs.webkit.org/show_bug.cgi?id=76574 -->
+<span style="unicode-bidi: embed;"><bdi dir="rtl">PASS if did not crash.</bdi></span>
Modified: trunk/Source/WebCore/ChangeLog (117657 => 117658)
--- trunk/Source/WebCore/ChangeLog 2012-05-18 23:44:05 UTC (rev 117657)
+++ trunk/Source/WebCore/ChangeLog 2012-05-18 23:47:08 UTC (rev 117658)
@@ -1,3 +1,42 @@
+2012-05-18 Eric Seidel <[email protected]>
+
+ Assertion failure in BidiResolver::commitExplicitEmbedding() (!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty()) at wikipedia.org
+ https://bugs.webkit.org/show_bug.cgi?id=76574
+
+ Reviewed by Levi Weintraub.
+
+ Consider this example:
+ <span style="unicode-bidi: embed"><span style="unicode-bidi: isolate">a</span></span>
+ Before this patch, we would ASSERT when computing the text runs, as we would have encountered
+ the "embed LTR" directive from the outer span, but not try to "commit" this embedding until
+ we encountered the first charater (an optimization to avoid creating unnecessary bidi embedding contexts).
+ The ASSERT we were hitting was to ensure that embedding directives inside an isolated span
+ did not bleed out and effect the surrounding text.
+
+ bidi "isolate" support uses a multi-pass Unicode Bidi Algorithm (UBA), which when encountering
+ "isolated" sections of text ignores them in the first pass, and then goes back and runs
+ a separate instance of the UBA on those isolated sections.
+
+ Once we scan into an "isolate" section (during an outer UBA application) we should not respect
+ any embedding directives inside that "isolate" section.
+
+ However, in the above example, our "don't commit embeddings until we need them" and
+ "assert we don't respect embeddings inside isolated spans" were conflicting.
+ The fix is to make sure we always commit any pending embedding directives *before*
+ we enter an isolate section.
+
+ Luckly there was no functional bug here as we were still respecting
+ the embedding directives we were belatedly committing. After this change we're commiting
+ those directives at a more appropriate time, thus avoiding the ASSERT.
+
+ Test: fast/text/bidi-isolate-embedding-crash.html
+
+ * platform/text/BidiResolver.h:
+ (WebCore::::commitExplicitEmbedding):
+ * rendering/InlineIterator.h:
+ (WebCore::notifyObserverEnteredObject):
+ (WebCore::IsolateTracker::commitExplicitEmbedding):
+
2012-05-18 Pratik Solanki <[email protected]>
BitmapImage::BitmapImage(CGImageRef, ImageObserver*) needs to set m_sizeRespectingOrientation
Modified: trunk/Source/WebCore/platform/text/BidiResolver.h (117657 => 117658)
--- trunk/Source/WebCore/platform/text/BidiResolver.h 2012-05-18 23:44:05 UTC (rev 117657)
+++ trunk/Source/WebCore/platform/text/BidiResolver.h 2012-05-18 23:47:08 UTC (rev 117658)
@@ -402,9 +402,10 @@
template <class Iterator, class Run>
bool BidiResolver<Iterator, Run>::commitExplicitEmbedding()
{
- // This gets called from bidiFirst when setting up our start position.
- // FIXME: Re-enable this assert once https://bugs.webkit.org/show_bug.cgi?id=76574 is fixed.
- // ASSERT(!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty());
+ // When we're "inIsolate()" we're resolving the parent context which
+ // ignores (skips over) the isolated content, including embedding levels.
+ // We should never accrue embedding levels while skipping over isolated content.
+ ASSERT(!inIsolate() || m_currentExplicitEmbeddingSequence.isEmpty());
using namespace WTF::Unicode;
Modified: trunk/Source/WebCore/rendering/InlineIterator.h (117657 => 117658)
--- trunk/Source/WebCore/rendering/InlineIterator.h 2012-05-18 23:44:05 UTC (rev 117657)
+++ trunk/Source/WebCore/rendering/InlineIterator.h 2012-05-18 23:47:08 UTC (rev 117658)
@@ -132,8 +132,10 @@
return;
}
if (isIsolated(unicodeBidi)) {
+ // Make sure that explicit embeddings are committed before we enter the isolated content.
+ observer->commitExplicitEmbedding();
observer->enterIsolate();
- // Embedding/Override characters implied by dir= are handled when
+ // Embedding/Override characters implied by dir= will be handled when
// we process the isolated span, not when laying out the "parent" run.
return;
}
@@ -451,6 +453,7 @@
// We don't care if we encounter bidi directional overrides.
void embed(WTF::Unicode::Direction, BidiEmbeddingSource) { }
+ void commitExplicitEmbedding() { }
void addFakeRunIfNecessary(RenderObject* obj, unsigned pos, InlineBidiResolver& resolver)
{
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
