Changeset 116693 in webkit

Timestamp:

May 10, 2012 3:08:27 PM (12 years ago)

Author:

jchaffraix@webkit.org

Message:

Crash in computedCSSPadding* functions due to RenderImage::imageDimensionsChanged called during attachment
​https://bugs.webkit.org/show_bug.cgi?id=85912

Reviewed by Eric Seidel.

Source/WebCore:

Tests: fast/images/link-body-content-imageDimensionChanged-crash.html

fast/images/script-counter-imageDimensionChanged-crash.html

The bug comes from CSS generated images that could end up calling imageDimensionsChanged during attachment. As the
rest of the code (e.g. computedCSSPadding*) would assumes that we are already inserted in the tree, we would crash.

The solution is to bail out in this case as newly inserted RenderObject will trigger layout later on and properly
handle what we would be doing as part of imageDimensionChanged (the only exception being updating our intrinsic
size which should be done as part of imageDimensionsChanged).

• rendering/RenderImage.cpp:

(WebCore::RenderImage::imageDimensionsChanged):

LayoutTests:

• fast/images/link-body-content-imageDimensionChanged-crash-expected.txt: Added.
• fast/images/link-body-content-imageDimensionChanged-crash.html: Added.
• fast/images/script-counter-imageDimensionChanged-crash-expected.txt: Added.
• fast/images/script-counter-imageDimensionChanged-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/images/link-body-content-imageDimensionChanged-crash-expected.txt (added)
• LayoutTests/fast/images/link-body-content-imageDimensionChanged-crash.html (added)
• LayoutTests/fast/images/script-counter-imageDimensionChanged-crash-expected.txt (added)
• LayoutTests/fast/images/script-counter-imageDimensionChanged-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderImage.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-05-10  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Crash in computedCSSPadding* functions due to RenderImage::imageDimensionsChanged called during attachment
+        https://bugs.webkit.org/show_bug.cgi?id=85912
+
+        Reviewed by Eric Seidel.
+
+        * fast/images/link-body-content-imageDimensionChanged-crash-expected.txt: Added.
+        * fast/images/link-body-content-imageDimensionChanged-crash.html: Added.
+        * fast/images/script-counter-imageDimensionChanged-crash-expected.txt: Added.
+        * fast/images/script-counter-imageDimensionChanged-crash.html: Added.
+
 2012-05-10  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-10  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Crash in computedCSSPadding* functions due to RenderImage::imageDimensionsChanged called during attachment
+        https://bugs.webkit.org/show_bug.cgi?id=85912
+
+        Reviewed by Eric Seidel.
+
+        Tests: fast/images/link-body-content-imageDimensionChanged-crash.html
+               fast/images/script-counter-imageDimensionChanged-crash.html
+
+        The bug comes from CSS generated images that could end up calling imageDimensionsChanged during attachment. As the
+        rest of the code (e.g. computedCSSPadding*) would assumes that we are already inserted in the tree, we would crash.
+
+        The solution is to bail out in this case as newly inserted RenderObject will trigger layout later on and properly
+        handle what we would be doing as part of imageDimensionChanged (the only exception being updating our intrinsic
+        size which should be done as part of imageDimensionsChanged).
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::imageDimensionsChanged):
+
 2012-05-10  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -189 +189 @@
 void RenderImage::imageDimensionsChanged(bool imageSizeChanged, const IntRect* rect)
 {
+    bool intrinsicSizeChanged = updateIntrinsicSizeIfNeeded(m_imageResource->imageSize(style()->effectiveZoom()), imageSizeChanged);
+
+    // In the case of generated image content using :before/:after/content, we might not be
+    // in the render tree yet. In that case, we just need to update our intrinsic size.
+    // layout() will be called after we are inserted in the tree which will take care of
+    // what we are doing here.
+    if (!containingBlock())
+        return;
+
     bool shouldRepaint = true;
-    if (updateIntrinsicSizeIfNeeded(m_imageResource->imageSize(style()->effectiveZoom()), imageSizeChanged)) {
-        // In the case of generated image content using :before/:after, we might not be in the
-        // render tree yet.  In that case, we don't need to worry about check for layout, since we'll get a
-        // layout when we get added in to the render tree hierarchy later.
-        if (containingBlock()) {
-            // lets see if we need to relayout at all..
-            int oldwidth = width();
-            int oldheight = height();
-            if (!preferredLogicalWidthsDirty())
-                setPreferredLogicalWidthsDirty(true);
-            computeLogicalWidth();
-            computeLogicalHeight();
-
-            if (imageSizeChanged || width() != oldwidth || height() != oldheight) {
-                shouldRepaint = false;
-                if (!selfNeedsLayout())
-                    setNeedsLayout(true);
-            }
-
-            setWidth(oldwidth);
-            setHeight(oldheight);
-        }
+    if (intrinsicSizeChanged) {
+        // lets see if we need to relayout at all..
+        int oldwidth = width();
+        int oldheight = height();
+        if (!preferredLogicalWidthsDirty())
+            setPreferredLogicalWidthsDirty(true);
+        computeLogicalWidth();
+        computeLogicalHeight();
+
+        if (imageSizeChanged || width() != oldwidth || height() != oldheight) {
+            shouldRepaint = false;
+            if (!selfNeedsLayout())
+                setNeedsLayout(true);
+        }
+
+        setWidth(oldwidth);
+        setHeight(oldheight);
     }