Title: [118420] trunk
Revision
118420
Author
[email protected]
Date
2012-05-24 14:47:47 -0700 (Thu, 24 May 2012)

Log Message

Avoid creating InlineBoxes for floating and positioned objects in isolates.
https://bugs.webkit.org/show_bug.cgi?id=87277

Reviewed by Eric Seidel.

Source/WebCore:

We currently will create a placeholder run for the first object we encounter inside an isolate. Then
in RenderBlockLineLayout's constructBidiRuns, we replace that run with the contents of the Isolate.
We run into problems when there are no valid contents in the Isolate. We can't simply remove the
placeholder if there's nothing to replace it with since it may be the logically last run, which we
track but can't rebuild by the time we're handling isolates (we've already shuffled the BidiRuns around).

With this change, we avoid creating a placeholder altogether until we hit contents in the isolate
that would warrant a BidiRun in the first place.

Test: fast/text/international/float-as-only-child-of-isolate-crash.html

* rendering/InlineIterator.h:
(WebCore::IsolateTracker::addFakeRunIfNecessary):
* rendering/RenderBlock.h:
(RenderBlock):
(WebCore::RenderBlock::shouldSkipCreatingRunsForObject):
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::appendRunsForObject):

LayoutTests:

* fast/text/international/float-as-only-child-of-isolate-crash-expected.txt: Added.
* fast/text/international/float-as-only-child-of-isolate-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (118419 => 118420)


--- trunk/LayoutTests/ChangeLog	2012-05-24 21:44:36 UTC (rev 118419)
+++ trunk/LayoutTests/ChangeLog	2012-05-24 21:47:47 UTC (rev 118420)
@@ -1,3 +1,13 @@
+2012-05-24  Levi Weintraub  <[email protected]>
+
+        Avoid creating InlineBoxes for floating and positioned objects in isolates.
+        https://bugs.webkit.org/show_bug.cgi?id=87277
+
+        Reviewed by Eric Seidel.
+
+        * fast/text/international/float-as-only-child-of-isolate-crash-expected.txt: Added.
+        * fast/text/international/float-as-only-child-of-isolate-crash.html: Added.
+
 2012-05-24  Jessie Berlin  <[email protected]>
 
         DOM ClipboardData.setData does not allow writes during clipboard events

Added: trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash-expected.txt (0 => 118420)


--- trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash-expected.txt	2012-05-24 21:47:47 UTC (rev 118420)
@@ -0,0 +1 @@
+This tests that we avoid trying to lay out floats inside an inline with unicode-bidi: isolate. Test passes if we don't crash.

Added: trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash.html (0 => 118420)


--- trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/international/float-as-only-child-of-isolate-crash.html	2012-05-24 21:47:47 UTC (rev 118420)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<body>
+This tests that we avoid trying to lay out floats inside an inline with unicode-bidi: isolate. Test passes if we don't crash.
+<bdi><div style='float: left; '></div></bdi>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+</body>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (118419 => 118420)


--- trunk/Source/WebCore/ChangeLog	2012-05-24 21:44:36 UTC (rev 118419)
+++ trunk/Source/WebCore/ChangeLog	2012-05-24 21:47:47 UTC (rev 118420)
@@ -1,3 +1,29 @@
+2012-05-24  Levi Weintraub  <[email protected]>
+
+        Avoid creating InlineBoxes for floating and positioned objects in isolates.
+        https://bugs.webkit.org/show_bug.cgi?id=87277
+
+        Reviewed by Eric Seidel.
+
+        We currently will create a placeholder run for the first object we encounter inside an isolate. Then
+        in RenderBlockLineLayout's constructBidiRuns, we replace that run with the contents of the Isolate.
+        We run into problems when there are no valid contents in the Isolate. We can't simply remove the
+        placeholder if there's nothing to replace it with since it may be the logically last run, which we
+        track but can't rebuild by the time we're handling isolates (we've already shuffled the BidiRuns around).
+
+        With this change, we avoid creating a placeholder altogether until we hit contents in the isolate
+        that would warrant a BidiRun in the first place.
+
+        Test: fast/text/international/float-as-only-child-of-isolate-crash.html
+
+        * rendering/InlineIterator.h:
+        (WebCore::IsolateTracker::addFakeRunIfNecessary):
+        * rendering/RenderBlock.h:
+        (RenderBlock):
+        (WebCore::RenderBlock::shouldSkipCreatingRunsForObject):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlock::appendRunsForObject):
+
 2012-05-24  Ryosuke Niwa  <[email protected]>
 
         There are too many poorly named functions to create a fragment from markup

Modified: trunk/Source/WebCore/rendering/InlineIterator.h (118419 => 118420)


--- trunk/Source/WebCore/rendering/InlineIterator.h	2012-05-24 21:44:36 UTC (rev 118419)
+++ trunk/Source/WebCore/rendering/InlineIterator.h	2012-05-24 21:47:47 UTC (rev 118420)
@@ -459,7 +459,8 @@
     {
         // We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.
         // We'll be called for every span inside the isolated span so we just ignore subsequent calls.
-        if (m_haveAddedFakeRunForRootIsolate)
+        // We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.
+        if (m_haveAddedFakeRunForRootIsolate || RenderBlock::shouldSkipCreatingRunsForObject(obj))
             return;
         m_haveAddedFakeRunForRootIsolate = true;
         // obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (118419 => 118420)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2012-05-24 21:44:36 UTC (rev 118419)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2012-05-24 21:47:47 UTC (rev 118420)
@@ -242,6 +242,11 @@
     RenderBlock* createAnonymousColumnSpanBlock() const { return createAnonymousColumnSpanWithParentRenderer(this); }
 
     virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const OVERRIDE;
+
+    static bool shouldSkipCreatingRunsForObject(RenderObject* obj)
+    {
+        return obj->isFloating() || (obj->isPositioned() && !obj->style()->isOriginalDisplayInlineType() && !obj->container()->isRenderInline());
+    }
     
     static void appendRunsForObject(BidiRunList<BidiRun>&, int start, int end, RenderObject*, InlineBidiResolver&);
 

Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (118419 => 118420)


--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2012-05-24 21:44:36 UTC (rev 118419)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2012-05-24 21:47:47 UTC (rev 118420)
@@ -315,8 +315,7 @@
 
 void RenderBlock::appendRunsForObject(BidiRunList<BidiRun>& runs, int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
 {
-    if (start > end || obj->isFloating() ||
-        (obj->isPositioned() && !obj->style()->isOriginalDisplayInlineType() && !obj->container()->isRenderInline()))
+    if (start > end || shouldSkipCreatingRunsForObject(obj))
         return;
 
     LineMidpointState& lineMidpointState = resolver.midpointState();
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to