- 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