Merge r182051 - Inline continuation code should not take anonymous co…

…ntaining wrapper granted. https://bugs.webkit.org/show_bug.cgi?id=133312 Reviewed by Dave Hyatt. It's wrong to assume that when RenderInline is part of an inline continuation, its containing block is an anonymous wrapper and its sibling might be a block level renderer. When the inline continuation is no longer needed, for example when the block level renderer that initiated the continuation is detached from the render tree, the inline renderes still continue to form continuation.(however they no longer require anonymous wrappers) Source/WebCore: Test: fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html * rendering/RenderInline.cpp: (WebCore::updateStyleOfAnonymousBlockContinuations): (WebCore::RenderInline::styleDidChange): LayoutTests: * fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation-expected.txt: Added. * fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html: Added.
WebKit · Apr 6, 2015 · 1a5d2ac · 1a5d2ac
1 parent c799bcc
commit 1a5d2ac
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 17 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2015-03-26  Zalan Bujtas  <zalan@apple.com>
+
+        Inline continuation code should not take anonymous containing wrapper granted.
+        https://bugs.webkit.org/show_bug.cgi?id=133312
+
+        Reviewed by Dave Hyatt.
+
+        It's wrong to assume that when RenderInline is part of an inline continuation, its containing block
+        is an anonymous wrapper and its sibling might be a block level renderer.
+        When the inline continuation is no longer needed, for example when the block level renderer that initiated the continuation
+        is detached from the render tree, the inline renderes still continue to form continuation.(however they no longer require
+        anonymous wrappers)
+
+        * fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation-expected.txt: Added.
+        * fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html: Added.
+
 2015-03-24  Yoav Weiss  <yoav@yoav.ws>
 
         Stop image from displaying when src attribute is removed or emptied

diff --git a/...inline/crash-when-position-property-is-changed-and-no-longer-in-continuation-expected.txt b/...inline/crash-when-position-property-is-changed-and-no-longer-in-continuation-expected.txt
@@ -0,0 +1 @@
+PASS if no crash or assert in debug.
diff --git a/...ts/fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html b/...ts/fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>This tests that position property can be changed on a inline element once it is not part of an active continuation.</title>
+</head>
+<body>
+PASS if no crash or assert in debug.
+<div style="position: absolute">
+  <span id=foo>
+    <div id=removethis></div>
+  </span>
+</div>
+<span></span>
+<script>
+  if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+  }
+  setTimeout(function() { 
+    var blockToRemove = document.getElementById("removethis");
+    blockToRemove.parentNode.removeChild(blockToRemove);
+    document.getElementById("foo").style.position="relative";
+    if (window.testRunner)
+       testRunner.notifyDone(); 
+  }, 0);
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2015-03-26  Zalan Bujtas  <zalan@apple.com>
+
+        Inline continuation code should not take anonymous containing wrapper granted.
+        https://bugs.webkit.org/show_bug.cgi?id=133312
+
+        Reviewed by Dave Hyatt.
+
+        It's wrong to assume that when RenderInline is part of an inline continuation, its containing block
+        is an anonymous wrapper and its sibling might be a block level renderer.
+        When the inline continuation is no longer needed, for example when the block level renderer that initiated the continuation
+        is detached from the render tree, the inline renderes still continue to form continuation.(however they no longer require
+        anonymous wrappers)
+
+        Test: fast/inline/crash-when-position-property-is-changed-and-no-longer-in-continuation.html
+
+        * rendering/RenderInline.cpp:
+        (WebCore::updateStyleOfAnonymousBlockContinuations):
+        (WebCore::RenderInline::styleDidChange):
+
 2015-03-24  Yoav Weiss  <yoav@yoav.ws>
 
         Stop image from displaying when src attribute is removed or emptied

diff --git a/Source/WebCore/rendering/RenderInline.cpp b/Source/WebCore/rendering/RenderInline.cpp
@@ -138,9 +138,9 @@ static RenderElement* inFlowPositionedInlineAncestor(RenderElement* p)
     return 0;
 }
 
-static void updateStyleOfAnonymousBlockContinuations(RenderBox* box, const RenderStyle* newStyle, const RenderStyle* oldStyle)
+static void updateStyleOfAnonymousBlockContinuations(RenderBlock& block, const RenderStyle* newStyle, const RenderStyle* oldStyle)
 {
-    for (;box && box->isAnonymousBlock(); box = box->nextSiblingBox()) {
+    for (RenderBox* box = &block; box && box->isAnonymousBlock(); box = box->nextSiblingBox()) {
         if (box->style().position() == newStyle->position())
             continue;
 
@@ -174,21 +174,23 @@ void RenderInline::styleDidChange(StyleDifference diff, const RenderStyle* oldSt
     // need to pass its style on to anyone else.
     RenderStyle& newStyle = style();
     RenderInline* continuation = inlineElementContinuation();
-    for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation()) {
-        RenderBoxModelObject* nextCont = currCont->continuation();
-        currCont->setContinuation(nullptr);
-        currCont->setStyle(newStyle);
-        currCont->setContinuation(nextCont);
-    }
-
-    // If an inline's in-flow positioning has changed then any descendant blocks will need to change their in-flow positioning accordingly.
-    // Do this by updating the position of the descendant blocks' containing anonymous blocks - there may be more than one.
-    if (continuation && oldStyle && newStyle.position() != oldStyle->position()
-        && (newStyle.hasInFlowPosition() || oldStyle->hasInFlowPosition())) {
-        // If any descendant blocks exist then they will be in the next anonymous block and its siblings.
-        RenderObject* block = containingBlock()->nextSibling();
-        ASSERT(block && block->isAnonymousBlock());
-        updateStyleOfAnonymousBlockContinuations(downcast<RenderBlock>(block), &newStyle, oldStyle);
+    if (continuation) {
+        for (RenderInline* currCont = continuation; currCont; currCont = currCont->inlineElementContinuation()) {
+            RenderBoxModelObject* nextCont = currCont->continuation();
+            currCont->setContinuation(nullptr);
+            currCont->setStyle(newStyle);
+            currCont->setContinuation(nextCont);
+        }
+        // If an inline's in-flow positioning has changed and it is part of an active continuation as a descendant of an anonymous containing block,
+        // then any descendant blocks will need to change their in-flow positioning accordingly.
+        // Do this by updating the position of the descendant blocks' containing anonymous blocks - there may be more than one.
+        if (containingBlock()->isAnonymousBlock() && oldStyle && newStyle.position() != oldStyle->position() && (newStyle.hasInFlowPosition() || oldStyle->hasInFlowPosition())) {
+            // If any descendant blocks exist then they will be in the next anonymous block and its siblings.
+            ASSERT(containingBlock()->nextSibling());
+            RenderBlock& block = downcast<RenderBlock>(*containingBlock()->nextSibling());
+            ASSERT(block.isAnonymousBlock());
+            updateStyleOfAnonymousBlockContinuations(block, &newStyle, oldStyle);
+        }
     }
 
     if (!alwaysCreateLineBoxes()) {