Bug 1068268 - Properly set the next-sibling pointer of an APZC when it is a child of a multi-layer APZ. r=botond

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -225,7 +225,7 @@ APZCTreeManager::PrepareAPZCForLayer(const LayerMetricsWrapper& aLayer,
                                      uint64_t aLayersId,
                                      const gfx::Matrix4x4& aAncestorTransform,
                                      const nsIntRegion& aObscured,
-                                     AsyncPanZoomController*& aOutParent,
+                                     AsyncPanZoomController* aParent,
                                      AsyncPanZoomController* aNextSibling,
                                      TreeBuildingState& aState)
 {
@@ -321,9 +321,10 @@ APZCTreeManager::PrepareAPZCForLayer(const LayerMetricsWrapper& aLayer,
     // Bind the APZC instance into the tree of APZCs
     if (aNextSibling) {
       aNextSibling->SetPrevSibling(apzc);
-    } else if (aOutParent) {
-      aOutParent->SetLastChild(apzc);
+    } else if (aParent) {
+      aParent->SetLastChild(apzc);
     } else {
+      MOZ_ASSERT(!mRootApzc);
       mRootApzc = apzc;
       apzc->MakeRoot();
     }
@@ -379,9 +380,6 @@ APZCTreeManager::PrepareAPZCForLayer(const LayerMetricsWrapper& aLayer,
     APZCTM_LOG("Adding region %s to visible region of APZC %p\n", Stringify(unobscured).c_str(), apzc);
   }
 
-  // Let this apzc be the parent of other controllers when we recurse downwards
-  aOutParent = apzc;
-
   return apzc;
 }
 
@@ -438,8 +436,15 @@ APZCTreeManager::UpdatePanZoomControllerTree(TreeBuildingState& aState,
   }
 
   // If there's no APZC at this level, any APZCs for our child layers will
-  // have our siblings as siblings.
-  AsyncPanZoomController* next = apzc ? nullptr : aNextSibling;
+  // have our siblings as their siblings, and our parent as their parent.
+  AsyncPanZoomController* next = aNextSibling;
+  if (apzc) {
+    // Otherwise, use this APZC as the parent going downwards, and start off
+    // with its first child as the next sibling
+    aParent = apzc;
+    next = apzc->GetFirstChild();
+  }
+
   for (LayerMetricsWrapper child = aLayer.GetLastChild(); child; child = child.GetPrevSibling()) {
     gfx::TreeAutoIndent indent(mApzcTreeLog);
     next = UpdatePanZoomControllerTree(aState, child, childLayersId,

gfx/layers/apz/src/APZCTreeManager.h

@@ -386,7 +386,7 @@ private:
                                               uint64_t aLayersId,
                                               const gfx::Matrix4x4& aAncestorTransform,
                                               const nsIntRegion& aObscured,
-                                              AsyncPanZoomController*& aOutParent,
+                                              AsyncPanZoomController* aParent,
                                               AsyncPanZoomController* aNextSibling,
                                               TreeBuildingState& aState);
 

gfx/layers/apz/src/AsyncPanZoomController.h

@@ -952,6 +952,14 @@ public:
   AsyncPanZoomController* GetPrevSibling() const { return mPrevSibling; }
   AsyncPanZoomController* GetParent() const { return mParent; }
 
+  AsyncPanZoomController* GetFirstChild() const {
+    AsyncPanZoomController* child = GetLastChild();
+    while (child && child->GetPrevSibling()) {
+      child = child->GetPrevSibling();
+    }
+    return child;
+  }
+
   /* Returns true if there is no APZC higher in the tree with the same
    * layers id.
    */

gfx/tests/gtest/TestAsyncPanZoomController.cpp

@@ -1535,6 +1535,17 @@ protected:
     };
     root = CreateLayerTree(layerTreeSyntax, layerVisibleRegion, nullptr, lm, layers);
   }
+
+  void CreatePotentiallyLeakingTree() {
+    const char* layerTreeSyntax = "c(c(c(c))c(c(c)))";
+    // LayerID                     0 1 2 3  4 5 6
+    root = CreateLayerTree(layerTreeSyntax, nullptr, nullptr, lm, layers);
+    SetScrollableFrameMetrics(layers[0], FrameMetrics::START_SCROLL_ID);
+    SetScrollableFrameMetrics(layers[2], FrameMetrics::START_SCROLL_ID + 1);
+    SetScrollableFrameMetrics(layers[5], FrameMetrics::START_SCROLL_ID + 1);
+    SetScrollableFrameMetrics(layers[3], FrameMetrics::START_SCROLL_ID + 2);
+    SetScrollableFrameMetrics(layers[6], FrameMetrics::START_SCROLL_ID + 3);
+  }
 };
 
 class APZHitTestingTester : public APZCTreeManagerTester {
@@ -1823,6 +1834,23 @@ TEST_F(APZCTreeManagerTester, ScrollableThebesLayers) {
   EXPECT_EQ(ApzcOf(layers[1]), ApzcOf(layers[2]));
 }
 
+TEST_F(APZCTreeManagerTester, Bug1068268) {
+  CreatePotentiallyLeakingTree();
+  ScopedLayerTreeRegistration registration(0, root, mcc);
+
+  manager->UpdatePanZoomControllerTree(nullptr, root, false, 0, 0);
+  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[0])->GetLastChild());
+  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[0])->GetFirstChild());
+  EXPECT_EQ(ApzcOf(layers[0]), ApzcOf(layers[2])->GetParent());
+  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[5]));
+
+  EXPECT_EQ(ApzcOf(layers[3]), ApzcOf(layers[2])->GetFirstChild());
+  EXPECT_EQ(ApzcOf(layers[6]), ApzcOf(layers[2])->GetLastChild());
+  EXPECT_EQ(ApzcOf(layers[3]), ApzcOf(layers[6])->GetPrevSibling());
+  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[3])->GetParent());
+  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[6])->GetParent());
+}
+
 TEST_F(APZHitTestingTester, ComplexMultiLayerTree) {
   CreateComplexMultiLayerTree();
   ScopedLayerTreeRegistration registration(0, root, mcc);