Deregister image client in LayoutObject::willBeDestroyed()

Source/core/layout/LayoutObject.cpp

Index: Source/core/layout/LayoutObject.cpp
diff --git a/Source/core/layout/LayoutObject.cpp b/Source/core/layout/LayoutObject.cpp
index e96113c8527123bac9e23bd9410e148474f9f1a9..a7b8e682b5abcdea7f19cc08ee328d0ce6aa8524 100644
--- a/Source/core/layout/LayoutObject.cpp
+++ b/Source/core/layout/LayoutObject.cpp
@@ -2417,6 +2417,27 @@ void LayoutObject::willBeDestroyed()
         selectionPaintInvalidationMap->remove(this);
 
     clearLayoutRootIfNeeded();
+
+    if (m_style) {
+        for (const FillLayer* bgLayer = &m_style->backgroundLayers(); bgLayer; bgLayer = bgLayer->next()) {
+            if (StyleImage* backgroundImage = bgLayer->image())
+                backgroundImage->removeClient(this);
+        }
+
+        for (const FillLayer* maskLayer = &m_style->maskLayers(); maskLayer; maskLayer = maskLayer->next()) {
+            if (StyleImage* maskImage = maskLayer->image())
+                maskImage->removeClient(this);
+        }
+
+        if (StyleImage* borderImage = m_style->borderImage().image())
+            borderImage->removeClient(this);
+
+        if (StyleImage* maskBoxImage = m_style->maskBoxImage().image())
+            maskBoxImage->removeClient(this);
+
+        removeShapeImageClient(m_style->shapeOutside());
+    }
+    ResourceLoadPriorityOptimizer::resourceLoadPriorityOptimizer()->removeLayoutObject(this);
 }
 
 void LayoutObject::insertedIntoTree()
@@ -2538,7 +2559,7 @@ void LayoutObject::destroyAndCleanupAnonymousWrappers()
 void LayoutObject::destroy()
 {
     willBeDestroyed();
-    postDestroy();
+    delete this;
 }
 
 void LayoutObject::removeShapeImageClient(ShapeValue* shapeValue)
@@ -2549,32 +2570,6 @@ void LayoutObject::removeShapeImageClient(ShapeValue* shapeValue)
         shapeImage->removeClient(this);
 }
 
-void LayoutObject::postDestroy()
-{
-    // It seems ugly that this is not in willBeDestroyed().

[trchen, 2015/06/16 04:05:55]

Hello cevans@, I noticed you added this comment in
https://codereview.chromium.org/20231002 in 2013. Do you still remember the
reason why this can't be done in LayoutObject::willBeDestroyed()?

I looked at the revision history of your CL. My best guess is that you were
trying to minimize behavioral difference?

Thanks in advance for your insights. :)

FYI, I made a recent change that removed the if(parent())return; check from
LayoutBox::imageChanged, and got this crash I described in CL comments. IMO
instead of adding back the superfluous check we should clean this up (if we
can).

-    if (m_style) {
-        for (const FillLayer* bgLayer = &m_style->backgroundLayers(); bgLayer; bgLayer = bgLayer->next()) {
-            if (StyleImage* backgroundImage = bgLayer->image())
-                backgroundImage->removeClient(this);
-        }
-
-        for (const FillLayer* maskLayer = &m_style->maskLayers(); maskLayer; maskLayer = maskLayer->next()) {
-            if (StyleImage* maskImage = maskLayer->image())
-                maskImage->removeClient(this);
-        }
-
-        if (StyleImage* borderImage = m_style->borderImage().image())
-            borderImage->removeClient(this);
-
-        if (StyleImage* maskBoxImage = m_style->maskBoxImage().image())
-            maskBoxImage->removeClient(this);
-
-        removeShapeImageClient(m_style->shapeOutside());
-    }
-    ResourceLoadPriorityOptimizer::resourceLoadPriorityOptimizer()->removeLayoutObject(this);
-    delete this;
-}
-
 PositionWithAffinity LayoutObject::positionForPoint(const LayoutPoint&)
 {
     return createPositionWithAffinity(caretMinOffset(), DOWNSTREAM);
@@ -2928,6 +2923,7 @@ bool LayoutObject::isInert() const
 
 void LayoutObject::imageChanged(ImageResource* image, const IntRect* rect)
 {
+    ASSERT(m_node);
     imageChanged(static_cast<WrappedImagePtr>(image), rect);
 }