Bug 1052063 - Prevent creating multiple APZC instances for the same ScrollableLayerGuid. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 19 Aug 2014 21:17:08 -0400
changeset 214808 c4402de567322182d2e7691689d3794110c11168
parent 214807 13dae58b4a5e31f024d597152d916f96559d536f
child 214809 073b4e162b5da8071bff284139021dd174b0a403
push idunknown
push userunknown
push dateunknown
reviewersbotond
bugs1052063
milestone34.0a1
Bug 1052063 - Prevent creating multiple APZC instances for the same ScrollableLayerGuid. r=botond
gfx/layers/FrameMetrics.h
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -558,16 +558,32 @@ struct ScrollableLayerGuid {
         && mPresShellId == other.mPresShellId
         && mScrollId == other.mScrollId;
   }
 
   bool operator!=(const ScrollableLayerGuid& other) const
   {
     return !(*this == other);
   }
+
+  bool operator<(const ScrollableLayerGuid& other) const
+  {
+    if (mLayersId < other.mLayersId) {
+      return true;
+    }
+    if (mLayersId == other.mLayersId) {
+      if (mPresShellId < other.mPresShellId) {
+        return true;
+      }
+      if (mPresShellId == other.mPresShellId) {
+        return mScrollId < other.mScrollId;
+      }
+    }
+    return false;
+  }
 };
 
 template <int LogLevel>
 gfx::Log<LogLevel>& operator<<(gfx::Log<LogLevel>& log, const ScrollableLayerGuid& aGuid) {
   return log << '(' << aGuid.mLayersId << ',' << aGuid.mPresShellId << ',' << aGuid.mScrollId << ')';
 }
 
 struct ZoomConstraints {
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -131,23 +131,25 @@ APZCTreeManager::UpdatePanZoomController
       testData = &state->mApzTestData;
       testData->StartNewPaint(aPaintSequenceNumber);
     }
   }
   APZPaintLogHelper paintLogger(testData, aPaintSequenceNumber);
 
   if (aRoot) {
     mApzcTreeLog << "[start]\n";
+    std::map<ScrollableLayerGuid, AsyncPanZoomController*> apzcMap;
     UpdatePanZoomControllerTree(aCompositor,
                                 aRoot,
                                 // aCompositor is null in gtest scenarios
                                 aCompositor ? aCompositor->RootLayerTreeId() : 0,
                                 Matrix4x4(), nullptr, nullptr,
                                 aIsFirstPaint, aOriginatingLayersId,
-                                paintLogger, &apzcsToDestroy, nsIntRegion());
+                                paintLogger, &apzcsToDestroy, apzcMap,
+                                nsIntRegion());
     mApzcTreeLog << "[end]\n";
   }
 
   for (size_t i = 0; i < apzcsToDestroy.Length(); i++) {
     APZCTM_LOG("Destroying APZC at %p\n", apzcsToDestroy[i].get());
     apzcsToDestroy[i]->Destroy();
   }
 }
@@ -157,16 +159,17 @@ APZCTreeManager::UpdatePanZoomController
                                              Layer* aLayer, uint64_t aLayersId,
                                              Matrix4x4 aTransform,
                                              AsyncPanZoomController* aParent,
                                              AsyncPanZoomController* aNextSibling,
                                              bool aIsFirstPaint,
                                              uint64_t aOriginatingLayersId,
                                              const APZPaintLogHelper& aPaintLogger,
                                              nsTArray< nsRefPtr<AsyncPanZoomController> >* aApzcsToDestroy,
+                                             std::map<ScrollableLayerGuid, AsyncPanZoomController*>& aApzcMap,
                                              const nsIntRegion& aObscured)
 {
   mTreeLock.AssertCurrentThreadOwns();
 
   Matrix4x4 transform = aLayer->GetTransform();
 
   AsyncPanZoomController* apzc = nullptr;
   mApzcTreeLog << aLayer->Name() << '\t';
@@ -174,146 +177,163 @@ APZCTreeManager::UpdatePanZoomController
   const FrameMetrics& metrics = aLayer->GetFrameMetrics();
   if (metrics.IsScrollable()) {
     const CompositorParent::LayerTreeState* state = CompositorParent::GetIndirectShadowTree(aLayersId);
     if (state && state->mController.get()) {
       // If we get here, aLayer is a scrollable layer and somebody
       // has registered a GeckoContentController for it, so we need to ensure
       // it has an APZC instance to manage its scrolling.
 
-      apzc = aLayer->GetAsyncPanZoomController();
+      // aApzcMap allows reusing the exact same APZC instance for different layers
+      // with the same FrameMetrics data. This is needed because in some cases content
+      // that is supposed to scroll together is split into multiple layers because of
+      // e.g. non-scrolling content interleaved in z-index order.
+      ScrollableLayerGuid guid(aLayersId, metrics);
+      auto insertResult = aApzcMap.insert(std::make_pair(guid, static_cast<AsyncPanZoomController*>(nullptr)));
+      if (!insertResult.second) {
+        apzc = insertResult.first->second;
+      }
+      APZCTM_LOG("Found APZC %p for layer %p with identifiers %lld %lld\n", apzc, aLayer, guid.mLayersId, guid.mScrollId);
 
-      // If the content represented by the scrollable layer has changed (which may
-      // be possible because of DLBI heuristics) then we don't want to keep using
-      // the same old APZC for the new content. Null it out so we run through the
-      // code to find another one or create one.
-      ScrollableLayerGuid guid(aLayersId, metrics);
-      if (apzc && !apzc->Matches(guid)) {
-        apzc = nullptr;
-      }
+      // If we haven't encountered a layer already with the same metrics, then we need to
+      // do the full reuse-or-make-an-APZC algorithm, which is contained inside the block
+      // below.
+      if (apzc == nullptr) {
+        apzc = aLayer->GetAsyncPanZoomController();
 
-      // If the layer doesn't have an APZC already, try to find one of our
-      // pre-existing ones that matches. In particular, if we find an APZC whose
-      // ScrollableLayerGuid is the same, then we know what happened is that the
-      // layout of the page changed causing the layer tree to be rebuilt, but the
-      // underlying content for which the APZC was originally created is still
-      // there. So it makes sense to pick up that APZC instance again and use it here.
-      if (apzc == nullptr) {
-        for (size_t i = 0; i < aApzcsToDestroy->Length(); i++) {
-          if (aApzcsToDestroy->ElementAt(i)->Matches(guid)) {
-            apzc = aApzcsToDestroy->ElementAt(i);
-            break;
+        // If the content represented by the scrollable layer has changed (which may
+        // be possible because of DLBI heuristics) then we don't want to keep using
+        // the same old APZC for the new content. Null it out so we run through the
+        // code to find another one or create one.
+        if (apzc && !apzc->Matches(guid)) {
+          apzc = nullptr;
+        }
+
+        // If the layer doesn't have an APZC already, try to find one of our
+        // pre-existing ones that matches. In particular, if we find an APZC whose
+        // ScrollableLayerGuid is the same, then we know what happened is that the
+        // layout of the page changed causing the layer tree to be rebuilt, but the
+        // underlying content for which the APZC was originally created is still
+        // there. So it makes sense to pick up that APZC instance again and use it here.
+        if (apzc == nullptr) {
+          for (size_t i = 0; i < aApzcsToDestroy->Length(); i++) {
+            if (aApzcsToDestroy->ElementAt(i)->Matches(guid)) {
+              apzc = aApzcsToDestroy->ElementAt(i);
+              break;
+            }
           }
         }
-      }
 
-      // The APZC we get off the layer may have been destroyed previously if the layer was inactive
-      // or omitted from the layer tree for whatever reason from a layers update. If it later comes
-      // back it will have a reference to a destroyed APZC and so we need to throw that out and make
-      // a new one.
-      bool newApzc = (apzc == nullptr || apzc->IsDestroyed());
-      if (newApzc) {
-        apzc = new AsyncPanZoomController(aLayersId, this, state->mController,
-                                          AsyncPanZoomController::USE_GESTURE_DETECTOR);
-        apzc->SetCompositorParent(aCompositor);
-        if (state->mCrossProcessParent != nullptr) {
-          apzc->ShareFrameMetricsAcrossProcesses();
-        }
-      } else {
-        // If there was already an APZC for the layer clear the tree pointers
-        // so that it doesn't continue pointing to APZCs that should no longer
-        // be in the tree. These pointers will get reset properly as we continue
-        // building the tree. Also remove it from the set of APZCs that are going
-        // to be destroyed, because it's going to remain active.
-        aApzcsToDestroy->RemoveElement(apzc);
-        apzc->SetPrevSibling(nullptr);
-        apzc->SetLastChild(nullptr);
-      }
-      APZCTM_LOG("Using APZC %p for layer %p with identifiers %lld %lld\n", apzc, aLayer, aLayersId, metrics.GetScrollId());
-
-      apzc->NotifyLayersUpdated(metrics,
-                                aIsFirstPaint && (aLayersId == aOriginatingLayersId));
-      apzc->SetScrollHandoffParentId(aLayer->GetScrollHandoffParentId());
-
-      // Use the composition bounds as the hit test region.
-      // Optionally, the GeckoContentController can provide a touch-sensitive
-      // region that constrains all frames associated with the controller.
-      // In this case we intersect the composition bounds with that region.
-      ParentLayerRect visible(metrics.mCompositionBounds);
-      CSSRect touchSensitiveRegion;
-      if (state->mController->GetTouchSensitiveRegion(&touchSensitiveRegion)) {
-        // Note: we assume here that touchSensitiveRegion is in the CSS pixels
-        // of our parent layer, which makes this coordinate conversion
-        // correct.
-        visible = visible.Intersect(touchSensitiveRegion
-                                    * metrics.mDevPixelsPerCSSPixel
-                                    * metrics.GetParentResolution());
-      }
-
-      // Not sure what rounding option is the most correct here, but if we ever
-      // figure it out we can change this. For now I'm rounding in to minimize
-      // the chances of getting a complex region.
-      ParentLayerIntRect roundedVisible = RoundedIn(visible);
-      nsIntRegion unobscured;
-      unobscured.Sub(nsIntRect(roundedVisible.x, roundedVisible.y,
-                               roundedVisible.width, roundedVisible.height),
-                     aObscured);
-
-      apzc->SetLayerHitTestData(unobscured, aTransform, transform);
-      APZCTM_LOG("Setting rect(%f %f %f %f) as visible region for APZC %p\n", visible.x, visible.y,
-                                                                            visible.width, visible.height,
-                                                                            apzc);
-
-      mApzcTreeLog << "APZC " << guid
-                   << "\tcb=" << visible
-                   << "\tsr=" << metrics.mScrollableRect
-                   << (aLayer->GetVisibleRegion().IsEmpty() ? "\tscrollinfo" : "")
-                   << (apzc->HasScrollgrab() ? "\tscrollgrab" : "")
-                   << "\t" << aLayer->GetContentDescription();
-
-      // Bind the APZC instance into the tree of APZCs
-      if (aNextSibling) {
-        aNextSibling->SetPrevSibling(apzc);
-      } else if (aParent) {
-        aParent->SetLastChild(apzc);
-      } else {
-        mRootApzc = apzc;
-        apzc->MakeRoot();
-      }
-
-      // For testing, log the parent scroll id of every APZC that has a
-      // parent. This allows test code to reconstruct the APZC tree.
-      // Note that we currently only do this for APZCs in the layer tree
-      // that originated the update, because the only identifying information
-      // we are logging about APZCs is the scroll id, and otherwise we could
-      // confuse APZCs from different layer trees with the same scroll id.
-      if (aLayersId == aOriginatingLayersId && apzc->GetParent()) {
-        aPaintLogger.LogTestData(metrics.GetScrollId(), "parentScrollId",
-            apzc->GetParent()->GetGuid().mScrollId);
-      }
-
-      // Let this apzc be the parent of other controllers when we recurse downwards
-      aParent = apzc;
-
-      if (newApzc) {
-        if (apzc->IsRootForLayersId()) {
-          // If we just created a new apzc that is the root for its layers ID, then
-          // we need to update its zoom constraints which might have arrived before this
-          // was created
-          ZoomConstraints constraints;
-          if (state->mController->GetRootZoomConstraints(&constraints)) {
-            apzc->UpdateZoomConstraints(constraints);
+        // The APZC we get off the layer may have been destroyed previously if the layer was inactive
+        // or omitted from the layer tree for whatever reason from a layers update. If it later comes
+        // back it will have a reference to a destroyed APZC and so we need to throw that out and make
+        // a new one.
+        bool newApzc = (apzc == nullptr || apzc->IsDestroyed());
+        if (newApzc) {
+          apzc = new AsyncPanZoomController(aLayersId, this, state->mController,
+                                            AsyncPanZoomController::USE_GESTURE_DETECTOR);
+          apzc->SetCompositorParent(aCompositor);
+          if (state->mCrossProcessParent != nullptr) {
+            apzc->ShareFrameMetricsAcrossProcesses();
           }
         } else {
-          // For an apzc that is not the root for its layers ID, we give it the
-          // same zoom constraints as its parent. This ensures that if e.g.
-          // user-scalable=no was specified, none of the APZCs allow double-tap
-          // to zoom.
-          apzc->UpdateZoomConstraints(apzc->GetParent()->GetZoomConstraints());
+          // If there was already an APZC for the layer clear the tree pointers
+          // so that it doesn't continue pointing to APZCs that should no longer
+          // be in the tree. These pointers will get reset properly as we continue
+          // building the tree. Also remove it from the set of APZCs that are going
+          // to be destroyed, because it's going to remain active.
+          aApzcsToDestroy->RemoveElement(apzc);
+          apzc->SetPrevSibling(nullptr);
+          apzc->SetLastChild(nullptr);
+        }
+        APZCTM_LOG("Using APZC %p for layer %p with identifiers %lld %lld\n", apzc, aLayer, aLayersId, metrics.GetScrollId());
+
+        apzc->NotifyLayersUpdated(metrics,
+                                  aIsFirstPaint && (aLayersId == aOriginatingLayersId));
+        apzc->SetScrollHandoffParentId(aLayer->GetScrollHandoffParentId());
+
+        // Use the composition bounds as the hit test region.
+        // Optionally, the GeckoContentController can provide a touch-sensitive
+        // region that constrains all frames associated with the controller.
+        // In this case we intersect the composition bounds with that region.
+        ParentLayerRect visible(metrics.mCompositionBounds);
+        CSSRect touchSensitiveRegion;
+        if (state->mController->GetTouchSensitiveRegion(&touchSensitiveRegion)) {
+          // Note: we assume here that touchSensitiveRegion is in the CSS pixels
+          // of our parent layer, which makes this coordinate conversion
+          // correct.
+          visible = visible.Intersect(touchSensitiveRegion
+                                      * metrics.mDevPixelsPerCSSPixel
+                                      * metrics.GetParentResolution());
         }
+
+        // Not sure what rounding option is the most correct here, but if we ever
+        // figure it out we can change this. For now I'm rounding in to minimize
+        // the chances of getting a complex region.
+        ParentLayerIntRect roundedVisible = RoundedIn(visible);
+        nsIntRegion unobscured;
+        unobscured.Sub(nsIntRect(roundedVisible.x, roundedVisible.y,
+                                 roundedVisible.width, roundedVisible.height),
+                       aObscured);
+
+        apzc->SetLayerHitTestData(unobscured, aTransform, transform);
+        APZCTM_LOG("Setting rect(%f %f %f %f) as visible region for APZC %p\n", visible.x, visible.y,
+                                                                              visible.width, visible.height,
+                                                                              apzc);
+
+        mApzcTreeLog << "APZC " << guid
+                     << "\tcb=" << visible
+                     << "\tsr=" << metrics.mScrollableRect
+                     << (aLayer->GetVisibleRegion().IsEmpty() ? "\tscrollinfo" : "")
+                     << (apzc->HasScrollgrab() ? "\tscrollgrab" : "")
+                     << "\t" << aLayer->GetContentDescription();
+
+        // Bind the APZC instance into the tree of APZCs
+        if (aNextSibling) {
+          aNextSibling->SetPrevSibling(apzc);
+        } else if (aParent) {
+          aParent->SetLastChild(apzc);
+        } else {
+          mRootApzc = apzc;
+          apzc->MakeRoot();
+        }
+
+        // For testing, log the parent scroll id of every APZC that has a
+        // parent. This allows test code to reconstruct the APZC tree.
+        // Note that we currently only do this for APZCs in the layer tree
+        // that originated the update, because the only identifying information
+        // we are logging about APZCs is the scroll id, and otherwise we could
+        // confuse APZCs from different layer trees with the same scroll id.
+        if (aLayersId == aOriginatingLayersId && apzc->GetParent()) {
+          aPaintLogger.LogTestData(metrics.GetScrollId(), "parentScrollId",
+              apzc->GetParent()->GetGuid().mScrollId);
+        }
+
+        // Let this apzc be the parent of other controllers when we recurse downwards
+        aParent = apzc;
+
+        if (newApzc) {
+          if (apzc->IsRootForLayersId()) {
+            // If we just created a new apzc that is the root for its layers ID, then
+            // we need to update its zoom constraints which might have arrived before this
+            // was created
+            ZoomConstraints constraints;
+            if (state->mController->GetRootZoomConstraints(&constraints)) {
+              apzc->UpdateZoomConstraints(constraints);
+            }
+          } else {
+            // For an apzc that is not the root for its layers ID, we give it the
+            // same zoom constraints as its parent. This ensures that if e.g.
+            // user-scalable=no was specified, none of the APZCs allow double-tap
+            // to zoom.
+            apzc->UpdateZoomConstraints(apzc->GetParent()->GetZoomConstraints());
+          }
+        }
+
+        insertResult.first->second = apzc;
       }
     }
   }
 
   aLayer->SetAsyncPanZoomController(apzc);
 
   mApzcTreeLog << '\n';
 
@@ -347,17 +367,17 @@ APZCTreeManager::UpdatePanZoomController
 
   // 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;
   for (Layer* child = aLayer->GetLastChild(); child; child = child->GetPrevSibling()) {
     gfx::TreeAutoIndent indent(mApzcTreeLog);
     next = UpdatePanZoomControllerTree(aCompositor, child, childLayersId, aTransform, aParent, next,
                                        aIsFirstPaint, aOriginatingLayersId,
-                                       aPaintLogger, aApzcsToDestroy, obscured);
+                                       aPaintLogger, aApzcsToDestroy, aApzcMap, obscured);
 
     // Each layer obscures its previous siblings, so we augment the obscured
     // region as we loop backwards through the children.
     nsIntRegion childRegion = child->GetVisibleRegion();
     childRegion.Transform(gfx::To3DMatrix(child->GetTransform()));
     if (child->GetClipRect()) {
       childRegion.AndWith(*child->GetClipRect());
     }
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_layers_APZCTreeManager_h
 #define mozilla_layers_APZCTreeManager_h
 
 #include <stdint.h>                     // for uint64_t, uint32_t
+#include <map>                          // for std::map
 #include "FrameMetrics.h"               // for FrameMetrics, etc
 #include "Units.h"                      // for CSSPoint, CSSRect, etc
 #include "gfxPoint.h"                   // for gfxPoint
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT_HELPER2
 #include "mozilla/EventForwards.h"      // for WidgetInputEvent, nsEventStatus
 #include "mozilla/Monitor.h"            // for Monitor
 #include "mozilla/gfx/Matrix.h"         // for Matrix4x4
 #include "nsAutoPtr.h"                  // for nsRefPtr
@@ -371,16 +372,17 @@ private:
                                                       Layer* aLayer, uint64_t aLayersId,
                                                       gfx::Matrix4x4 aTransform,
                                                       AsyncPanZoomController* aParent,
                                                       AsyncPanZoomController* aNextSibling,
                                                       bool aIsFirstPaint,
                                                       uint64_t aOriginatingLayersId,
                                                       const APZPaintLogHelper& aPaintLogger,
                                                       nsTArray< nsRefPtr<AsyncPanZoomController> >* aApzcsToDestroy,
+                                                      std::map<ScrollableLayerGuid, AsyncPanZoomController*>& aApzcMap,
                                                       const nsIntRegion& aObscured);
 
 private:
   /* Whenever walking or mutating the tree rooted at mRootApzc, mTreeLock must be held.
    * This lock does not need to be held while manipulating a single APZC instance in
    * isolation (that is, if its tree pointers are not being accessed or mutated). The
    * lock also needs to be held when accessing the mRootApzc instance variable, as that
    * is considered part of the APZC tree management state.