Bug 1109873 - Allow the APZC stored in the HitTestingTreeNode to be null. r=botond

2015-01-08 09:40:01 -05:00 · 2015-01-08 09:40:01 -05:00 · ceba82c3a0
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@ -207,7 +207,7 @@ APZCTreeManager::UpdateHitTestingTree(CompositorParent* aCompositor,
  for (size_t i = 0; i < state.mNodesToDestroy.Length(); i++) {
    APZCTM_LOG("Destroying node at %p with APZC %p\n",
        state.mNodesToDestroy[i].get(),
-        state.mNodesToDestroy[i]->Apzc());
+        state.mNodesToDestroy[i]->GetApzc());
    state.mNodesToDestroy[i]->Destroy();
  }
 }
@ -344,14 +344,14 @@ APZCTreeManager::PrepareNodeForLayer(const LayerMetricsWrapper& aLayer,
    // care about those nodes getting destroyed.
    for (size_t i = 0; i < aState.mNodesToDestroy.Length(); i++) {
      nsRefPtr<HitTestingTreeNode> n = aState.mNodesToDestroy[i];
-      if (n->IsPrimaryHolder() && n->Apzc()->Matches(guid)) {
+      if (n->IsPrimaryHolder() && n->GetApzc() && n->GetApzc()->Matches(guid)) {
        node = n;
        if (apzc != nullptr) {
          // If there is an APZC already then it should match the one from the
          // old primary-holder node
-          MOZ_ASSERT(apzc == node->Apzc());
+          MOZ_ASSERT(apzc == node->GetApzc());
        }
-        apzc = node->Apzc();
+        apzc = node->GetApzc();
        break;
      }
    }
@ -388,7 +388,7 @@ APZCTreeManager::PrepareNodeForLayer(const LayerMetricsWrapper& aLayer,
    // Since this is the first time we are encountering an APZC with this guid,
    // the node holding it must be the primary holder. It may be newly-created
    // or not, depending on whether it went through the newApzc branch above.
-    MOZ_ASSERT(node->IsPrimaryHolder() && node->Apzc()->Matches(guid));
+    MOZ_ASSERT(node->IsPrimaryHolder() && node->GetApzc() && node->GetApzc()->Matches(guid));

    if (!gfxPrefs::LayoutEventRegionsEnabled()) {
      nsIntRegion unobscured = ComputeTouchSensitiveRegion(state->mController, aMetrics, aObscured);
@ -478,7 +478,7 @@ APZCTreeManager::UpdateHitTestingTree(TreeBuildingState& aState,
  HitTestingTreeNode* node = PrepareNodeForLayer(aLayer,
        aLayer.Metrics(), aLayersId, aAncestorTransform,
        aObscured, aParent, aNextSibling, aState);
-  AsyncPanZoomController* apzc = (node ? node->Apzc() : nullptr);
+  AsyncPanZoomController* apzc = (node ? node->GetApzc() : nullptr);
  aLayer.SetApzc(apzc);

  mApzcTreeLog << '\n';
@ -1092,9 +1092,11 @@ APZCTreeManager::UpdateZoomConstraints(const ScrollableLayerGuid& aGuid,
                                       const ZoomConstraints& aConstraints)
 {
  nsRefPtr<HitTestingTreeNode> node = GetTargetNode(aGuid, nullptr);
+  MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
+
  // For a given layers id, non-root APZCs inherit the zoom constraints
  // of their root.
-  if (node && node->Apzc()->IsRootForLayersId()) {
+  if (node && node->GetApzc()->IsRootForLayersId()) {
    MonitorAutoLock lock(mTreeLock);
    UpdateZoomConstraintsRecursively(node.get(), aConstraints);
  }
@ -1107,13 +1109,15 @@ APZCTreeManager::UpdateZoomConstraintsRecursively(HitTestingTreeNode* aNode,
  mTreeLock.AssertCurrentThreadOwns();

  if (aNode->IsPrimaryHolder()) {
-    aNode->Apzc()->UpdateZoomConstraints(aConstraints);
+    MOZ_ASSERT(aNode->GetApzc());
+    aNode->GetApzc()->UpdateZoomConstraints(aConstraints);
  }
  for (HitTestingTreeNode* child = aNode->GetLastChild(); child; child = child->GetPrevSibling()) {
    // We can have subtrees with their own layers id - leave those alone.
-    if (!child->Apzc()->IsRootForLayersId()) {
-      UpdateZoomConstraintsRecursively(child, aConstraints);
+    if (child->GetApzc() && child->GetApzc()->IsRootForLayersId()) {
+      continue;
    }
+    UpdateZoomConstraintsRecursively(child, aConstraints);
  }
 }

@ -1124,7 +1128,8 @@ APZCTreeManager::FlushRepaintsRecursively(HitTestingTreeNode* aNode)

  for (HitTestingTreeNode* node = aNode; node; node = node->GetPrevSibling()) {
    if (node->IsPrimaryHolder()) {
-      node->Apzc()->FlushRepaintForNewInputBlock();
+      MOZ_ASSERT(node->GetApzc());
+      node->GetApzc()->FlushRepaintForNewInputBlock();
    }
    FlushRepaintsRecursively(node->GetLastChild());
  }
@ -1302,7 +1307,8 @@ APZCTreeManager::GetTargetAPZC(const ScrollableLayerGuid& aGuid,
                               GuidComparator aComparator)
 {
  nsRefPtr<HitTestingTreeNode> node = GetTargetNode(aGuid, aComparator);
-  nsRefPtr<AsyncPanZoomController> apzc = node ? node->Apzc() : nullptr;
+  MOZ_ASSERT(!node || node->GetApzc()); // any node returned must have an APZC
+  nsRefPtr<AsyncPanZoomController> apzc = node ? node->GetApzc() : nullptr;
  return apzc.forget();
 }

@ -1424,10 +1430,12 @@ APZCTreeManager::FindTargetNode(HitTestingTreeNode* aNode,
    }

    bool matches = false;
-    if (aComparator) {
-      matches = aComparator(aGuid, node->Apzc()->GetGuid());
-    } else {
-      matches = node->Apzc()->Matches(aGuid);
+    if (node->GetApzc()) {
+      if (aComparator) {
+        matches = aComparator(aGuid, node->GetApzc()->GetGuid());
+      } else {
+        matches = node->GetApzc()->Matches(aGuid);
+      }
    }
    if (matches) {
      return node;
@ -1446,7 +1454,7 @@ APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
  // This walks the tree in depth-first, reverse order, so that it encounters
  // APZCs front-to-back on the screen.
  for (HitTestingTreeNode* node = aNode; node; node = node->GetPrevSibling()) {
-    AsyncPanZoomController* apzc = node->Apzc();
+    AsyncPanZoomController* apzc = node->GetApzc();

    // The comments below assume there is a chain of layers L..R with L and P
    // having APZC instances as explained in the comment above
@ -1459,7 +1467,10 @@ APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
    // transformed layer coordinates to apzc's parent layer's layer coordinates.
    // It is PC.Inverse() * OC.Inverse() * NC.Inverse() * MC.Inverse() at recursion level for L,
    //   and RC.Inverse() * QC.Inverse()                               at recursion level for P.
-    Matrix4x4 ancestorUntransform = apzc->GetAncestorTransform().Inverse();
+    Matrix4x4 ancestorUntransform;
+    if (apzc) {
+      ancestorUntransform = apzc->GetAncestorTransform().Inverse();
+    }

    // Hit testing for this layer takes place in our parent layer coordinates,
    // since the composition bounds (used to initialize the visible rect against
@ -1473,7 +1484,11 @@ APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
    // layer coordinates to apzc's CSS-transformed layer coordinates.
    // It is PC.Inverse() * OC.Inverse() * NC.Inverse() * MC.Inverse() * LA.Inverse() at L
    //   and RC.Inverse() * QC.Inverse() * PA.Inverse()                               at P.
-    Matrix4x4 childUntransform = ancestorUntransform * Matrix4x4(apzc->GetCurrentAsyncTransform()).Inverse();
+    Matrix4x4 asyncUntransform;
+    if (apzc) {
+      asyncUntransform = Matrix4x4(apzc->GetCurrentAsyncTransform()).Inverse();
+    }
+    Matrix4x4 childUntransform = ancestorUntransform * asyncUntransform;
    Point4D hitTestPointForChildLayers = childUntransform.ProjectPoint(aHitTestPoint);
    APZCTM_LOG("Untransformed %f %f to layer coordinates %f %f for APZC %p\n",
        aHitTestPoint.x, aHitTestPoint.y,
@ -1503,14 +1518,13 @@ APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
    // If we are overscrolled, and the point matches us or one of our children,
    // the result is inside an overscrolled APZC, inform our caller of this
    // (callers typically ignore events targeted at overscrolled APZCs).
-    if (result && apzc->IsOverscrolled()) {
+    if (result && apzc && apzc->IsOverscrolled()) {
      *aOutHitResult = OverscrolledApzc;
      return nullptr;
    }

    if (result) {
-      if (!gfxPrefs::LayoutEventRegionsEnabled() && node->GetPrevSibling()
-          && result == node->GetPrevSibling()->Apzc()) {
+      if (!gfxPrefs::LayoutEventRegionsEnabled()) {
        // When event-regions are disabled, we treat scrollinfo layers as
        // regular scrollable layers. Unfortunately, their "hit region" (which
        // we create from the composition bounds) is their full area, and they
@ -1521,7 +1535,16 @@ APZCTreeManager::GetAPZCAtPoint(HitTestingTreeNode* aNode,
        // see if there are any other non-scrollinfo siblings that have children
        // that match this input. If so, they should take priority. With event-
        // regions enabled we ignore scrollinfo layers and this is unnecessary.
-        continue;
+        AsyncPanZoomController* prevSiblingApzc = nullptr;
+        for (HitTestingTreeNode* n = node->GetPrevSibling(); n; n = n->GetPrevSibling()) {
+          if (n->GetApzc()) {
+            prevSiblingApzc = n->GetApzc();
+            break;
+          }
+        }
+        if (result == prevSiblingApzc) {
+          continue;
+        }
      }

      return result;
--- a/gfx/layers/apz/src/HitTestingTreeNode.cpp
+++ b/gfx/layers/apz/src/HitTestingTreeNode.cpp
@ -21,7 +21,9 @@ HitTestingTreeNode::HitTestingTreeNode(AsyncPanZoomController* aApzc,
  : mApzc(aApzc)
  , mIsPrimaryApzcHolder(aIsPrimaryHolder)
 {
-  MOZ_ASSERT(mApzc);
+  if (mIsPrimaryApzcHolder) {
+    MOZ_ASSERT(mApzc);
+  }
 }

 HitTestingTreeNode::~HitTestingTreeNode()
@ -50,14 +52,17 @@ HitTestingTreeNode::SetLastChild(HitTestingTreeNode* aChild)
 {
  mLastChild = aChild;
  if (aChild) {
-    // We assume that HitTestingTreeNodes with an ancestor/descendant
-    // relationship cannot both point to the same APZC instance. This assertion
-    // only covers a subset of cases in which that might occur, but it's better
-    // than nothing.
-    MOZ_ASSERT(aChild->Apzc() != Apzc());
-
    aChild->mParent = this;
-    aChild->Apzc()->SetParent(Apzc());
+
+    if (aChild->GetApzc()) {
+      AsyncPanZoomController* parent = GetNearestContainingApzc();
+      // We assume that HitTestingTreeNodes with an ancestor/descendant
+      // relationship cannot both point to the same APZC instance. This
+      // assertion only covers a subset of cases in which that might occur,
+      // but it's better than nothing.
+      MOZ_ASSERT(aChild->GetApzc() != parent);
+      aChild->SetApzcParent(parent);
+    }
  }
 }

@ -67,7 +72,11 @@ HitTestingTreeNode::SetPrevSibling(HitTestingTreeNode* aSibling)
  mPrevSibling = aSibling;
  if (aSibling) {
    aSibling->mParent = mParent;
-    aSibling->Apzc()->SetParent(mParent ? mParent->Apzc() : nullptr);
+
+    if (aSibling->GetApzc()) {
+      AsyncPanZoomController* parent = mParent ? mParent->GetNearestContainingApzc() : nullptr;
+      aSibling->SetApzcParent(parent);
+    }
  }
 }

@ -75,7 +84,10 @@ void
 HitTestingTreeNode::MakeRoot()
 {
  mParent = nullptr;
-  Apzc()->SetParent(nullptr);
+
+  if (GetApzc()) {
+    SetApzcParent(nullptr);
+  }
 }

 HitTestingTreeNode*
@ -107,11 +119,22 @@ HitTestingTreeNode::GetParent() const
 }

 AsyncPanZoomController*
-HitTestingTreeNode::Apzc() const
+HitTestingTreeNode::GetApzc() const
 {
  return mApzc;
 }

+AsyncPanZoomController*
+HitTestingTreeNode::GetNearestContainingApzc() const
+{
+  for (const HitTestingTreeNode* n = this; n; n = n->GetParent()) {
+    if (n->GetApzc()) {
+      return n->GetApzc();
+    }
+  }
+  return nullptr;
+}
+
 bool
 HitTestingTreeNode::IsPrimaryHolder() const
 {
@ -148,7 +171,10 @@ HitTestingTreeNode::HitTest(const ParentLayerPoint& aPoint) const
  }

  // convert into Layer coordinate space
-  gfx::Matrix4x4 localTransform = mTransform * gfx::Matrix4x4(mApzc->GetCurrentAsyncTransform());
+  gfx::Matrix4x4 localTransform = mTransform;
+  if (mApzc) {
+    localTransform = localTransform * gfx::Matrix4x4(mApzc->GetCurrentAsyncTransform());
+  }
  gfx::Point4D pointInLayerPixels = localTransform.Inverse().ProjectPoint(aPoint.ToUnknownPoint());
  if (!pointInLayerPixels.HasPositiveWCoord()) {
    return HitTestResult::NoApzcHit;
@ -172,7 +198,7 @@ HitTestingTreeNode::Dump(const char* aPrefix) const
    mPrevSibling->Dump(aPrefix);
  }
  printf_stderr("%sHitTestingTreeNode (%p) APZC (%p) g=(%s) r=(%s) t=(%s) c=(%s)\n",
-    aPrefix, this, mApzc.get(), Stringify(mApzc->GetGuid()).c_str(),
+    aPrefix, this, mApzc.get(), mApzc ? Stringify(mApzc->GetGuid()).c_str() : "",
    Stringify(mEventRegions).c_str(), Stringify(mTransform).c_str(),
    Stringify(mClipRegion).c_str());
  if (mLastChild) {
@ -180,5 +206,17 @@ HitTestingTreeNode::Dump(const char* aPrefix) const
  }
 }

+void
+HitTestingTreeNode::SetApzcParent(AsyncPanZoomController* aParent)
+{
+  // precondition: GetApzc() is non-null
+  MOZ_ASSERT(GetApzc() != nullptr);
+  if (IsPrimaryHolder()) {
+    GetApzc()->SetParent(aParent);
+  } else {
+    MOZ_ASSERT(GetApzc()->GetParent() == aParent);
+  }
+}
+
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/src/HitTestingTreeNode.h
+++ b/gfx/layers/apz/src/HitTestingTreeNode.h
@ -34,10 +34,6 @@ class AsyncPanZoomController;
 * primary holder, only that that there will be exactly one for each APZC in
 * the tree.
 *
- * Note that every HitTestingTreeNode instance will have a pointer to an APZC,
- * and that pointer will be non-null. This will NOT be the case after the
- * HitTestingTreeNode has been Destroy()'d.
- *
 * The reason this tree exists at all is so that we can do hit-testing on the
 * thread that we receive input on (referred to the as the controller thread in
 * APZ terminology), which may be different from the compositor thread.
@ -67,7 +63,8 @@ public:
  HitTestingTreeNode* GetParent() const;

  /* APZC related methods */
-  AsyncPanZoomController* Apzc() const;
+  AsyncPanZoomController* GetApzc() const;
+  AsyncPanZoomController* GetNearestContainingApzc() const;
  bool IsPrimaryHolder() const;

  /* Hit test related methods */
@ -80,6 +77,8 @@ public:
  void Dump(const char* aPrefix = "") const;

 private:
+  void SetApzcParent(AsyncPanZoomController* aApzc);
+
  nsRefPtr<HitTestingTreeNode> mLastChild;
  nsRefPtr<HitTestingTreeNode> mPrevSibling;
  nsRefPtr<HitTestingTreeNode> mParent;
--- a/gfx/tests/gtest/TestAsyncPanZoomController.cpp
+++ b/gfx/tests/gtest/TestAsyncPanZoomController.cpp
@ -2035,15 +2035,15 @@ TEST_F(APZCTreeManagerTester, Bug1068268) {
  nsRefPtr<HitTestingTreeNode> node2 = root->GetFirstChild();
  nsRefPtr<HitTestingTreeNode> node5 = root->GetLastChild();

-  EXPECT_EQ(ApzcOf(layers[2]), node5->Apzc());
-  EXPECT_EQ(ApzcOf(layers[2]), node2->Apzc());
+  EXPECT_EQ(ApzcOf(layers[2]), node5->GetApzc());
+  EXPECT_EQ(ApzcOf(layers[2]), node2->GetApzc());
  EXPECT_EQ(ApzcOf(layers[0]), ApzcOf(layers[2])->GetParent());
  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[5]));

  EXPECT_EQ(node2->GetFirstChild(), node2->GetLastChild());
-  EXPECT_EQ(ApzcOf(layers[3]), node2->GetLastChild()->Apzc());
+  EXPECT_EQ(ApzcOf(layers[3]), node2->GetLastChild()->GetApzc());
  EXPECT_EQ(node5->GetFirstChild(), node5->GetLastChild());
-  EXPECT_EQ(ApzcOf(layers[6]), node5->GetLastChild()->Apzc());
+  EXPECT_EQ(ApzcOf(layers[6]), node5->GetLastChild()->GetApzc());
  EXPECT_EQ(ApzcOf(layers[2]), ApzcOf(layers[3])->GetParent());
  EXPECT_EQ(ApzcOf(layers[5]), ApzcOf(layers[6])->GetParent());
 }
@ -2087,7 +2087,7 @@ TEST_F(APZHitTestingTester, ComplexMultiLayerTree) {
  EXPECT_EQ(nullptr, layers4_6_8->GetParent());
  EXPECT_EQ(layers4_6_8, layer7->GetParent());
  EXPECT_EQ(nullptr, layer9->GetParent());
-  EXPECT_EQ(layer9, root->Apzc());
+  EXPECT_EQ(layer9, root->GetApzc());
  TestAsyncPanZoomController* expected[] = {
    layer9,
    layers4_6_8, layers4_6_8, layers4_6_8,
@ -2095,10 +2095,10 @@ TEST_F(APZHitTestingTester, ComplexMultiLayerTree) {
  };
  int i = 0;
  for (HitTestingTreeNode *iter = root; iter; iter = iter->GetPrevSibling()) {
-    EXPECT_EQ(expected[i++], iter->Apzc());
+    EXPECT_EQ(expected[i++], iter->GetApzc());
  }
  HitTestingTreeNode* node6 = root->GetPrevSibling()->GetPrevSibling();
-  EXPECT_EQ(layer7, node6->GetLastChild()->Apzc());
+  EXPECT_EQ(layer7, node6->GetLastChild()->GetApzc());
  EXPECT_EQ(nullptr, node6->GetLastChild()->GetPrevSibling());

  nsRefPtr<AsyncPanZoomController> hit = GetTargetAPZC(ScreenPoint(25, 25));