Deregister image client in LayoutObject::willBeDestroyed()

Source/core/layout/LayoutObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 2399 matching lines @@
         if (registry.eventHandlerTargets(EventHandlerRegistry::TouchEvent)->contains(node()))
             registry.didRemoveEventHandler(*node(), EventHandlerRegistry::TouchEvent);
     }
 
     setAncestorLineBoxDirty(false);
 
     if (selectionPaintInvalidationMap)
         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()
 {
     // FIXME: We should ASSERT(isRooted()) here but generated content makes some out-of-order insertion.
 
     // Keep our layer hierarchy updated. Optimize for the common case where we don't have any children
     // and don't have a layer attached to ourselves.
     DeprecatedPaintLayer* layer = nullptr;
     if (slowFirstChild() || hasLayer()) {
@@ skipping 101 matching lines @@
     }
 
     destroyRoot->destroy();
 
     // WARNING: |this| is deleted here.
 }
 
 void LayoutObject::destroy()
 {
     willBeDestroyed();
-    postDestroy();
+    delete this;
 }
 
 void LayoutObject::removeShapeImageClient(ShapeValue* shapeValue)
 {
     if (!shapeValue)
         return;
     if (StyleImage* shapeImage = shapeValue->image())
         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);
 }
 
 void LayoutObject::updateDragState(bool dragOn)
 {
     bool valueChanged = (dragOn != isDragging());
     setIsDragging(dragOn);
     if (valueChanged && node()) {
@@ skipping 333 matching lines @@
 bool LayoutObject::isInert() const
 {
     const LayoutObject* layoutObject = this;
     while (!layoutObject->node())
         layoutObject = layoutObject->parent();
     return layoutObject->node()->isInert();
 }
 
 void LayoutObject::imageChanged(ImageResource* image, const IntRect* rect)
 {
+    ASSERT(m_node);
     imageChanged(static_cast<WrappedImagePtr>(image), rect);
 }
 
 Element* LayoutObject::offsetParent() const
 {
     if (isDocumentElement() || isBody())
         return nullptr;
 
     if (isOutOfFlowPositioned() && style()->position() == FixedPosition)
         return nullptr;
@@ skipping 329 matching lines @@
         const blink::LayoutObject* root = object1;
         while (root->parent())
             root = root->parent();
         root->showLayoutTreeAndMark(object1, "*", object2, "-", 0);
     } else {
         fprintf(stderr, "Cannot showLayoutTree. Root is (nil)\n");
     }
 }
 
 #endif