Switch how destroyHardwareResources works

destroyHardwareResources will now only force-destroy
the specific node it was called on, which are only
ever the root nodes. Rely on onRemovedFromTree()
to clean up resources for all other nodes.

Bug: 34736819

Test: RenderNode.multiTreeValidity passes, manually
verified fixes b/34736819

Change-Id: I1c275ad6a98b63bf50f265602f09bffe3e1f169b

4 files changed, 114 insertions, 14 deletions

diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index 7d8f0462920a..f1c8232760e1 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -406,21 +406,13 @@ void RenderNode::deleteDisplayList(TreeObserver& observer, TreeInfo* info) {
 }
 
 void RenderNode::destroyHardwareResources(TreeInfo* info) {
-    ImmediateRemoved observer(info);
-    destroyHardwareResourcesImpl(observer, info);
-}
-
-void RenderNode::destroyHardwareResourcesImpl(TreeObserver& observer, TreeInfo* info) {
     if (hasLayer()) {
         renderthread::CanvasContext::destroyLayer(this);
     }
-    if (mDisplayList) {
-        mDisplayList->updateChildren([&observer, info](RenderNode* child) {
-            child->destroyHardwareResourcesImpl(observer, info);
-        });
-        setStagingDisplayList(nullptr);
-        deleteDisplayList(observer, info);
-    }
+    setStagingDisplayList(nullptr);
+
+    ImmediateRemoved observer(info);
+    deleteDisplayList(observer, info);
 }
 
 void RenderNode::destroyLayers() {
diff --git a/libs/hwui/RenderNode.h b/libs/hwui/RenderNode.h
index 14a664c6a364..a971e835e604 100644
--- a/libs/hwui/RenderNode.h
+++ b/libs/hwui/RenderNode.h
@@ -262,7 +262,6 @@ private:
     void prepareLayer(TreeInfo& info, uint32_t dirtyMask);
     void pushLayerUpdate(TreeInfo& info);
     void deleteDisplayList(TreeObserver& observer, TreeInfo* info = nullptr);
-    void destroyHardwareResourcesImpl(TreeObserver& observer, TreeInfo* info = nullptr);
     void damageSelf(TreeInfo& info);
 
     void incParentRefCount() { mParentCount++; }
diff --git a/libs/hwui/tests/common/TestUtils.h b/libs/hwui/tests/common/TestUtils.h
index 16bc6f7825b3..0c7bb6079fea 100644
--- a/libs/hwui/tests/common/TestUtils.h
+++ b/libs/hwui/tests/common/TestUtils.h
@@ -357,7 +357,10 @@ private:
     static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {
         MarkAndSweepRemoved observer(nullptr);
         node->syncProperties();
-        node->syncDisplayList(observer, nullptr);
+        if (node->mNeedsDisplayListSync) {
+            node->mNeedsDisplayListSync = false;
+            node->syncDisplayList(observer, nullptr);
+        }
         auto displayList = node->getDisplayList();
         if (displayList) {
             for (auto&& childOp : displayList->getChildren()) {
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp
index 8af4bbe28673..2925243d9f1c 100644
--- a/libs/hwui/tests/unit/RenderNodeTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -130,6 +130,112 @@ TEST(RenderNode, validity) {
     EXPECT_TRUE(parent->nothingToDraw());
 }
 
+TEST(RenderNode, multiTreeValidity) {
+    auto child = TestUtils::createNode(0, 0, 200, 400,
+            [](RenderProperties& props, Canvas& canvas) {
+        canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver);
+    });
+    auto parent1 = TestUtils::createNode(0, 0, 200, 400,
+            [&child](RenderProperties& props, Canvas& canvas) {
+        canvas.drawRenderNode(child.get());
+    });
+    auto parent2 = TestUtils::createNode(0, 0, 200, 400,
+            [&child](RenderProperties& props, Canvas& canvas) {
+        canvas.drawRenderNode(child.get());
+    });
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_TRUE(child->nothingToDraw());
+    EXPECT_TRUE(parent1->nothingToDraw());
+    EXPECT_TRUE(parent2->nothingToDraw());
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_FALSE(child->nothingToDraw());
+    EXPECT_FALSE(parent1->nothingToDraw());
+    EXPECT_TRUE(parent2->nothingToDraw());
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_FALSE(child->nothingToDraw());
+    EXPECT_FALSE(parent1->nothingToDraw());
+    EXPECT_FALSE(parent2->nothingToDraw());
+
+    TestUtils::recordNode(*parent1, [](Canvas& canvas) {
+        canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);
+    });
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_FALSE(child->nothingToDraw());
+    EXPECT_FALSE(parent1->nothingToDraw());
+    EXPECT_FALSE(parent2->nothingToDraw());
+
+    TestUtils::recordNode(*parent2, [](Canvas& canvas) {
+        canvas.drawColor(Color::Amber_500, SkBlendMode::kSrcOver);
+    });
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);
+
+    EXPECT_FALSE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_TRUE(child->nothingToDraw());
+    EXPECT_FALSE(parent1->nothingToDraw());
+    EXPECT_FALSE(parent2->nothingToDraw());
+
+    TestUtils::recordNode(*child, [](Canvas& canvas) {
+        canvas.drawColor(Color::Red_500, SkBlendMode::kSrcOver);
+    });
+    TestUtils::syncHierarchyPropertiesAndDisplayList(child);
+
+    TestUtils::recordNode(*parent1, [&child](Canvas& canvas) {
+        canvas.drawRenderNode(child.get());
+    });
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent1);
+
+    TestUtils::recordNode(*parent2, [&child](Canvas& canvas) {
+        canvas.drawRenderNode(child.get());
+    });
+    TestUtils::syncHierarchyPropertiesAndDisplayList(parent2);
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_TRUE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_FALSE(child->nothingToDraw());
+    EXPECT_FALSE(parent1->nothingToDraw());
+    EXPECT_FALSE(parent2->nothingToDraw());
+
+    parent1->destroyHardwareResources();
+
+    EXPECT_TRUE(child->isValid());
+    EXPECT_FALSE(parent1->isValid());
+    EXPECT_TRUE(parent2->isValid());
+    EXPECT_FALSE(child->nothingToDraw());
+    EXPECT_TRUE(parent1->nothingToDraw());
+    EXPECT_FALSE(parent2->nothingToDraw());
+
+    parent2->destroyHardwareResources();
+
+    EXPECT_FALSE(child->isValid());
+    EXPECT_FALSE(parent1->isValid());
+    EXPECT_FALSE(parent2->isValid());
+    EXPECT_TRUE(child->nothingToDraw());
+    EXPECT_TRUE(parent1->nothingToDraw());
+    EXPECT_TRUE(parent2->nothingToDraw());
+}
+
 TEST(RenderNode, releasedCallback) {
     class DecRefOnReleased : public GlFunctorLifecycleListener {
     public: