Title: [117007] trunk
Revision
117007
Author
[email protected]
Date
2012-05-14 15:36:03 -0700 (Mon, 14 May 2012)

Log Message

Crash in WebCore::RenderObject::repaint
https://bugs.webkit.org/show_bug.cgi?id=86162

Patch by Takashi Sakamoto <[email protected]> on 2012-05-14
Reviewed by Abhishek Arya.

Source/WebCore:

As RenderScrollbarPart has no parent renderer, we crash in
WebCore::RenderBoxModelObject::paddingLeft when paddingLeft has
percent value, e.g. 5%. However if we set the scrollbar's parent
renderer to a renderer owning the scrollbar by using setParent method,
RenderScrollbarPart::styleWillChange will invoke parent renderer's
repaint. This causes crash in WebCore::RenderObject::repaint if the
owning renderer is already destroyed.
To fix the first crash without the second crash, modify
RenderObject::containingBlock() to check isRenderScrollbarPart or not,
if parent() is 0.
If so, use scrollbar's owningRenderer from RenderScrollbarPart.

Test: scrollbars/scrollbar-percent-padding-crash.html
      scrollbars/scrollbar-scrollbarparts-repaint-crash.html

* rendering/RenderObject.cpp:
(WebCore::RenderObject::containingBlock):
Modifying containingBlock. If parent() is 0 and isRenderScrollbarPart()
is true, use RenderScrollbarPart's m_scrollbar->owningRenderer()
instead of parent().
* rendering/RenderObject.h:
(WebCore::RenderObject::isRenderScrollbarPart):
(RenderObject):
Adding a new method, isRenderScrollbarPart.
* rendering/RenderScrollbarPart.cpp:
(WebCore::RenderScrollbarPart::rendererOwningScrollbar):
(WebCore):
Adding a new method, scrollbarOwningRenderer to obtain m_scrollar's
owningRenderer.
* rendering/RenderScrollbarPart.h:
(RenderScrollbarPart):
Removing "friend class RenderScrollbar".
(WebCore::RenderScrollbarPart::isRenderScrollbarPart):
(WebCore::toRenderScrollbarPart):
(WebCore):
Implementing isRenderScrollbarPart and toRenderScrollbarPart.

LayoutTests:

* scrollbars/scrollbar-scrollbarparts-repaint-crash-expected.txt: Added.
* scrollbars/scrollbar-scrollbarparts-repaint-crash.html: Added.
* scrollbars/scrollbar-percent-padding-crash-expected.txt: Added.
* scrollbars/scrollbar-percent-padding-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (117006 => 117007)


--- trunk/LayoutTests/ChangeLog	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/LayoutTests/ChangeLog	2012-05-14 22:36:03 UTC (rev 117007)
@@ -1,3 +1,15 @@
+2012-05-14  Takashi Sakamoto  <[email protected]>
+
+        Crash in WebCore::RenderObject::repaint
+        https://bugs.webkit.org/show_bug.cgi?id=86162
+
+        Reviewed by Abhishek Arya.
+
+        * scrollbars/scrollbar-scrollbarparts-repaint-crash-expected.txt: Added.
+        * scrollbars/scrollbar-scrollbarparts-repaint-crash.html: Added.
+        * scrollbars/scrollbar-percent-padding-crash-expected.txt: Added.
+        * scrollbars/scrollbar-percent-padding-crash.html: Added.
+
 2012-05-14  Mike West  <[email protected]>
 
         Content Security Policy console errors include violated directive.

Added: trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash-expected.txt (0 => 117007)


--- trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash-expected.txt	2012-05-14 22:36:03 UTC (rev 117007)
@@ -0,0 +1,3 @@
+Test for bug 86162: This tests that there is no crash when using percentage value for scrollbar's padding property. On success you should see a frame with scrollbars and one PASS message in it.
+
+

Added: trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash.html (0 => 117007)


--- trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash.html	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-percent-padding-crash.html	2012-05-14 22:36:03 UTC (rev 117007)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+::-webkit-scrollbar {
+ -webkit-padding-start: 1%; background: #666 -webkit-gradient(linear, left top, right top, from(rgba(255,255,255,0.5)), color-stop(0.5, rgba(255,255,255,0.1)), color-stop(0.5, rgba(0,0,0,0)), to(rgba(0,0,0,0.01)));
+}
+</style>
+<script>
+function runTest() {
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        document.body.offsetTop;
+        layoutTestController.display();
+    }
+};
+</script>
+</head>
+<body _onload_="runTest()">
+<p>Test for <a href="" 86162</a>:
+This tests that there is no crash when using percentage value for scrollbar's padding property. On success you should see a frame with scrollbars and one PASS message in it.
+<div style="height: 1000px;"></div>
+</body>
+</html>

Added: trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash-expected.txt (0 => 117007)


--- trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash-expected.txt	2012-05-14 22:36:03 UTC (rev 117007)
@@ -0,0 +1 @@
+Test for bug 86162: This test passes if there is no crash when scrollbar's style changes.

Added: trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash.html (0 => 117007)


--- trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash.html	                        (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-scrollbarparts-repaint-crash.html	2012-05-14 22:36:03 UTC (rev 117007)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body::-webkit-scrollbar {
+    -webkit-font-feature-settings: "a bc";
+}
+</style>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+</head>
+<body style="-webkit-animation: move 1249008099s 471976428s linear backwards; border-bottom: solid mintcream 214em;">
+Test for <a href="" 86162</a>:
+This test passes if there is no crash when scrollbar's style changes.
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (117006 => 117007)


--- trunk/Source/WebCore/ChangeLog	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/Source/WebCore/ChangeLog	2012-05-14 22:36:03 UTC (rev 117007)
@@ -1,3 +1,47 @@
+2012-05-14  Takashi Sakamoto  <[email protected]>
+
+        Crash in WebCore::RenderObject::repaint
+        https://bugs.webkit.org/show_bug.cgi?id=86162
+
+        Reviewed by Abhishek Arya.
+
+        As RenderScrollbarPart has no parent renderer, we crash in
+        WebCore::RenderBoxModelObject::paddingLeft when paddingLeft has
+        percent value, e.g. 5%. However if we set the scrollbar's parent
+        renderer to a renderer owning the scrollbar by using setParent method,
+        RenderScrollbarPart::styleWillChange will invoke parent renderer's
+        repaint. This causes crash in WebCore::RenderObject::repaint if the
+        owning renderer is already destroyed.
+        To fix the first crash without the second crash, modify
+        RenderObject::containingBlock() to check isRenderScrollbarPart or not,
+        if parent() is 0.
+        If so, use scrollbar's owningRenderer from RenderScrollbarPart.
+
+        Test: scrollbars/scrollbar-percent-padding-crash.html
+              scrollbars/scrollbar-scrollbarparts-repaint-crash.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::containingBlock):
+        Modifying containingBlock. If parent() is 0 and isRenderScrollbarPart()
+        is true, use RenderScrollbarPart's m_scrollbar->owningRenderer()
+        instead of parent().
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::isRenderScrollbarPart):
+        (RenderObject):
+        Adding a new method, isRenderScrollbarPart.
+        * rendering/RenderScrollbarPart.cpp:
+        (WebCore::RenderScrollbarPart::rendererOwningScrollbar):
+        (WebCore):
+        Adding a new method, scrollbarOwningRenderer to obtain m_scrollar's
+        owningRenderer.
+        * rendering/RenderScrollbarPart.h:
+        (RenderScrollbarPart):
+        Removing "friend class RenderScrollbar".
+        (WebCore::RenderScrollbarPart::isRenderScrollbarPart):
+        (WebCore::toRenderScrollbarPart):
+        (WebCore):
+        Implementing isRenderScrollbarPart and toRenderScrollbarPart.
+
 2012-05-14  Mike West  <[email protected]>
 
         Content Security Policy console errors include violated directive.

Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (117006 => 117007)


--- trunk/Source/WebCore/rendering/RenderObject.cpp	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp	2012-05-14 22:36:03 UTC (rev 117007)
@@ -56,6 +56,7 @@
 #include "RenderRegion.h"
 #include "RenderRuby.h"
 #include "RenderRubyText.h"
+#include "RenderScrollbarPart.h"
 #include "RenderTableCaption.h"
 #include "RenderTableCell.h"
 #include "RenderTableCol.h"
@@ -698,6 +699,8 @@
 RenderBlock* RenderObject::containingBlock() const
 {
     RenderObject* o = parent();
+    if (!o && isRenderScrollbarPart())
+        o = toRenderScrollbarPart(this)->rendererOwningScrollbar();
     if (!isText() && m_style->position() == FixedPosition) {
         while (o && !o->isRenderView() && !(o->hasTransform() && o->isRenderBlock()))
             o = o->parent();

Modified: trunk/Source/WebCore/rendering/RenderObject.h (117006 => 117007)


--- trunk/Source/WebCore/rendering/RenderObject.h	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/Source/WebCore/rendering/RenderObject.h	2012-05-14 22:36:03 UTC (rev 117007)
@@ -355,6 +355,7 @@
 
     virtual bool isRenderFlowThread() const { return false; }
     virtual bool isRenderNamedFlowThread() const { return false; }
+    virtual bool isRenderScrollbarPart() const { return false; }
     bool canHaveRegionStyle() const { return isRenderBlock() && !isAnonymous() && !isRenderFlowThread(); }
 
     bool isRoot() const { return document()->documentElement() == m_node; }

Modified: trunk/Source/WebCore/rendering/RenderScrollbarPart.cpp (117006 => 117007)


--- trunk/Source/WebCore/rendering/RenderScrollbarPart.cpp	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/Source/WebCore/rendering/RenderScrollbarPart.cpp	2012-05-14 22:36:03 UTC (rev 117007)
@@ -184,4 +184,11 @@
     paint(paintInfo, paintOffset);
 }
 
+RenderObject* RenderScrollbarPart::rendererOwningScrollbar() const
+{
+    if (!m_scrollbar)
+        return 0;
+    return m_scrollbar->owningRenderer();
 }
+
+}

Modified: trunk/Source/WebCore/rendering/RenderScrollbarPart.h (117006 => 117007)


--- trunk/Source/WebCore/rendering/RenderScrollbarPart.h	2012-05-14 21:53:58 UTC (rev 117006)
+++ trunk/Source/WebCore/rendering/RenderScrollbarPart.h	2012-05-14 22:36:03 UTC (rev 117007)
@@ -53,6 +53,9 @@
     virtual LayoutUnit marginLeft() const { ASSERT(isIntegerValue(m_marginLeft)); return m_marginLeft; }
     virtual LayoutUnit marginRight() const { ASSERT(isIntegerValue(m_marginRight)); return m_marginRight; }
 
+    virtual bool isRenderScrollbarPart() const { return true; }
+    RenderObject* rendererOwningScrollbar() const;
+
 protected:
     virtual void styleWillChange(StyleDifference diff, const RenderStyle* newStyle);
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
@@ -69,6 +72,21 @@
     ScrollbarPart m_part;
 };
 
+inline RenderScrollbarPart* toRenderScrollbarPart(RenderObject* object)
+{
+    ASSERT(!object || object->isRenderScrollbarPart());
+    return static_cast<RenderScrollbarPart*>(object);
+}
+
+inline const RenderScrollbarPart* toRenderScrollbarPart(const RenderObject* object)
+{
+    ASSERT(!object || object->isRenderScrollbarPart());
+    return static_cast<const RenderScrollbarPart*>(object);
+}
+
+// This will catch anyone doing an unnecessary cast.
+void toRenderScrollbarPart(const RenderScrollbarPart*);
+
 } // namespace WebCore
 
 #endif // RenderScrollbarPart_h
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to