Changeset 25726 in webkit

Timestamp:

Sep 25, 2007 11:35:54 AM (17 years ago)

Author:

bdakin

Message:

Reviewed by Darin.

Fix for <rdar://problem/5466459> CrashTracer: [USER] 1 crash in
NetNewsWire at com.apple.WebCore: WebCore::bidiNext + 485 (15241)

Mitz discovered that this crash appeared because r25128 made it
possible for RenderPartObject::updateWidget() to be called during
layout. updateWidget() can, through a series of calls, cause an
attach/detach to happen, which is very bad in the middle of a
layout and is what led to this crash. This patch fixes that by
having the FrameView keep track of a queue of RenderPartObjects
that need to call updateWidget(), and it goes through the queue
calling updateWidget() as soon as layout is done.

• page/FrameView.cpp: We only want to call updateWidget() if we are not in a nested layout. Unfortunately, the existing variables on FrameViewPrivate do not have exactly the information that we need, so I added nestedLayoutCount. (WebCore::FrameViewPrivate::reset): Reset nestedLayoutCount. (WebCore::FrameView::layout): Increment nestedLayoutCount once we have gotten through all of the early returns. Call updateWidget() after layout is nestedLayoutCount is 1 and there are widgets to update. Decrement nestedLayoutCount at the end. (WebCore::FrameView::addWidgetToUpdate): (WebCore::FrameView::removeWidgetToUpdate):
• page/FrameView.h:
• rendering/RenderPartObject.cpp: (WebCore::RenderPartObject::~RenderPartObject): Remove this from the FrameView's update set. (WebCore::RenderPartObject::layout): Instead of calling updateWidget() immediately, add this to the update widget set on FrameView.
• rendering/RenderPartObject.h:

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/FrameView.cpp (modified) (5 diffs)
• page/FrameView.h (modified) (4 diffs)
• rendering/RenderPartObject.cpp (modified) (3 diffs)
• rendering/RenderPartObject.h (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-09-25  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Darin.
+
+        Fix for <rdar://problem/5466459> CrashTracer: [USER] 1 crash in 
+        NetNewsWire at com.apple.WebCore: WebCore::bidiNext + 485 (15241)
+
+        Mitz discovered that this crash appeared because r25128 made it 
+        possible for RenderPartObject::updateWidget() to be called during 
+        layout. updateWidget() can, through a series of calls, cause an 
+        attach/detach to happen, which is very bad in the middle of a 
+        layout and is what led to this crash. This patch fixes that by 
+        having the FrameView keep track of a queue of RenderPartObjects 
+        that need to call updateWidget(), and it goes through the queue 
+        calling updateWidget() as soon as layout is done.
+
+        * page/FrameView.cpp: We only want to call updateWidget() if we are 
+        not in a nested layout. Unfortunately, the existing variables on 
+        FrameViewPrivate do not have exactly the information that we need, 
+        so I added nestedLayoutCount.
+        (WebCore::FrameViewPrivate::reset): Reset nestedLayoutCount.
+        (WebCore::FrameView::layout): Increment nestedLayoutCount once we 
+        have gotten through all of the early returns. Call updateWidget() 
+        after layout is nestedLayoutCount is 1 and there are widgets to 
+        update. Decrement nestedLayoutCount at the end.
+        (WebCore::FrameView::addWidgetToUpdate):
+        (WebCore::FrameView::removeWidgetToUpdate):
+        * page/FrameView.h:
+        * rendering/RenderPartObject.cpp:
+        (WebCore::RenderPartObject::~RenderPartObject): Remove this from 
+        the FrameView's update set.
+        (WebCore::RenderPartObject::layout): Instead of calling 
+        updateWidget() immediately, add this to the update widget set on 
+        FrameView.
+        * rendering/RenderPartObject.h:
+
 2007-09-25  David Kilzer  <ddkilzer@webkit.org>
 

trunk/WebCore/page/FrameView.cpp

@@ -39 +39 @@
 #include "OverflowEvent.h"
 #include "RenderPart.h"
+#include "RenderPartObject.h"
 #include "RenderTheme.h"
 #include "RenderView.h"
@@ -82 +83 @@
         midLayout = false;
         layoutCount = 0;
+        nestedLayoutCount = 0;
         firstLayout = true;
         repaintRects.clear();
@@ -103 +105 @@
     bool midLayout;
     int layoutCount;
+    unsigned nestedLayoutCount;
 
     bool firstLayout;
@@ -346 +349 @@
         return;
     }
+
+    d->nestedLayoutCount++;
 
     ScrollbarMode hMode = d->hmode;
@@ -473 +478 @@
                              visibleHeight() < contentsHeight());
 
+    if (m_widgetUpdateSet && d->nestedLayoutCount == 1) {
+        HashSet<RenderPartObject*> set;
+        m_widgetUpdateSet->swap(set);
+        
+        HashSet<RenderPartObject*>::iterator end = set.end();
+        for (HashSet<RenderPartObject*>::iterator it = set.begin(); it != end; ++it)
+            (*it)->updateWidget(false);
+    }
+
     // Allow events scheduled during layout to fire
     resumeScheduledEvents();
+
+    d->nestedLayoutCount--;
+}
+
+void FrameView::addWidgetToUpdate(RenderPartObject* object)
+{
+    if (!m_widgetUpdateSet)
+        m_widgetUpdateSet.set(new HashSet<RenderPartObject*>);
+
+    m_widgetUpdateSet->add(object);
+}
+
+void FrameView::removeWidgetToUpdate(RenderPartObject* object)
+{
+    if (!m_widgetUpdateSet)
+        return;
+
+    m_widgetUpdateSet->remove(object);
 }
 

trunk/WebCore/page/FrameView.h

@@ -29 +29 @@
 #include "IntSize.h"
 #include <wtf/Forward.h>
+#include <wtf/OwnPtr.h>
 
 namespace WebCore {
@@ -42 +43 @@
 class RenderLayer;
 class RenderObject;
+class RenderPartObject;
 class String;
 
@@ -126 +128 @@
     void setWasScrolledByUser(bool);
 
+    void addWidgetToUpdate(RenderPartObject*);
+    void removeWidgetToUpdate(RenderPartObject*);
+
     // FIXME: This method should be used by all platforms, but currently depends on ScrollView::children,
     // which not all methods have. Once FrameView and ScrollView are merged, this #if should be removed.
@@ -152 +157 @@
     IntSize m_size;
     IntSize m_margins;
+    OwnPtr<HashSet<RenderPartObject*> > m_widgetUpdateSet;
     RefPtr<Frame> m_frame;
     FrameViewPrivate* d;

trunk/WebCore/rendering/RenderPartObject.cpp

@@ -41 +41 @@
 #include "MIMETypeRegistry.h"
 #include "Page.h"
+#include "RenderView.h"
 #include "Text.h"
 
@@ -53 +54 @@
     setInline(true);
     m_hasFallbackContent = false;
+}
+
+RenderPartObject::~RenderPartObject()
+{
+    if (m_view)
+        m_view->removeWidgetToUpdate(this);
 }
 
@@ -268 +275 @@
     RenderPart::layout();
 
-    if (!m_widget)
-        updateWidget(false);
+    if (!m_widget && m_view)
+        m_view->addWidgetToUpdate(this);
     
     setNeedsLayout(false);

trunk/WebCore/rendering/RenderPartObject.h

@@ -33 +33 @@
 public:
     RenderPartObject(HTMLFrameOwnerElement*);
+    virtual ~RenderPartObject();
 
     virtual const char* renderName() const { return "RenderPartObject"; }