Changeset 19005 in webkit

Timestamp:

Jan 20, 2007 4:35:32 PM (17 years ago)

Author:

weinig

Message:

LayoutTests:

Reviewed by Darin.

• test for ​http://bugs.webkit.org/show_bug.cgi?id=8360 Repro crash when onscroll handler deletes the scrolled object

• fast/layers/removed-by-scroll-handler-expected.checksum: Added.
• fast/layers/removed-by-scroll-handler-expected.png: Added.
• fast/layers/removed-by-scroll-handler-expected.txt: Added.
• fast/layers/removed-by-scroll-handler.html: Added.

WebCore:

Reviewed by Darin.

• fix ​http://bugs.webkit.org/show_bug.cgi?id=8360 Repro crash when onscroll handler deletes the scrolled object

Test: fast/layers/removed-by-scroll-handler.html

• rendering/RenderLayer.cpp: (WebCore::RenderLayer::RenderLayer): (WebCore::RenderLayer::dispatchScrollEvent): Added. Called when the timer fires and dispatches the scroll event to the layer's element. (WebCore::RenderLayer::scrollToOffset): Instead of dispatching the scroll event from this function, which is called during layout, schedule a 0-duration timer to dispatch the event after layout. (WebCore::RenderLayer::scrollRectToVisible): (WebCore::RenderLayer::updateScrollInfoAfterLayout): (WebCore::Marquee::start):
• rendering/RenderLayer.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/layers/removed-by-scroll-handler-expected.checksum (added)
• LayoutTests/fast/layers/removed-by-scroll-handler-expected.png (added)
• LayoutTests/fast/layers/removed-by-scroll-handler-expected.txt (added)
• LayoutTests/fast/layers/removed-by-scroll-handler.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/rendering/RenderLayer.cpp (modified) (6 diffs)
• WebCore/rendering/RenderLayer.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2007-01-20  Mitz Pettel  <mitz@webkit.org>
+
+        Reviewed by Darin.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=8360
+          Repro crash when onscroll handler deletes the scrolled object
+
+        * fast/layers/removed-by-scroll-handler-expected.checksum: Added.
+        * fast/layers/removed-by-scroll-handler-expected.png: Added.
+        * fast/layers/removed-by-scroll-handler-expected.txt: Added.
+        * fast/layers/removed-by-scroll-handler.html: Added.
+
 2007-01-20  Nikolas Zimmermann  <zimmermann@kde.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-01-20  Mitz Pettel  <mitz@webkit.org>
+
+        Reviewed by Darin.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=8360
+          Repro crash when onscroll handler deletes the scrolled object
+
+        Test: fast/layers/removed-by-scroll-handler.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::RenderLayer):
+        (WebCore::RenderLayer::dispatchScrollEvent): Added. Called when the timer
+        fires and dispatches the scroll event to the layer's element.
+        (WebCore::RenderLayer::scrollToOffset): Instead of dispatching the scroll
+        event from this function, which is called during layout, schedule a 0-duration
+        timer to dispatch the event after layout.
+        (WebCore::RenderLayer::scrollRectToVisible):
+        (WebCore::RenderLayer::updateScrollInfoAfterLayout):
+        (WebCore::Marquee::start):
+        * rendering/RenderLayer.h:
+
 2007-01-20  Nikolas Zimmermann  <zimmermann@kde.org>
 

trunk/WebCore/rendering/RenderLayer.cpp

@@ -113 +113 @@
 
 RenderLayer::RenderLayer(RenderObject* object)
-: m_object(object),
-m_parent(0),
-m_previous(0),
-m_next(0),
-m_first(0),
-m_last(0),
-m_repaintX(0),
-m_repaintY(0),
-m_relX(0),
-m_relY(0),
-m_x(0),
-m_y(0),
-m_width(0),
-m_height(0),
-m_scrollX(0),
-m_scrollY(0),
-m_scrollOriginX(0),
-m_scrollLeftOverflow(0),
-m_scrollWidth(0),
-m_scrollHeight(0),
-m_inResizeMode(false),
-m_posZOrderList(0),
-m_negZOrderList(0),
-m_overflowList(0),
-m_clipRects(0) ,
-m_scrollDimensionsDirty(true),
-m_zOrderListsDirty(true),
-m_overflowListDirty(true),
-m_isOverflowOnly(shouldBeOverflowOnly()),
-m_usedTransparency(false),
-m_inOverflowRelayout(false),
-m_repaintOverflowOnResize(false),
-m_overflowStatusDirty(true),
-m_visibleContentStatusDirty( true ),
-m_hasVisibleContent( false ),
-m_visibleDescendantStatusDirty( false ),
-m_hasVisibleDescendant( false ),
-m_marquee(0)
+    : m_object(object)
+    , m_parent(0)
+    , m_previous(0)
+    , m_next(0)
+    , m_first(0)
+    , m_last(0)
+    , m_repaintX(0)
+    , m_repaintY(0)
+    , m_relX(0)
+    , m_relY(0)
+    , m_x(0)
+    , m_y(0)
+    , m_width(0)
+    , m_height(0)
+    , m_scrollX(0)
+    , m_scrollY(0)
+    , m_scrollOriginX(0)
+    , m_scrollLeftOverflow(0)
+    , m_scrollWidth(0)
+    , m_scrollHeight(0)
+    , m_inResizeMode(false)
+    , m_posZOrderList(0)
+    , m_negZOrderList(0)
+    , m_overflowList(0)
+    , m_clipRects(0)
+    , m_scrollDimensionsDirty(true)
+    , m_zOrderListsDirty(true)
+    , m_overflowListDirty(true)
+    , m_isOverflowOnly(shouldBeOverflowOnly())
+    , m_usedTransparency(false)
+    , m_inOverflowRelayout(false)
+    , m_repaintOverflowOnResize(false)
+    , m_overflowStatusDirty(true)
+    , m_visibleContentStatusDirty(true)
+    , m_hasVisibleContent(false)
+    , m_visibleDescendantStatusDirty(false)
+    , m_hasVisibleDescendant(false)
+    , m_marquee(0)
+    , m_scrollEventTimer(this, &RenderLayer::dispatchScrollEvent)
 {
     if (!object->firstChild() && object->style()) {
@@ -673 +674 @@
 }
 
+void RenderLayer::dispatchScrollEvent(Timer<RenderLayer>* timer)
+{
+    EventTargetNodeCast(m_object->element())->dispatchHTMLEvent(scrollEvent, true, false);
+}
+
 void RenderLayer::scrollToOffset(int x, int y, bool updateScrollbars, bool repaint)
 {
@@ -722 +728 @@
     }
 
-    // Fire the scroll DOM event.
-    EventTargetNodeCast(m_object->element())->dispatchHTMLEvent(scrollEvent, true, false);
+    // Schedule the scroll DOM event. Since the event handler might destroy the object and its renderer,
+    // we do not dispatch the event here during layout.
+    m_scrollEventTimer.startOneShot(0);
 }
 
@@ -750 +757 @@
             int diffY = scrollYOffset();
             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 = scrollYOffset() - diffY;
@@ -1185 +1190 @@
         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.
     }
 
@@ -2153 +2156 @@
         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 {

trunk/WebCore/rendering/RenderLayer.h

@@ -378 +378 @@
     void updateVisibilityStatus();
 
+    void dispatchScrollEvent(Timer<RenderLayer>*);
+
 protected:   
     RenderObject* m_object;
@@ -453 +455 @@
 
     Marquee* m_marquee; // Used by layers with overflow:marquee
+    
+    Timer<RenderLayer> m_scrollEventTimer;
 };