<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[41469] trunk/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/41469">41469</a></dd>
<dt>Author</dt> <dd>eric@webkit.org</dd>
<dt>Date</dt> <dd>2009-03-05 18:40:36 -0800 (Thu, 05 Mar 2009)</dd>
</dl>

<h3>Log Message</h3>
<pre>        Reviewed by David Hyatt.

        Changes to RenderLayer destruction to hopefully help catch an elusive crasher
        https://bugs.webkit.org/show_bug.cgi?id=24409

        Added a new RenderBoxModelObject::destroyLayer() call which is
        now the only way which RenderLayers should ever be destroyed.
        This ensures that the pointer to the layer is cleared in the
        RenderObject after destruction, allowing us to ASSERT in the
        RenderBoxModelObject destructor.

        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::calcAbsoluteHorizontalReplaced):
        * rendering/RenderBoxModelObject.cpp:
        (WebCore::RenderBoxModelObject::~RenderBoxModelObject):
        (WebCore::RenderBoxModelObject::destroyLayer):
        (WebCore::RenderBoxModelObject::destroy):
        (WebCore::RenderBoxModelObject::styleDidChange):
        * rendering/RenderBoxModelObject.h:
        * rendering/RenderLayer.cpp:
        (WebCore::RenderLayer::stackingContext):
        (WebCore::RenderLayer::destroy):
        (WebCore::RenderLayer::removeOnlyThisLayer):
        * rendering/RenderLayer.h:
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::destroy):
        * rendering/RenderWidget.cpp:
        (WebCore::RenderWidget::destroy):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorerenderingRenderBoxcpp">trunk/WebCore/rendering/RenderBox.cpp</a></li>
<li><a href="#trunkWebCorerenderingRenderBoxModelObjectcpp">trunk/WebCore/rendering/RenderBoxModelObject.cpp</a></li>
<li><a href="#trunkWebCorerenderingRenderBoxModelObjecth">trunk/WebCore/rendering/RenderBoxModelObject.h</a></li>
<li><a href="#trunkWebCorerenderingRenderLayercpp">trunk/WebCore/rendering/RenderLayer.cpp</a></li>
<li><a href="#trunkWebCorerenderingRenderLayerh">trunk/WebCore/rendering/RenderLayer.h</a></li>
<li><a href="#trunkWebCorerenderingRenderObjectcpp">trunk/WebCore/rendering/RenderObject.cpp</a></li>
<li><a href="#trunkWebCorerenderingRenderWidgetcpp">trunk/WebCore/rendering/RenderWidget.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/ChangeLog        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -2,6 +2,37 @@
</span><span class="cx"> 
</span><span class="cx">         Reviewed by David Hyatt.
</span><span class="cx"> 
</span><ins>+        Changes to RenderLayer destruction to hopefully help catch an elusive crasher
+        https://bugs.webkit.org/show_bug.cgi?id=24409
+        
+        Added a new RenderBoxModelObject::destroyLayer() call which is
+        now the only way which RenderLayers should ever be destroyed.
+        This ensures that the pointer to the layer is cleared in the
+        RenderObject after destruction, allowing us to ASSERT in the
+        RenderBoxModelObject destructor.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::calcAbsoluteHorizontalReplaced):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::~RenderBoxModelObject):
+        (WebCore::RenderBoxModelObject::destroyLayer):
+        (WebCore::RenderBoxModelObject::destroy):
+        (WebCore::RenderBoxModelObject::styleDidChange):
+        * rendering/RenderBoxModelObject.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::stackingContext):
+        (WebCore::RenderLayer::destroy):
+        (WebCore::RenderLayer::removeOnlyThisLayer):
+        * rendering/RenderLayer.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::destroy):
+        * rendering/RenderWidget.cpp:
+        (WebCore::RenderWidget::destroy):
+
+2009-03-05  Eric Seidel  &lt;eric@webkit.org&gt;
+
+        Reviewed by David Hyatt.
+
</ins><span class="cx">         Remove old, unused IE 5.5 scrollbar-* CSS properties.
</span><span class="cx">         Sort the unimplemented getComputedStyle properties so it's
</span><span class="cx">         easier to see which ones actually need implementation.
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderBox.cpp (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderBox.cpp        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderBox.cpp        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -2354,7 +2354,7 @@
</span><span class="cx">         } else {
</span><span class="cx">             RenderObject* po = parent();
</span><span class="cx">             // 'staticX' should already have been set through layout of the parent.
</span><del>-            int staticPosition = layer()-&gt;staticX() + containerWidth + toRenderBoxModelObject(containerBlock)-&gt;borderRight();
</del><ins>+            int staticPosition = layer()-&gt;staticX() + containerWidth + containerBlock-&gt;borderRight();
</ins><span class="cx">             for ( ; po &amp;&amp; po != containerBlock; po = po-&gt;parent()) {
</span><span class="cx">                 if (po-&gt;isBox())
</span><span class="cx">                     staticPosition += toRenderBox(po)-&gt;x();
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderBoxModelObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderBoxModelObject.cpp (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderBoxModelObject.cpp        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderBoxModelObject.cpp        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -50,13 +50,27 @@
</span><span class="cx"> 
</span><span class="cx"> RenderBoxModelObject::~RenderBoxModelObject()
</span><span class="cx"> {
</span><ins>+    // Our layer should have been destroyed and cleared by now
+    ASSERT(!hasLayer());
+    ASSERT(!m_layer);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+void RenderBoxModelObject::destroyLayer()
+{
+    ASSERT(hasLayer());
+    ASSERT(m_layer);
+    m_layer-&gt;destroy(renderArena());
+    m_layer = 0;
+    setHasLayer(false);
+}
+
</ins><span class="cx"> void RenderBoxModelObject::destroy()
</span><span class="cx"> {
</span><span class="cx">     // This must be done before we destroy the RenderObject.
</span><span class="cx">     if (m_layer)
</span><span class="cx">         m_layer-&gt;clearClipRects();
</span><ins>+
+    // RenderObject::destroy calls back to destroyLayer() for layer destruction
</ins><span class="cx">     RenderObject::destroy();
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -100,13 +114,9 @@
</span><span class="cx">                 m_layer-&gt;updateLayerPositions();
</span><span class="cx">         }
</span><span class="cx">     } else if (layer() &amp;&amp; layer()-&gt;parent()) {
</span><del>-        
-        RenderLayer* layer = m_layer;
-        m_layer = 0;
-        setHasLayer(false);
</del><span class="cx">         setHasTransform(false); // Either a transform wasn't specified or the object doesn't support transforms, so just null out the bit.
</span><span class="cx">         setHasReflection(false);
</span><del>-        layer-&gt;removeOnlyThisLayer();
</del><ins>+        m_layer-&gt;removeOnlyThisLayer(); // calls destroyLayer() which clears m_layer
</ins><span class="cx">         if (s_wasFloating &amp;&amp; isFloating())
</span><span class="cx">             setChildNeedsLayout(true);
</span><span class="cx">     }
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderBoxModelObjecth"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderBoxModelObject.h (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderBoxModelObject.h        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderBoxModelObject.h        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -96,6 +96,9 @@
</span><span class="cx">     // The difference between this inline's baseline position and the line's baseline position.
</span><span class="cx">     int verticalPosition(bool firstLine) const;
</span><span class="cx"> 
</span><ins>+    // Called by RenderObject::destroy() (and RenderWidget::destroy()) and is the only way layers should ever be destroyed
+    void destroyLayer();
+
</ins><span class="cx"> protected:
</span><span class="cx">     void calculateBackgroundImageGeometry(const FillLayer*, int tx, int ty, int w, int h, IntRect&amp; destRect, IntPoint&amp; phase, IntSize&amp; tileSize);
</span><span class="cx">     IntSize calculateBackgroundSize(const FillLayer*, int scaledWidth, int scaledHeight) const;
</span><span class="lines">@@ -123,7 +126,7 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> // This will catch anyone doing an unnecessary cast.
</span><del>-void toRenderBoxModelObject(const RenderBox*);
</del><ins>+void toRenderBoxModelObject(const RenderBoxModelObject*);
</ins><span class="cx"> 
</span><span class="cx"> } // namespace WebCore
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderLayercpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderLayer.cpp (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderLayer.cpp        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderLayer.cpp        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -610,13 +610,12 @@
</span><span class="cx">                       style-&gt;perspectiveOriginY().calcFloatValue(borderBox.height()));
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-RenderLayer *RenderLayer::stackingContext() const
</del><ins>+RenderLayer* RenderLayer::stackingContext() const
</ins><span class="cx"> {
</span><del>-    RenderLayer* curr = parent();
-    for ( ; curr &amp;&amp; !curr-&gt;renderer()-&gt;isRenderView() &amp;&amp; !curr-&gt;renderer()-&gt;isRoot() &amp;&amp;
-          curr-&gt;renderer()-&gt;style()-&gt;hasAutoZIndex();
-          curr = curr-&gt;parent()) { }
-    return curr;
</del><ins>+    RenderLayer* layer = parent();
+    while (layer &amp;&amp; !layer-&gt;renderer()-&gt;isRenderView() &amp;&amp; !layer-&gt;renderer()-&gt;isRoot() &amp;&amp; layer-&gt;renderer()-&gt;style()-&gt;hasAutoZIndex())
+        layer = layer-&gt;parent();
+    return layer;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> RenderLayer* RenderLayer::enclosingPositionedAncestor() const
</span><span class="lines">@@ -770,7 +769,7 @@
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RenderLayer::destroy(RenderArena* renderArena)
</span><del>-{    
</del><ins>+{
</ins><span class="cx">     delete this;
</span><span class="cx"> 
</span><span class="cx">     // Recover the size left there for us by operator delete and free the memory.
</span><span class="lines">@@ -880,8 +879,8 @@
</span><span class="cx">         current-&gt;updateLayerPositions();
</span><span class="cx">         current = next;
</span><span class="cx">     }
</span><del>-    
-    destroy(renderer()-&gt;renderArena());
</del><ins>+
+    m_renderer-&gt;destroyLayer();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RenderLayer::insertOnlyThisLayer()
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderLayerh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderLayer.h (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderLayer.h        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderLayer.h        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -414,8 +414,6 @@
</span><span class="cx">     bool preserves3D() const { return renderer()-&gt;style()-&gt;transformStyle3D() == TransformStyle3DPreserve3D; }
</span><span class="cx">     bool has3DTransform() const { return m_transform &amp;&amp; !m_transform-&gt;isAffine(); }
</span><span class="cx"> 
</span><del>-    void destroy(RenderArena*);
-
</del><span class="cx">      // Overloaded new operator.  Derived classes must override operator new
</span><span class="cx">     // in order to allocate out of the RenderArena.
</span><span class="cx">     void* operator new(size_t, RenderArena*) throw();
</span><span class="lines">@@ -519,7 +517,11 @@
</span><span class="cx"> private:
</span><span class="cx">     friend class RenderLayerBacking;
</span><span class="cx">     friend class RenderLayerCompositor;
</span><ins>+    friend class RenderBoxModelObject;
</ins><span class="cx"> 
</span><ins>+    // Only safe to call from RenderBoxModelObject::destroyLayer(RenderArena*)
+    void destroy(RenderArena*);
+
</ins><span class="cx"> protected:
</span><span class="cx">     RenderBoxModelObject* m_renderer;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderObject.cpp (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderObject.cpp        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderObject.cpp        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -1839,10 +1839,9 @@
</span><span class="cx"> 
</span><span class="cx">     // FIXME: Would like to do this in RenderBoxModelObject, but the timing is so complicated that this can't easily
</span><span class="cx">     // be moved into RenderBoxModelObject::destroy.
</span><del>-    RenderArena* arena = renderArena();
</del><span class="cx">     if (hasLayer())
</span><del>-        toRenderBoxModelObject(this)-&gt;layer()-&gt;destroy(arena);
-    arenaDelete(arena, this);
</del><ins>+        toRenderBoxModelObject(this)-&gt;destroyLayer();
+    arenaDelete(renderArena(), this);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RenderObject::arenaDelete(RenderArena* arena, void* base)
</span></span></pre></div>
<a id="trunkWebCorerenderingRenderWidgetcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/rendering/RenderWidget.cpp (41468 => 41469)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/rendering/RenderWidget.cpp        2009-03-06 02:35:01 UTC (rev 41468)
+++ trunk/WebCore/rendering/RenderWidget.cpp        2009-03-06 02:40:36 UTC (rev 41469)
</span><span class="lines">@@ -91,19 +91,18 @@
</span><span class="cx">     if (hasOverrideSize())
</span><span class="cx">         setOverrideSize(-1);
</span><span class="cx"> 
</span><del>-    RenderArena* arena = renderArena();
-
-    if (hasLayer())
-        layer()-&gt;clearClipRects();
-
</del><span class="cx">     if (style() &amp;&amp; (style()-&gt;height().isPercent() || style()-&gt;minHeight().isPercent() || style()-&gt;maxHeight().isPercent()))
</span><span class="cx">         RenderBlock::removePercentHeightDescendant(this);
</span><span class="cx"> 
</span><del>-    setNode(0);
</del><ins>+    if (hasLayer()) {
+        layer()-&gt;clearClipRects();
+        destroyLayer();
+    }
</ins><span class="cx"> 
</span><del>-    if (hasLayer())
-        layer()-&gt;destroy(arena);
-
</del><ins>+    // Grab the arena from node()-&gt;document()-&gt;renderArena() before clearing the node pointer.
+    // Clear the node before deref-ing, as this may be deleted when deref is called.
+    RenderArena* arena = renderArena();
+    setNode(0);
</ins><span class="cx">     deref(arena);
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>