Merge "Fix potential use-after-free in LayerUpdateQueue"

5 files changed, 9 insertions, 8 deletions

diff --git a/libs/hwui/FrameBuilder.cpp b/libs/hwui/FrameBuilder.cpp
index 35ff635930ab..1b57e290c198 100644
--- a/libs/hwui/FrameBuilder.cpp
+++ b/libs/hwui/FrameBuilder.cpp
@@ -78,7 +78,7 @@ void FrameBuilder::deferLayers(const LayerUpdateQueue& layers) {
     // Render all layers to be updated, in order. Defer in reverse order, so that they'll be
     // updated in the order they're passed in (mLayerBuilders are issued to Renderer in reverse)
     for (int i = layers.entries().size() - 1; i >= 0; i--) {
-        RenderNode* layerNode = layers.entries()[i].renderNode;
+        RenderNode* layerNode = layers.entries()[i].renderNode.get();
         // only schedule repaint if node still on layer - possible it may have been
         // removed during a dropped frame, but layers may still remain scheduled so
         // as not to lose info on what portion is damaged
diff --git a/libs/hwui/LayerUpdateQueue.h b/libs/hwui/LayerUpdateQueue.h
index 5b1a8543dd0d..38f3596d70df 100644
--- a/libs/hwui/LayerUpdateQueue.h
+++ b/libs/hwui/LayerUpdateQueue.h
@@ -19,6 +19,7 @@
 
 #include "Rect.h"
 #include "utils/Macros.h"
+#include <utils/StrongPointer.h>
 
 #include <vector>
 #include <unordered_map>
@@ -35,7 +36,7 @@ public:
         Entry(RenderNode* renderNode, const Rect& damage)
                 : renderNode(renderNode)
                 , damage(damage) {}
-        RenderNode* renderNode;
+        sp<RenderNode> renderNode;
         Rect damage;
     };
 
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
index 430d6bea70c1..11dc1f4c2161 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
@@ -80,7 +80,7 @@ void SkiaPipeline::renderLayers(const FrameBuilder::LightGeometry& lightGeometry
 void SkiaPipeline::renderLayersImpl(const LayerUpdateQueue& layers, bool opaque) {
     // Render all layers that need to be updated, in order.
     for (size_t i = 0; i < layers.entries().size(); i++) {
-        RenderNode* layerNode = layers.entries()[i].renderNode;
+        RenderNode* layerNode = layers.entries()[i].renderNode.get();
         // only schedule repaint if node still on layer - possible it may have been
         // removed during a dropped frame, but layers may still remain scheduled so
         // as not to lose info on what portion is damaged
diff --git a/libs/hwui/tests/unit/LayerUpdateQueueTests.cpp b/libs/hwui/tests/unit/LayerUpdateQueueTests.cpp
index 4db1cb935902..91c7514e271e 100644
--- a/libs/hwui/tests/unit/LayerUpdateQueueTests.cpp
+++ b/libs/hwui/tests/unit/LayerUpdateQueueTests.cpp
@@ -48,11 +48,11 @@ TEST(LayerUpdateQueue, enqueueSimple) {
 
     EXPECT_EQ(3u, queue.entries().size());
 
-    EXPECT_EQ(a.get(), queue.entries()[0].renderNode);
+    EXPECT_EQ(a.get(), queue.entries()[0].renderNode.get());
     EXPECT_EQ(Rect(25, 25, 75, 75), queue.entries()[0].damage);
-    EXPECT_EQ(b.get(), queue.entries()[1].renderNode);
+    EXPECT_EQ(b.get(), queue.entries()[1].renderNode.get());
     EXPECT_EQ(Rect(100, 100, 200, 200), queue.entries()[1].damage); // clipped to bounds
-    EXPECT_EQ(c.get(), queue.entries()[2].renderNode);
+    EXPECT_EQ(c.get(), queue.entries()[2].renderNode.get());
     EXPECT_EQ(Rect(0, 0, 1, 1), queue.entries()[2].damage); // rounded out
 }
 
@@ -65,7 +65,7 @@ TEST(LayerUpdateQueue, enqueueUnion) {
 
     EXPECT_EQ(1u, queue.entries().size());
 
-    EXPECT_EQ(a.get(), queue.entries()[0].renderNode);
+    EXPECT_EQ(a.get(), queue.entries()[0].renderNode.get());
     EXPECT_EQ(Rect(10, 10, 40, 40), queue.entries()[0].damage);
 }
 
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp
index 2925243d9f1c..eda4a9de49ae 100644
--- a/libs/hwui/tests/unit/RenderNodeTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -331,7 +331,7 @@ RENDERTHREAD_TEST(RenderNode, prepareTree_HwLayer_AVD_enqueueDamage) {
     // damage rect.
     EXPECT_TRUE(rootNode->getDisplayList()->hasVectorDrawables());
     EXPECT_FALSE(info.layerUpdateQueue->entries().empty());
-    EXPECT_EQ(rootNode.get(), info.layerUpdateQueue->entries().at(0).renderNode);
+    EXPECT_EQ(rootNode.get(), info.layerUpdateQueue->entries().at(0).renderNode.get());
     EXPECT_EQ(uirenderer::Rect(0, 0, 200, 400), info.layerUpdateQueue->entries().at(0).damage);
     canvasContext->destroy();
 }