Changeset 27585 in webkit

Timestamp:

Nov 7, 2007 5:54:14 PM (16 years ago)

Author:

mitz@apple.com

Message:

Reviewed by Darin Adler.

• fix <rdar://problem/5523503> Safari crashes clicking scroll bar in FaceBook 'Trips'

Layers and listboxes are two kinds of ScrollBarClient that can be
removed while the scrollbar is tracking the mouse. The scrollbar is not
destroyed until later, and meanwhile it can try to call the client,
which results in a crash.

• manual-tests/stale-scrollbar-client-crash.html: Added.
• platform/ScrollBar.h: (WebCore::Scrollbar::setClient): Added.
• rendering/RenderLayer.cpp: (WebCore::RenderLayer::destroyScrollbar): Call Scrollbar::setClient().
• rendering/RenderListBox.cpp: (WebCore::RenderListBox::~RenderListBox): Ditto.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• manual-tests/stale-scrollbar-client-crash.html (added)
• platform/ScrollBar.h (modified) (1 diff)
• rendering/RenderLayer.cpp (modified) (1 diff)
• rendering/RenderListBox.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-11-07  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Darin Adler.
+
+        - fix <rdar://problem/5523503> Safari crashes clicking scroll bar in FaceBook 'Trips'
+
+        Layers and listboxes are two kinds of ScrollBarClient that can be
+        removed while the scrollbar is tracking the mouse. The scrollbar is not
+        destroyed until later, and meanwhile it can try to call the client,
+        which results in a crash.
+
+        * manual-tests/stale-scrollbar-client-crash.html: Added.
+        * platform/ScrollBar.h:
+        (WebCore::Scrollbar::setClient): Added.
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::destroyScrollbar): Call Scrollbar::setClient().
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::~RenderListBox): Ditto.
+
 2007-11-07  Adam Roben  <aroben@apple.com>
 

trunk/WebCore/platform/ScrollBar.h

@@ -60 +60 @@
     virtual ~Scrollbar() {}
 
+    void setClient(ScrollbarClient* client) { m_client = client; }
+
     virtual bool isWidget() const = 0;
 

trunk/WebCore/rendering/RenderLayer.cpp

@@ -1026 +1026 @@
         if (scrollbar->isWidget())
             static_cast<PlatformScrollbar*>(scrollbar.get())->removeFromParent();
+        scrollbar->setClient(0);
 
         // FIXME: Destroy the engine scrollbar.

trunk/WebCore/rendering/RenderListBox.cpp

@@ -80 +80 @@
 RenderListBox::~RenderListBox()
 {
-    if (m_vBar && m_vBar->isWidget())
-        if (FrameView* view = node()->document()->view())
-            view->removeChild(static_cast<PlatformScrollbar*>(m_vBar.get()));
+    if (m_vBar) {
+        if (m_vBar->isWidget()) {
+            if (FrameView* view = node()->document()->view())
+                view->removeChild(static_cast<PlatformScrollbar*>(m_vBar.get()));
+        }
+        m_vBar->setClient(0);
+    }
 }