Changeset 82144 in webkit

Timestamp:

Mar 28, 2011 12:44:19 PM (13 years ago)

Author:

hyatt@apple.com

Message:

​https://bugs.webkit.org/show_bug.cgi?id=57221, memory corruption/crashes when positioned objects
occur at the end of a line.

Reviewed by Simon Fraser and Darin Adler.

The old code and new code for dealing with a trailing space object midpoint manipulated a raw
array instead of the Vector. Otherwise this corruption would have been caught prior to check-in.

I have patched the code to only go through the Vector and to make it handle the case that led to
the corruption. Trailing positioned objects can occur both prior to and following the trailing space
object's midpoint, so we have to be prepared to deal with both cases.

This is already tested by fast/block/positioning/052.html, and that test now properly progresses
like the other positioning tests did.

Source/WebCore:

• rendering/RenderBlockLineLayout.cpp:

(WebCore::RenderBlock::findNextLineBreak):

LayoutTests:

• platform/mac/fast/block/positioning/052-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/fast/block/positioning/052-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-03-28  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=57221, memory corruption/crashes when positioned objects
+        occur at the end of a line.
+        
+        The old code and new code for dealing with a trailing space object midpoint manipulated a raw
+        array instead of the Vector. Otherwise this corruption would have been caught prior to check-in.
+        
+        I have patched the code to only go through the Vector and to make it handle the case that led to
+        the corruption. Trailing positioned objects can occur both prior to and following the trailing space
+        object's midpoint, so we have to be prepared to deal with both cases.
+        
+        This is already tested by fast/block/positioning/052.html, and that test now properly progresses
+        like the other positioning tests did.
+
+        * platform/mac/fast/block/positioning/052-expected.txt:
+
 2011-03-28  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/LayoutTests/platform/mac/fast/block/positioning/052-expected.txt

@@ -6 +6 @@
       RenderBlock {DIV} at (0,0) size 784x18
         RenderText {#text} at (0,0) size 0x0
-layer at (108,8) size 39x18
-  RenderInline (relative positioned) {SPAN} at (0,0) size 39x18
-    RenderText {#text} at (0,0) size 39x18
-      text run at (0,0) width 39: "Hello "
+layer at (108,8) size 35x18
+  RenderInline (relative positioned) {SPAN} at (0,0) size 35x18
+    RenderText {#text} at (0,0) size 35x18
+      text run at (0,0) width 35: "Hello"
 layer at (158,58) size 35x18
   RenderBlock (positioned) {DIV} at (50,50) size 35x18 [color=#FFFFFF] [bgcolor=#008000]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-03-28  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Simon Fraser and Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=57221, memory corruption/crashes when positioned objects
+        occur at the end of a line.
+        
+        The old code and new code for dealing with a trailing space object midpoint manipulated a raw
+        array instead of the Vector. Otherwise this corruption would have been caught prior to check-in.
+        
+        I have patched the code to only go through the Vector and to make it handle the case that led to
+        the corruption. Trailing positioned objects can occur both prior to and following the trailing space
+        object's midpoint, so we have to be prepared to deal with both cases.
+        
+        This is already tested by fast/block/positioning/052.html, and that test now properly progresses
+        like the other positioning tests did.
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlock::findNextLineBreak):
+
 2011-03-28  Andrei Popescu  <andreip@google.com>
 

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -1639 +1639 @@
                 // then start ignoring spaces again.
                 if (isInlineType || o->container()->isRenderInline()) {
-                    ignoreStart.obj = o;
-                    ignoreStart.pos = 0;
                     if (ignoringSpaces) {
+                        ignoreStart.obj = o;
+                        ignoreStart.pos = 0;
                         addMidpoint(lineMidpointState, ignoreStart); // Stop ignoring spaces.
                         addMidpoint(lineMidpointState, ignoreStart); // Start ignoring again.
@@ -2118 +2118 @@
         // exclude the space, allowing it to - in effect - collapse into the newline.
         if (lineMidpointState.numMidpoints % 2) {
-            InlineIterator* midpoints = lineMidpointState.midpoints.data();
-            midpoints[lineMidpointState.numMidpoints - trailingPositionedBoxes.size() * 2 - 1].pos--;
+            // Find the trailing space object's midpoint.
+            int trailingSpaceMidpoint = lineMidpointState.numMidpoints - 1;
+            for ( ; trailingSpaceMidpoint >= 0 && lineMidpointState.midpoints[trailingSpaceMidpoint].obj != trailingSpaceObject; --trailingSpaceMidpoint) { }
+            ASSERT(trailingSpaceMidpoint >= 0);
+            lineMidpointState.midpoints[trailingSpaceMidpoint].pos--;
+
+            // Now make sure every single trailingPositionedBox following the trailingSpaceMidpoint properly stops and starts 
+            // ignoring spaces.
+            size_t currentMidpoint = trailingSpaceMidpoint + 1;
+            for (size_t i = 0; i < trailingPositionedBoxes.size(); ++i) {
+                if (currentMidpoint >= lineMidpointState.numMidpoints) {
+                    // We don't have a midpoint for this box yet.
+                    InlineIterator ignoreStart(this, trailingPositionedBoxes[i], 0);
+                    addMidpoint(lineMidpointState, ignoreStart); // Stop ignoring.
+                    addMidpoint(lineMidpointState, ignoreStart); // Start ignoring again.
+                } else {
+                    ASSERT(lineMidpointState.midpoints[currentMidpoint].obj == trailingPositionedBoxes[i]);
+                    ASSERT(lineMidpointState.midpoints[currentMidpoint + 1].obj == trailingPositionedBoxes[i]);
+                }
+                currentMidpoint += 2;
+            }
         } else if (!lBreak.obj && trailingSpaceObject->isText()) {
             // Add a new end midpoint that stops right at the very end.