CSSRegions: Crash when using style in region for removed element.
authormihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Aug 2012 15:57:25 +0000 (15:57 +0000)
committermihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 12 Aug 2012 15:57:25 +0000 (15:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93276

Reviewed by Abhishek Arya.

Source/WebCore:

When a RenderInline object from within a render flow thread is split, the cloned
hierarchy built during the split process does not have the inRenderFlowThread bit
set properly. If the cloned hierarchy is flowed into a region with region style rules,
we compute the style in region also for objects that do not have inRenderFlowThread bit
set and we store the computed style in region for caching purposes. But we only remove
an object style in region information if that object has the inRenderFlowThread bit set.
Under these circumstances, it is possible to remove a object with cached style in region
and without inRenderFlowThread bit set from the render tree and leave the associated cached
information un-removed. Such information will be accesses during the next paint phase of
the region, thus resulting a crash.

The fix is to modify RenderBlock::clone() and RenderInline::clone() functions to also copy the inRenderFlowThread bit
from the source into the clone, therefore the cloned hierarchies will have the inRenderFlowThread
bit set properly.

Test: fast/regions/removed-element-style-in-region-crash.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::clone):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::clone): Replace former static RenderInline::cloneInline with member RenderInline::clone.
(WebCore::RenderInline::splitInlines):
* rendering/RenderInline.h:
(RenderInline):
* rendering/RenderRegion.cpp:
(WebCore::RenderRegion::setObjectStyleInRegion):
Added an assert to make sure that when we are computing style in region, we are doing for objects
with inRenderFlowThread set. Also, bail out early in this case to prevent further crashes.

LayoutTests:

Added test reproducing the problem.

* fast/regions/removed-element-style-in-region-crash-expected.txt: Added.
* fast/regions/removed-element-style-in-region-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@125376 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/regions/removed-element-style-in-region-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/regions/removed-element-style-in-region-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderInline.cpp
Source/WebCore/rendering/RenderInline.h
Source/WebCore/rendering/RenderRegion.cpp

index 739494c..3d67ce7 100644 (file)
@@ -1,3 +1,15 @@
+2012-08-12  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        CSSRegions: Crash when using style in region for removed element.
+        https://bugs.webkit.org/show_bug.cgi?id=93276
+
+        Reviewed by Abhishek Arya.
+
+        Added test reproducing the problem.
+
+        * fast/regions/removed-element-style-in-region-crash-expected.txt: Added.
+        * fast/regions/removed-element-style-in-region-crash.html: Added.
+
 2012-08-12  Christophe Dumez  <christophe.dumez@intel.com>
 
         [EFL][WK2] Skip webintents/web-intents-obj-constructor.html test
diff --git a/LayoutTests/fast/regions/removed-element-style-in-region-crash-expected.txt b/LayoutTests/fast/regions/removed-element-style-in-region-crash-expected.txt
new file mode 100644 (file)
index 0000000..033c5c4
--- /dev/null
@@ -0,0 +1,7 @@
+Test for WebKit Bug 93276 Crash when using style in region for removed element.
+
+The test passes if it does not crash or assert.
+
+PASS
+
+
diff --git a/LayoutTests/fast/regions/removed-element-style-in-region-crash.html b/LayoutTests/fast/regions/removed-element-style-in-region-crash.html
new file mode 100644 (file)
index 0000000..3435d4e
--- /dev/null
@@ -0,0 +1,26 @@
+<!doctype html>
+<html>
+    <head>
+        <style>
+            #article1 { -webkit-flow-into: flow1; }
+            #region1  { -webkit-flow-from: flow1; }
+            @-webkit-region #region1 { div { background-color: green; } }
+        </style>
+    </head>
+    <body>
+        <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=93276">WebKit Bug 93276</a> Crash when using style in region for removed element.</p>
+        <p>The test passes if it does not crash or assert.</p>
+        <p>PASS</p>
+        <div id="article1">
+            <span><span id="span1"><div></div></span></span>
+        </div>
+        <div id="region1"><div>
+        <script>
+            if (window.testRunner)
+                window.testRunner.dumpAsText();
+            document.body.offsetTop;
+            child = document.getElementById("span1");
+            child.parentNode.removeChild(child);
+        </script>
+    </body>
+</html>
index 72f59ec..cb92456 100644 (file)
@@ -1,3 +1,39 @@
+2012-08-12  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        CSSRegions: Crash when using style in region for removed element.
+        https://bugs.webkit.org/show_bug.cgi?id=93276
+
+        Reviewed by Abhishek Arya.
+
+        When a RenderInline object from within a render flow thread is split, the cloned
+        hierarchy built during the split process does not have the inRenderFlowThread bit
+        set properly. If the cloned hierarchy is flowed into a region with region style rules,
+        we compute the style in region also for objects that do not have inRenderFlowThread bit
+        set and we store the computed style in region for caching purposes. But we only remove
+        an object style in region information if that object has the inRenderFlowThread bit set.
+        Under these circumstances, it is possible to remove a object with cached style in region
+        and without inRenderFlowThread bit set from the render tree and leave the associated cached
+        information un-removed. Such information will be accesses during the next paint phase of
+        the region, thus resulting a crash.
+
+        The fix is to modify RenderBlock::clone() and RenderInline::clone() functions to also copy the inRenderFlowThread bit
+        from the source into the clone, therefore the cloned hierarchies will have the inRenderFlowThread
+        bit set properly.
+
+        Test: fast/regions/removed-element-style-in-region-crash.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::clone):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::clone): Replace former static RenderInline::cloneInline with member RenderInline::clone.
+        (WebCore::RenderInline::splitInlines):
+        * rendering/RenderInline.h:
+        (RenderInline):
+        * rendering/RenderRegion.cpp:
+        (WebCore::RenderRegion::setObjectStyleInRegion):
+        Added an assert to make sure that when we are computing style in region, we are doing for objects
+        with inRenderFlowThread set. Also, bail out early in this case to prevent further crashes.
+
 2012-08-12  Huang Dongsung  <luxtella@company100.net>
 
         Set the access qualifier of two methods to query frame specific info of BitmapImage to protected.
index c250906..918421b 100755 (executable)
@@ -554,6 +554,7 @@ RenderBlock* RenderBlock::clone() const
         // generated content added yet.
         cloneBlock->setChildrenInline(cloneBlock->firstChild() ? cloneBlock->firstChild()->isInline() : childrenInline());
     }
+    cloneBlock->setInRenderFlowThread(inRenderFlowThread());
     return cloneBlock;
 }
 
index da120be..1166c8c 100644 (file)
@@ -338,11 +338,12 @@ void RenderInline::addChildIgnoringContinuation(RenderObject* newChild, RenderOb
     newChild->setNeedsLayoutAndPrefWidthsRecalc();
 }
 
-RenderInline* RenderInline::cloneInline(RenderInline* src)
+RenderInline* RenderInline::clone() const
 {
-    RenderInline* o = new (src->renderArena()) RenderInline(src->node());
-    o->setStyle(src->style());
-    return o;
+    RenderInline* cloneInline = new (renderArena()) RenderInline(node());
+    cloneInline->setStyle(style());
+    cloneInline->setInRenderFlowThread(inRenderFlowThread());
+    return cloneInline;
 }
 
 void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
@@ -350,8 +351,8 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
                                 RenderObject* beforeChild, RenderBoxModelObject* oldCont)
 {
     // Create a clone of this inline.
-    RenderInline* clone = cloneInline(this);
-    clone->setContinuation(oldCont);
+    RenderInline* cloneInline = clone();
+    cloneInline->setContinuation(oldCont);
 
     // Now take all of the children from beforeChild to the end and remove
     // them from |this| and place them in the clone.
@@ -359,12 +360,12 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
     while (o) {
         RenderObject* tmp = o;
         o = tmp->nextSibling();
-        clone->addChildIgnoringContinuation(children()->removeChildNode(this, tmp), 0);
+        cloneInline->addChildIgnoringContinuation(children()->removeChildNode(this, tmp), 0);
         tmp->setNeedsLayoutAndPrefWidthsRecalc();
     }
 
     // Hook |clone| up as the continuation of the middle block.
-    middleBlock->setContinuation(clone);
+    middleBlock->setContinuation(cloneInline);
 
     // We have been reparented and are now under the fromBlock.  We need
     // to walk up our inline parent chain until we hit the containing block.
@@ -382,17 +383,17 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
         ASSERT(curr->isRenderInline());
         if (splitDepth < cMaxSplitDepth) {
             // Create a new clone.
-            RenderInline* cloneChild = clone;
-            clone = cloneInline(toRenderInline(curr));
+            RenderInline* cloneChild = cloneInline;
+            cloneInline = toRenderInline(curr)->clone();
 
             // Insert our child clone as the first child.
-            clone->addChildIgnoringContinuation(cloneChild, 0);
+            cloneInline->addChildIgnoringContinuation(cloneChild, 0);
 
             // Hook the clone up as a continuation of |curr|.
             RenderInline* inlineCurr = toRenderInline(curr);
             oldCont = inlineCurr->continuation();
-            inlineCurr->setContinuation(clone);
-            clone->setContinuation(oldCont);
+            inlineCurr->setContinuation(cloneInline);
+            cloneInline->setContinuation(oldCont);
 
             // Someone may have indirectly caused a <q> to split.  When this happens, the :after content
             // has to move into the inline continuation.  Call updateBeforeAfterContent to ensure that the inline's :after
@@ -406,7 +407,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
             while (o) {
                 RenderObject* tmp = o;
                 o = tmp->nextSibling();
-                clone->addChildIgnoringContinuation(inlineCurr->children()->removeChildNode(curr, tmp), 0);
+                cloneInline->addChildIgnoringContinuation(inlineCurr->children()->removeChildNode(curr, tmp), 0);
                 tmp->setNeedsLayoutAndPrefWidthsRecalc();
             }
         }
@@ -418,7 +419,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
     }
 
     // Now we are at the block level. We need to put the clone into the toBlock.
-    toBlock->children()->appendChildNode(toBlock, clone);
+    toBlock->children()->appendChildNode(toBlock, cloneInline);
 
     // Now take all the children after currChild and remove them from the fromBlock
     // and put them in the toBlock.
index e207caf..fdd65db 100644 (file)
@@ -165,7 +165,7 @@ private:
     
     virtual void updateBoxModelInfoFromStyle();
     
-    static RenderInline* cloneInline(RenderInline* src);
+    RenderInline* clone() const;
 
     void paintOutlineForLine(GraphicsContext*, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine,
                              const LayoutRect& nextLine, const Color);
index aa4e050..0682ac9 100644 (file)
@@ -399,6 +399,10 @@ void RenderRegion::computeChildrenStyleInRegion(const RenderObject* object)
  
 void RenderRegion::setObjectStyleInRegion(RenderObject* object, PassRefPtr<RenderStyle> styleInRegion, bool objectRegionStyleCached)
 {
+    ASSERT(object->inRenderFlowThread());
+    if (!object->inRenderFlowThread())
+        return;
+
     RefPtr<RenderStyle> objectOriginalStyle = object->style();
     object->setStyleInternal(styleInRegion);