Fix layers lifecycle issues

 Bug: 16118540

 Fix an issue where we could have a reference to a Layer after
 the GL context was destroyed

Change-Id: I7bfd909d735ca6b942ebe188fc10099422eb6d95

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index f1a0913..1e5f448 100644 (file)
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -13625,9 +13625,11 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
      * @hide
      */
     protected void destroyHardwareResources() {
-        // Intentionally empty. RenderNode's lifecycle is now fully managed
-        // by the hardware renderer.
-        // However some subclasses (eg, WebView, TextureView) still need this signal
+        // Although the Layer will be destroyed by RenderNode, we want to release
+        // the staging display list, which is also a signal to RenderNode that it's
+        // safe to free its copy of the display list as it knows that we will
+        // push an updated DisplayList if we try to draw again
+        resetDisplayList();
     }
 
     /**

diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index 89105ea..fe03806 100644 (file)
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -63,11 +63,12 @@ RenderNode::RenderNode()
         , mDisplayListData(0)
         , mStagingDisplayListData(0)
         , mAnimatorManager(*this)
-        , mLayer(0) {
+        , mLayer(0)
+        , mParentCount(0) {
 }
 
 RenderNode::~RenderNode() {
-    delete mDisplayListData;
+    deleteDisplayListData();
     delete mStagingDisplayListData;
     LayerRenderer::destroyLayerDeferred(mLayer);
 }
@@ -196,27 +197,12 @@ void RenderNode::pushLayerUpdate(TreeInfo& info) {
 void RenderNode::prepareTreeImpl(TreeInfo& info) {
     info.damageAccumulator->pushTransform(this);
 
-    switch (info.mode) {
-    case TreeInfo::MODE_FULL:
-        pushStagingPropertiesChanges(info);
-        mAnimatorManager.animate(info);
-        break;
-    case TreeInfo::MODE_MAYBE_DETACHING:
+    if (info.mode == TreeInfo::MODE_FULL) {
         pushStagingPropertiesChanges(info);
-        break;
-    case TreeInfo::MODE_RT_ONLY:
-        mAnimatorManager.animate(info);
-        break;
-    case TreeInfo::MODE_DESTROY_RESOURCES:
-        // This will also release the hardware layer if we have one as
-        // isRenderable() will return false, thus causing pushLayerUpdate
-        // to recycle the hardware layer
-        setStagingDisplayList(NULL);
-        break;
     }
-
+    mAnimatorManager.animate(info);
     prepareLayer(info);
-    if (info.mode == TreeInfo::MODE_FULL || info.mode == TreeInfo::MODE_DESTROY_RESOURCES) {
+    if (info.mode == TreeInfo::MODE_FULL) {
         pushStagingDisplayListChanges(info);
     }
     prepareSubTree(info, mDisplayListData);
@@ -258,17 +244,30 @@ void RenderNode::applyLayerPropertiesToLayer(TreeInfo& info) {
 void RenderNode::pushStagingDisplayListChanges(TreeInfo& info) {
     if (mNeedsDisplayListDataSync) {
         mNeedsDisplayListDataSync = false;
-        // Do a push pass on the old tree to handle freeing DisplayListData
-        // that are no longer used
-        TreeInfo oldTreeInfo(TreeInfo::MODE_MAYBE_DETACHING, info);
-        prepareSubTree(oldTreeInfo, mDisplayListData);
-        delete mDisplayListData;
+        // Make sure we inc first so that we don't fluctuate between 0 and 1,
+        // which would thrash the layer cache
+        if (mStagingDisplayListData) {
+            for (size_t i = 0; i < mStagingDisplayListData->children().size(); i++) {
+                mStagingDisplayListData->children()[i]->mRenderNode->incParentRefCount();
+            }
+        }
+        deleteDisplayListData();
         mDisplayListData = mStagingDisplayListData;
-        mStagingDisplayListData = 0;
+        mStagingDisplayListData = NULL;
         damageSelf(info);
     }
 }
 
+void RenderNode::deleteDisplayListData() {
+    if (mDisplayListData) {
+        for (size_t i = 0; i < mDisplayListData->children().size(); i++) {
+            mDisplayListData->children()[i]->mRenderNode->decParentRefCount();
+        }
+    }
+    delete mDisplayListData;
+    mDisplayListData = NULL;
+}
+
 void RenderNode::prepareSubTree(TreeInfo& info, DisplayListData* subtree) {
     if (subtree) {
         TextureCache& cache = Caches::getInstance().textureCache;
@@ -291,6 +290,35 @@ void RenderNode::prepareSubTree(TreeInfo& info, DisplayListData* subtree) {
     }
 }
 
+void RenderNode::destroyHardwareResources() {
+    if (mLayer) {
+        LayerRenderer::destroyLayer(mLayer);
+        mLayer = NULL;
+    }
+    if (mDisplayListData) {
+        for (size_t i = 0; i < mDisplayListData->children().size(); i++) {
+            mDisplayListData->children()[i]->mRenderNode->destroyHardwareResources();
+        }
+        if (mNeedsDisplayListDataSync) {
+            // Next prepare tree we are going to push a new display list, so we can
+            // drop our current one now
+            deleteDisplayListData();
+        }
+    }
+}
+
+void RenderNode::decParentRefCount() {
+    LOG_ALWAYS_FATAL_IF(!mParentCount, "already 0!");
+    mParentCount--;
+    if (!mParentCount) {
+        // If a child of ours is being attached to our parent then this will incorrectly
+        // destroy its hardware resources. However, this situation is highly unlikely
+        // and the failure is "just" that the layer is re-created, so this should
+        // be safe enough
+        destroyHardwareResources();
+    }
+}
+
 /*
  * For property operations, we pass a savecount of 0, since the operations aren't part of the
  * displaylist, and thus don't have to compensate for the record-time/playback-time discrepancy in

diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h
index 7d42b59..54fa143 100644 (file)
--- a/libs/hwui/RenderNode.h
+++ b/libs/hwui/RenderNode.h
@@ -168,6 +168,7 @@ public:
     }
 
     ANDROID_API virtual void prepareTree(TreeInfo& info);
+    void destroyHardwareResources();
 
     // UI thread only!
     ANDROID_API void addAnimator(const sp<BaseRenderNodeAnimator>& animator);
@@ -248,6 +249,10 @@ private:
     void applyLayerPropertiesToLayer(TreeInfo& info);
     void prepareLayer(TreeInfo& info);
     void pushLayerUpdate(TreeInfo& info);
+    void deleteDisplayListData();
+
+    void incParentRefCount() { mParentCount++; }
+    void decParentRefCount();
 
     String8 mName;
 
@@ -256,6 +261,7 @@ private:
     RenderProperties mStagingProperties;
 
     bool mNeedsDisplayListDataSync;
+    // WARNING: Do not delete this directly, you must go through deleteDisplayListData()!
     DisplayListData* mDisplayListData;
     DisplayListData* mStagingDisplayListData;
 
@@ -272,6 +278,14 @@ private:
 
     // for projection surfaces, contains a list of all children items
     Vector<DrawRenderNodeOp*> mProjectedNodes;
+
+    // How many references our parent(s) have to us. Typically this should alternate
+    // between 2 and 1 (when a staging push happens we inc first then dec)
+    // When this hits 0 we are no longer in the tree, so any hardware resources
+    // (specifically Layers) should be released.
+    // This is *NOT* thread-safe, and should therefore only be tracking
+    // mDisplayListData, not mStagingDisplayListData.
+    uint32_t mParentCount;
 }; // class RenderNode
 
 } /* namespace uirenderer */

diff --git a/libs/hwui/TreeInfo.h b/libs/hwui/TreeInfo.h
index 9746ac5..de09755 100644 (file)
--- a/libs/hwui/TreeInfo.h
+++ b/libs/hwui/TreeInfo.h
@@ -58,15 +58,6 @@ public:
         // animators, but potentially things like SurfaceTexture updates
         // could be handled by this as well if there are no listeners
         MODE_RT_ONLY,
-        // The subtree is being detached. Maybe. If the RenderNode is present
-        // in both the old and new display list's children then it will get a
-        // MODE_MAYBE_DETACHING followed shortly by a MODE_FULL.
-        // Push any pending display list changes in case it is detached,
-        // but don't evaluate animators and such as if it isn't detached as a
-        // MODE_FULL will follow shortly.
-        MODE_MAYBE_DETACHING,
-        // Destroy all hardware resources, including DisplayListData, in the tree.
-        MODE_DESTROY_RESOURCES,
     };
 
     explicit TreeInfo(TraversalMode mode, RenderState& renderState)

diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index f5d4f8b..57279b7 100644 (file)
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -250,8 +250,7 @@ void CanvasContext::destroyHardwareResources() {
     stopDrawing();
     if (mEglManager.hasEglContext()) {
         requireGlContext();
-        TreeInfo info(TreeInfo::MODE_DESTROY_RESOURCES, mRenderThread.renderState());
-        mRootRenderNode->prepareTree(info);
+        mRootRenderNode->destroyHardwareResources();
         Caches::getInstance().flush(Caches::kFlushMode_Layers);
     }
 }