Diff
Modified: trunk/LayoutTests/ChangeLog (116475 => 116476)
--- trunk/LayoutTests/ChangeLog 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/LayoutTests/ChangeLog 2012-05-09 01:45:23 UTC (rev 116476)
@@ -1,3 +1,13 @@
+2012-05-08 Abhishek Arya <[email protected]>
+
+ Crash due to owning renderer not removed from custom scrollbar.
+ https://bugs.webkit.org/show_bug.cgi?id=80610
+
+ Reviewed by Eric Seidel.
+
+ * scrollbars/scrollbar-owning-renderer-crash-expected.txt: Added.
+ * scrollbars/scrollbar-owning-renderer-crash.html: Added.
+
2012-05-08 Kangil Han <[email protected]>
[EFL][DRT] Implement TextInputController.
Added: trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash-expected.txt (0 => 116476)
--- trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash-expected.txt 2012-05-09 01:45:23 UTC (rev 116476)
@@ -0,0 +1,2 @@
+WebKit bug 80610 - Crash due to owning renderer not removed from custom scrollbar.
+PASS. WebKit didn't crash.
Added: trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash.html (0 => 116476)
--- trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash.html (rev 0)
+++ trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash.html 2012-05-09 01:45:23 UTC (rev 116476)
@@ -0,0 +1,37 @@
+<html id="html1">
+<head id="head1">
+<style>
+::-webkit-scrollbar { height: 50000; }
+ </style>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+function runTest() {
+ child = document.getElementById('head1');
+ child.parentNode.removeChild(child);
+ document.body.offsetTop;
+ child = document.getElementById('html1');
+ child.parentNode.removeChild(child);
+ document.open();
+ document.write("WebKit bug 80610 - Crash due to owning renderer not removed from custom scrollbar. <br />");
+ document.write("PASS. WebKit didn't crash.");
+ document.close();
+}
+
+function finish() {
+ if (window.eventSender)
+ eventSender.mouseMoveTo(100, 100);
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+setTimeout("if (window.eventSender) eventSender.mouseMoveTo(0, 0);", 0);
+setTimeout("runTest();", 10);
+setTimeout("finish();", 11);
+</script>
+0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+</head>
+</html>
\ No newline at end of file
Property changes on: trunk/LayoutTests/scrollbars/scrollbar-owning-renderer-crash.html
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (116475 => 116476)
--- trunk/Source/WebCore/ChangeLog 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/ChangeLog 2012-05-09 01:45:23 UTC (rev 116476)
@@ -1,3 +1,38 @@
+2012-05-08 Abhishek Arya <[email protected]>
+
+ Crash due to owning renderer not removed from custom scrollbar.
+ https://bugs.webkit.org/show_bug.cgi?id=80610
+
+ Reviewed by Eric Seidel.
+
+ Test: scrollbars/scrollbar-owning-renderer-crash.html
+
+ Changed RenderScrollbar to keep pointer to owning node, instead of the
+ renderer. Renderer can get destroyed without informing the scrollbar, causing
+ crashes later. Remove code from r94107 since it is not needed anymore and saves
+ times when RenderBox is getting destroyed.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::createScrollbar): pass renderer's node.
+ * page/FrameView.h:
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBox::willBeDestroyed): no longer need this. came originally from r94107.
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::createScrollbar): pass renderer's node.
+ (WebCore::RenderLayer::destroyScrollbar): no longer need to clear owning renderer.
+ * rendering/RenderListBox.cpp:
+ (WebCore::RenderListBox::createScrollbar): pass renderer's node.
+ * rendering/RenderMenuList.cpp:
+ (WebCore::RenderMenuList::createScrollbar): pass renderer's node.
+ * rendering/RenderScrollbar.cpp:
+ (WebCore::RenderScrollbar::createCustomScrollbar): Store owner node instead of renderer.
+ (WebCore::RenderScrollbar::RenderScrollbar): Store owner node instead of renderer.
+ (WebCore::RenderScrollbar::owningRenderer): calculate owning renderer from owner node.
+ * rendering/RenderScrollbar.h:
+ (RenderScrollbar):
+ * rendering/RenderTextControlSingleLine.cpp:
+ (WebCore::RenderTextControlSingleLine::createScrollbar): pass renderer's node.
+
2012-05-08 Jon Lee <[email protected]>
Safari warns that it needs to resend the form in an iFrame when going back
Modified: trunk/Source/WebCore/page/FrameView.cpp (116475 => 116476)
--- trunk/Source/WebCore/page/FrameView.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/page/FrameView.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -482,12 +482,12 @@
// Try the <body> element first as a scrollbar source.
Element* body = doc ? doc->body() : 0;
if (body && body->renderer() && body->renderer()->style()->hasPseudoStyle(SCROLLBAR))
- return RenderScrollbar::createCustomScrollbar(this, orientation, body->renderer()->enclosingBox());
+ return RenderScrollbar::createCustomScrollbar(this, orientation, body);
// If the <body> didn't have a custom style, then the root element might.
Element* docElement = doc ? doc->documentElement() : 0;
if (docElement && docElement->renderer() && docElement->renderer()->style()->hasPseudoStyle(SCROLLBAR))
- return RenderScrollbar::createCustomScrollbar(this, orientation, docElement->renderBox());
+ return RenderScrollbar::createCustomScrollbar(this, orientation, docElement);
// If we have an owning iframe/frame element, then it can set the custom scrollbar also.
RenderPart* frameRenderer = m_frame->ownerRenderer();
@@ -2913,23 +2913,6 @@
return false;
}
-void FrameView::clearOwningRendererForCustomScrollbars(RenderBox* box)
-{
- const HashSet<RefPtr<Widget> >* viewChildren = children();
- HashSet<RefPtr<Widget> >::const_iterator end = viewChildren->end();
- for (HashSet<RefPtr<Widget> >::const_iterator current = viewChildren->begin(); current != end; ++current) {
- Widget* widget = current->get();
- if (widget->isScrollbar()) {
- Scrollbar* scrollbar = static_cast<Scrollbar*>(widget);
- if (scrollbar->isCustomScrollbar()) {
- RenderScrollbar* customScrollbar = toRenderScrollbar(scrollbar);
- if (customScrollbar->owningRenderer() == box)
- customScrollbar->clearOwningRenderer();
- }
- }
- }
-}
-
FrameView* FrameView::parentFrameView() const
{
if (Widget* parentView = parent()) {
Modified: trunk/Source/WebCore/page/FrameView.h (116475 => 116476)
--- trunk/Source/WebCore/page/FrameView.h 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/page/FrameView.h 2012-05-09 01:45:23 UTC (rev 116476)
@@ -318,8 +318,6 @@
void setAnimatorsAreActive();
RenderBox* embeddedContentBox() const;
-
- void clearOwningRendererForCustomScrollbars(RenderBox*);
void setTracksRepaints(bool);
bool isTrackingRepaints() const { return m_isTrackingRepaints; }
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -256,11 +256,6 @@
if (styleToUse) {
if (RenderView* view = this->view()) {
if (FrameView* frameView = view->frameView()) {
- // If this renderer is owning renderer for the FrameView's custom scrollbars,
- // we need to clear it from the scrollbar. See webkit bug 64737.
- if (styleToUse->hasPseudoStyle(SCROLLBAR))
- frameView->clearOwningRendererForCustomScrollbars(this);
-
if (styleToUse->position() == FixedPosition)
frameView->removeFixedObject();
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -2194,7 +2194,7 @@
RenderObject* actualRenderer = renderer()->node() ? renderer()->node()->shadowAncestorNode()->renderer() : renderer();
bool hasCustomScrollbarStyle = actualRenderer->isBox() && actualRenderer->style()->hasPseudoStyle(SCROLLBAR);
if (hasCustomScrollbarStyle)
- widget = RenderScrollbar::createCustomScrollbar(this, orientation, toRenderBox(actualRenderer));
+ widget = RenderScrollbar::createCustomScrollbar(this, orientation, actualRenderer->node());
else {
widget = Scrollbar::createNativeScrollbar(this, orientation, RegularScrollbar);
if (orientation == HorizontalScrollbar)
@@ -2210,14 +2210,10 @@
{
RefPtr<Scrollbar>& scrollbar = orientation == HorizontalScrollbar ? m_hBar : m_vBar;
if (scrollbar) {
- if (scrollbar->isCustomScrollbar())
- toRenderScrollbar(scrollbar.get())->clearOwningRenderer();
- else {
- if (orientation == HorizontalScrollbar)
- willRemoveHorizontalScrollbar(scrollbar.get());
- else
- willRemoveVerticalScrollbar(scrollbar.get());
- }
+ if (orientation == HorizontalScrollbar)
+ willRemoveHorizontalScrollbar(scrollbar.get());
+ else
+ willRemoveVerticalScrollbar(scrollbar.get());
scrollbar->removeFromParent();
scrollbar->disconnectFromScrollableArea();
Modified: trunk/Source/WebCore/rendering/RenderListBox.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderListBox.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderListBox.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -831,7 +831,7 @@
RefPtr<Scrollbar> widget;
bool hasCustomScrollbarStyle = style()->hasPseudoStyle(SCROLLBAR);
if (hasCustomScrollbarStyle)
- widget = RenderScrollbar::createCustomScrollbar(this, VerticalScrollbar, this);
+ widget = RenderScrollbar::createCustomScrollbar(this, VerticalScrollbar, this->node());
else {
widget = Scrollbar::createNativeScrollbar(this, VerticalScrollbar, theme()->scrollbarControlSizeForPart(ListboxPart));
didAddVerticalScrollbar(widget.get());
Modified: trunk/Source/WebCore/rendering/RenderMenuList.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderMenuList.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderMenuList.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -495,7 +495,7 @@
RefPtr<Scrollbar> widget;
bool hasCustomScrollbarStyle = style()->hasPseudoStyle(SCROLLBAR);
if (hasCustomScrollbarStyle)
- widget = RenderScrollbar::createCustomScrollbar(scrollableArea, orientation, this);
+ widget = RenderScrollbar::createCustomScrollbar(scrollableArea, orientation, this->node());
else
widget = Scrollbar::createNativeScrollbar(scrollableArea, orientation, controlSize);
return widget.release();
Modified: trunk/Source/WebCore/rendering/RenderScrollbar.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderScrollbar.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderScrollbar.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -34,16 +34,18 @@
namespace WebCore {
-PassRefPtr<Scrollbar> RenderScrollbar::createCustomScrollbar(ScrollableArea* scrollableArea, ScrollbarOrientation orientation, RenderBox* renderer, Frame* owningFrame)
+PassRefPtr<Scrollbar> RenderScrollbar::createCustomScrollbar(ScrollableArea* scrollableArea, ScrollbarOrientation orientation, Node* ownerNode, Frame* owningFrame)
{
- return adoptRef(new RenderScrollbar(scrollableArea, orientation, renderer, owningFrame));
+ return adoptRef(new RenderScrollbar(scrollableArea, orientation, ownerNode, owningFrame));
}
-RenderScrollbar::RenderScrollbar(ScrollableArea* scrollableArea, ScrollbarOrientation orientation, RenderBox* renderer, Frame* owningFrame)
+RenderScrollbar::RenderScrollbar(ScrollableArea* scrollableArea, ScrollbarOrientation orientation, Node* ownerNode, Frame* owningFrame)
: Scrollbar(scrollableArea, orientation, RegularScrollbar, RenderScrollbarTheme::renderScrollbarTheme())
- , m_owner(renderer)
+ , m_owner(ownerNode)
, m_owningFrame(owningFrame)
{
+ ASSERT(ownerNode || owningFrame);
+
// FIXME: We need to do this because RenderScrollbar::styleChanged is called as soon as the scrollbar is created.
// Update the scrollbar size.
@@ -81,7 +83,7 @@
RenderBox* currentRenderer = m_owningFrame->ownerRenderer();
return currentRenderer;
}
- return m_owner;
+ return m_owner && m_owner->renderer() ? m_owner->renderer()->enclosingBox() : 0;
}
void RenderScrollbar::setParent(ScrollView* parent)
Modified: trunk/Source/WebCore/rendering/RenderScrollbar.h (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderScrollbar.h 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderScrollbar.h 2012-05-09 01:45:23 UTC (rev 116476)
@@ -26,6 +26,7 @@
#ifndef RenderScrollbar_h
#define RenderScrollbar_h
+#include "Node.h"
#include "RenderStyleConstants.h"
#include "Scrollbar.h"
#include <wtf/HashMap.h>
@@ -39,18 +40,17 @@
class RenderScrollbar : public Scrollbar {
protected:
- RenderScrollbar(ScrollableArea*, ScrollbarOrientation, RenderBox*, Frame*);
+ RenderScrollbar(ScrollableArea*, ScrollbarOrientation, Node*, Frame*);
public:
friend class Scrollbar;
- static PassRefPtr<Scrollbar> createCustomScrollbar(ScrollableArea*, ScrollbarOrientation, RenderBox*, Frame* owningFrame = 0);
+ static PassRefPtr<Scrollbar> createCustomScrollbar(ScrollableArea*, ScrollbarOrientation, Node*, Frame* owningFrame = 0);
virtual ~RenderScrollbar();
static ScrollbarPart partForStyleResolve();
static RenderScrollbar* scrollbarForStyleResolve();
RenderBox* owningRenderer() const;
- void clearOwningRenderer() { m_owner = 0; }
void paintPart(GraphicsContext*, ScrollbarPart, const IntRect&);
@@ -80,7 +80,12 @@
PassRefPtr<RenderStyle> getScrollbarPseudoStyle(ScrollbarPart, PseudoId);
void updateScrollbarPart(ScrollbarPart, bool destroy = false);
- RenderBox* m_owner;
+ // This Scrollbar(Widget) may outlive the DOM which created it (during tear down),
+ // so we keep a reference to the Node which caused this custom scrollbar creation.
+ // This will not create a reference cycle as the Widget tree is owned by our containing
+ // FrameView which this Node pointer can in no way keep alive. See webkit bug 80610.
+ RefPtr<Node> m_owner;
+
Frame* m_owningFrame;
HashMap<unsigned, RenderScrollbarPart*> m_parts;
};
Modified: trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp (116475 => 116476)
--- trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp 2012-05-09 01:32:43 UTC (rev 116475)
+++ trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp 2012-05-09 01:45:23 UTC (rev 116476)
@@ -750,7 +750,7 @@
RefPtr<Scrollbar> widget;
bool hasCustomScrollbarStyle = style()->hasPseudoStyle(SCROLLBAR);
if (hasCustomScrollbarStyle)
- widget = RenderScrollbar::createCustomScrollbar(scrollableArea, orientation, this);
+ widget = RenderScrollbar::createCustomScrollbar(scrollableArea, orientation, this->node());
else
widget = Scrollbar::createNativeScrollbar(scrollableArea, orientation, controlSize);
return widget.release();
