Reviewed by Maciej.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 11:48:25 +0000 (11:48 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2006 11:48:25 +0000 (11:48 +0000)
        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=7048
          Reproducible crash when onscroll handler deletes the layer or its object

        Test: onscroll-layer-self-destruct.html

        * rendering/render_layer.cpp: (WebCore::RenderLayer::scrollToOffset):
        Don't send the scroll event until after we've done everything else we
        need to do.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@12574 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/rendering/render_layer.cpp

index 20b8e7587ebd96d27d4f26ca36b39582fa8f7a5a..b6024656a1bd385eea74a9d1353d94a4872a1d49 100644 (file)
@@ -1,3 +1,16 @@
+2006-02-05  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - fix http://bugzilla.opendarwin.org/show_bug.cgi?id=7048
+          Reproducible crash when onscroll handler deletes the layer or its object
+
+        Test: onscroll-layer-self-destruct.html
+
+        * rendering/render_layer.cpp: (WebCore::RenderLayer::scrollToOffset):
+        Don't send the scroll event until after we've done everything else we
+        need to do.
+
 2006-02-05  Darin Adler  <darin@apple.com>
 
         Rubber stamped by Maciej.
index b7928cb2abf55d2a597408b1d7b179415e1a29f3..f8bf16df7627d111298e9c575151f7923262167d 100644 (file)
@@ -105,8 +105,7 @@ void ClipRects::destroy(RenderArena* renderArena)
     renderArena->free(*(size_t *)this, this);
 }
 
-void
-RenderScrollMediator::slotValueChanged(int val)
+void RenderScrollMediator::slotValueChanged(int val)
 {
     m_layer->updateScrollPositionFromScrollbars();
 }
@@ -524,8 +523,7 @@ RenderLayer::subtractScrollOffset(int& x, int& y)
     y -= scrollYOffset();
 }
 
-void
-RenderLayer::scrollToOffset(int x, int y, bool updateScrollbars, bool repaint)
+void RenderLayer::scrollToOffset(int x, int y, bool updateScrollbars, bool repaint)
 {
     if (renderer()->style()->overflow() != OMARQUEE) {
         if (x < 0) x = 0;
@@ -563,9 +561,6 @@ RenderLayer::scrollToOffset(int x, int y, bool updateScrollbars, bool repaint)
         m_object->canvas()->updateWidgetPositions();
     }
 
-    // Fire the scroll DOM event.
-    m_object->element()->dispatchHTMLEvent(scrollEvent, true, false);
-
     // Just schedule a full repaint of our object.
     if (repaint)
         m_object->repaint();
@@ -576,6 +571,9 @@ RenderLayer::scrollToOffset(int x, int y, bool updateScrollbars, bool repaint)
         if (m_vBar)
             m_vBar->setValue(m_scrollY);
     }
+
+    // Fire the scroll DOM event.
+    m_object->element()->dispatchHTMLEvent(scrollEvent, true, false);
 }
 
 void RenderLayer::scrollRectToVisible(const IntRect &rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
@@ -599,6 +597,8 @@ void RenderLayer::scrollRectToVisible(const IntRect &rect, const ScrollAlignment
             int diffX = scrollXOffset();
             int diffY = m_scrollY;
             scrollToOffset(xOffset, yOffset);
+            // FIXME: At this point a scroll event fired, which could have deleted this layer.
+            // Need to handle this case.
             diffX = scrollXOffset() - diffX;
             diffY = m_scrollY - diffY;
             newRect.setX(rect.x() - diffX);
@@ -874,6 +874,8 @@ RenderLayer::updateScrollInfoAfterLayout()
         int newY = kMax(0, kMin(m_scrollY, scrollHeight() - m_object->clientHeight()));
         if (newX != scrollXOffset() || newY != m_scrollY)
             scrollToOffset(newX, newY);
+        // FIXME: At this point a scroll event fired, which could have deleted this layer.
+        // Need to handle this case.
     }
 
     bool haveHorizontalBar = m_hBar;
@@ -1647,6 +1649,8 @@ void Marquee::start()
             else
                 m_layer->scrollToOffset(0, m_start, false, false);
         }
+        // FIXME: At this point a scroll event fired, which could have deleted this layer,
+        // including the marquee. Need to handle this case.
     }
     else {
         m_suspended = false;