Bug 1446022 - Guard against dereferencing a null APZC pointer in degenerate cases. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 19 Mar 2018 17:11:22 -0400
changeset 769602 d83f0954016c2ad775c9d31262c887fef6227cb9
parent 769595 c8dbb4ed05f38f40ef3607a6e36545bd95b8a287
push id103182
push userkgupta@mozilla.com
push dateMon, 19 Mar 2018 21:11:49 +0000
reviewersbotond
bugs1446022, 1443792
milestone61.0a1
Bug 1446022 - Guard against dereferencing a null APZC pointer in degenerate cases. r?botond This rolls back a few of the changes from bug 1443792. Although in theory a LayerMetricsWrapper having an APZC should be equivalent to it having a scrollable metrics, this might not always be strictly true. For example, if there is no GeckoContentController registered for a layer tree, then there might not be APZCs for that layer tree even though it has scrollable metrics. More importantly, a malicious child process might be able to trigger scenarios where the equivalence doesn't hold, and thereby trigger failures in the UI/GPU process. MozReview-Commit-ID: 1gfbILx7HWU
gfx/layers/Layers.h
gfx/layers/composite/AsyncCompositionManager.cpp
gfx/layers/composite/ContainerLayerComposite.cpp
--- a/gfx/layers/Layers.h
+++ b/gfx/layers/Layers.h
@@ -1793,20 +1793,23 @@ public:
    * composited.
    */
   virtual void ClearInvalidRegion() { mInvalidRegion.SetEmpty(); }
 
   // These functions allow attaching an AsyncPanZoomController to this layer,
   // and can be used anytime.
   // A layer has an APZC at index aIndex only-if GetFrameMetrics(aIndex).IsScrollable();
   // attempting to get an APZC for a non-scrollable metrics will return null.
-  // The reverse is also true (that if GetFrameMetrics(aIndex).IsScrollable()
-  // is true, then the layer will have an APZC), although that only holds on
+  // The reverse is also generally true (that if GetFrameMetrics(aIndex).IsScrollable()
+  // is true, then the layer will have an APZC). However, it only holds on the
   // the compositor-side layer tree, and only after the APZ code has had a chance
-  // to rebuild its internal hit-testing tree using the layer tree.
+  // to rebuild its internal hit-testing tree using the layer tree. Also, it may
+  // not hold in certain "exceptional" scenarios such as if the layer tree
+  // doesn't have a GeckoContentController registered for it, or if there is a
+  // malicious content process trying to trip up the compositor over IPC.
   // The aIndex for these functions must be less than GetScrollMetadataCount().
   void SetAsyncPanZoomController(uint32_t aIndex, AsyncPanZoomController *controller);
   AsyncPanZoomController* GetAsyncPanZoomController(uint32_t aIndex) const;
   // The ScrollMetadataChanged function is used internally to ensure the APZC array length
   // matches the frame metrics array length.
 
   virtual void ClearCachedResources() {}
 
--- a/gfx/layers/composite/AsyncCompositionManager.cpp
+++ b/gfx/layers/composite/AsyncCompositionManager.cpp
@@ -881,20 +881,21 @@ AsyncCompositionManager::ApplyAsyncConte
             ancestorMaskLayers.AppendElement(
                 layer->GetAncestorMaskLayerAt(*scrolledClip->GetMaskLayerIndex()));
           }
         }
 
         if (RefPtr<APZSampler> sampler = mCompositorBridge->GetAPZSampler()) {
           for (uint32_t i = 0; i < layer->GetScrollMetadataCount(); i++) {
             LayerMetricsWrapper wrapper(layer, i);
-            const FrameMetrics& metrics = wrapper.Metrics();
-            if (!metrics.IsScrollable()) {
+            if (!wrapper.GetApzc()) {
               continue;
             }
+            const FrameMetrics& metrics = wrapper.Metrics();
+            MOZ_ASSERT(metrics.IsScrollable());
 
             hasAsyncTransform = true;
 
             AsyncTransform asyncTransformWithoutOverscroll =
                 sampler->GetCurrentAsyncTransform(wrapper);
             AsyncTransformComponentMatrix overscrollTransform =
                 sampler->GetOverscrollTransform(wrapper);
             AsyncTransformComponentMatrix asyncTransform =
@@ -1064,20 +1065,21 @@ AsyncCompositionManager::ApplyAsyncConte
       });
 
   return appliedTransform;
 }
 
 static bool
 LayerIsScrollbarTarget(const LayerMetricsWrapper& aTarget, Layer* aScrollbar)
 {
-  const FrameMetrics& metrics = aTarget.Metrics();
-  if (!metrics.IsScrollable()) {
+  if (!aTarget.GetApzc()) {
     return false;
   }
+  const FrameMetrics& metrics = aTarget.Metrics();
+  MOZ_ASSERT(metrics.IsScrollable());
   if (metrics.GetScrollId() != aScrollbar->GetScrollbarTargetContainerId()) {
     return false;
   }
   return !metrics.IsScrollInfoLayer();
 }
 
 static void
 ApplyAsyncTransformToScrollbarForContent(const RefPtr<APZSampler>& aSampler,
--- a/gfx/layers/composite/ContainerLayerComposite.cpp
+++ b/gfx/layers/composite/ContainerLayerComposite.cpp
@@ -294,20 +294,21 @@ RenderMinimap(ContainerT* aContainer,
   Compositor* compositor = aManager->GetCompositor();
   MOZ_ASSERT(aSampler);
 
   if (aLayer->GetScrollMetadataCount() < 1) {
     return;
   }
 
   LayerMetricsWrapper wrapper(aLayer, 0);
-  const FrameMetrics& fm = wrapper.Metrics();
-  if (!fm.IsScrollable()) {
+  if (!wrapper.GetApzc()) {
     return;
   }
+  const FrameMetrics& fm = wrapper.Metrics();
+  MOZ_ASSERT(fm.IsScrollable());
 
   ParentLayerPoint scrollOffset = aSampler->GetCurrentAsyncScrollOffset(wrapper);
 
   // Options
   const int verticalPadding = 10;
   const int horizontalPadding = 5;
   gfx::Color backgroundColor(0.3f, 0.3f, 0.3f, 0.3f);
   gfx::Color tileActiveColor(1, 1, 1, 0.4f);
@@ -456,17 +457,18 @@ RenderLayers(ContainerT* aContainer, Lay
     // Within the list of scroll frames for a layer, the layer border for a
     // scroll frame lower down is affected by the async transforms on scroll
     // frames higher up, so loop from the top down, and accumulate an async
     // transform as we go along.
     Matrix4x4 asyncTransform;
     if (sampler) {
       for (uint32_t i = layer->GetScrollMetadataCount(); i > 0; --i) {
         LayerMetricsWrapper wrapper(layer, i - 1);
-        if (wrapper.Metrics().IsScrollable()) {
+        if (wrapper.GetApzc()) {
+          MOZ_ASSERT(wrapper.Metrics().IsScrollable());
           // Since the composition bounds are in the parent layer's coordinates,
           // use the parent's effective transform rather than the layer's own.
           ParentLayerRect compositionBounds = wrapper.Metrics().GetCompositionBounds();
           aManager->GetCompositor()->DrawDiagnostics(DiagnosticFlags::CONTAINER,
                                                      compositionBounds.ToUnknownRect(),
                                                      aClipRect.ToUnknownRect(),
                                                      asyncTransform * aContainer->GetEffectiveTransform());
           asyncTransform =