Oilpan: Move more code of RenderObject destructors to destroy().

Source/core/rendering/RenderBlock.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 198 matching lines @@
         appendImageIfNotNull(images, toImageContentData(contentData)->image());
     if (blockStyle.boxReflect())
         appendImageIfNotNull(images, blockStyle.boxReflect()->mask().image());
     appendImageIfNotNull(images, blockStyle.listStyleImage());
     appendImageIfNotNull(images, blockStyle.borderImageSource());
     appendImageIfNotNull(images, blockStyle.maskBoxImageSource());
     if (blockStyle.shapeOutside())
         appendImageIfNotNull(images, blockStyle.shapeOutside()->image());
 }
 
-RenderBlock::~RenderBlock()
+void RenderBlock::removeFromGlobalMaps()
 {
     if (hasColumns())
         gColumnInfoMap->take(this);
     if (gPercentHeightDescendantsMap)
         removeBlockFromDescendantAndContainerMaps(this, gPercentHeightDescendantsMap, gPercentHeightContainerMap);
     if (gPositionedDescendantsMap)
         removeBlockFromDescendantAndContainerMaps(this, gPositionedDescendantsMap, gPositionedContainerMap);
 }
 
+RenderBlock::~RenderBlock()
+{
+#if !ENABLE(OILPAN)
+    removeFromGlobalMaps();

[haraken, 2014/08/12 08:08:15]

Can't we call removeFromGlobalMaps() in destroy() in non-oilpan builds as well?
We don't want to have different call paths between oilpan builds and non-oilpan
builds if possible.

[tkent, 2014/08/12 08:37:51]

We can't.  |this| is deleted in destroy().

+#endif
+}
+
+void RenderBlock::destroy()
+{
+    RenderBox::destroy();

[haraken, 2014/08/12 08:08:15]

Nit: Make the destroy call to the parent class a tail call (just for consistency
with other destroy methods).

[tkent, 2014/08/12 08:37:51]

We can't do it.  destroy() depends on the global maps.

[haraken, 2014/08/12 08:47:00]

I understand what's going on. Then it seems to me that it would be better to
keep removeFromGlobalMaps() in ~RenderBlock(), because conceptually
removeFromGlobalMaps() must be called after the RenderBlock becomes unreachable.

With your CL, once we have the following code, we'll hit a crash:

class RenderSomething : public RenderBlock {
  void destroy() {
    RenderBlock::destroy();
    doSomething();
  }
};

(In the future, we should replace those global maps with weak maps of
RenderObjects.)

+#if ENABLE(OILPAN)
+    // RenderObject::removeChild called in destory() depends on gColumnInfoMap.
+    removeFromGlobalMaps();
+#endif
+}
+
 void RenderBlock::willBeDestroyed()
 {
     // Mark as being destroyed to avoid trouble with merges in removeChild().
     m_beingDestroyed = true;
 
     // Make sure to destroy anonymous children first while they are still connected to the rest of the tree, so that they will
     // properly dirty line boxes that they are removed from. Effects that do :before/:after only on hover could crash otherwise.
     children()->destroyLeftoverChildren();
 
     // Destroy our continuation before anything other than anonymous children.
@@ skipping 4594 matching lines @@
 void RenderBlock::showLineTreeAndMark(const InlineBox* markedBox1, const char* markedLabel1, const InlineBox* markedBox2, const char* markedLabel2, const RenderObject* obj) const
 {
     showRenderObject();
     for (const RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox())
         root->showLineTreeAndMark(markedBox1, markedLabel1, markedBox2, markedLabel2, obj, 1);
 }
 
 #endif
 
 } // namespace blink