From f80768ca404a5cc01fa10084d4af1d558d1e6d95 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Mon, 19 Mar 2018 17:11:22 -0400 Subject: [PATCH] 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 --HG-- extra : rebase_source : 69a2bd82a812d674046957346c4f5036211d94cf --- gfx/layers/Layers.h | 9 ++++++--- gfx/layers/composite/AsyncCompositionManager.cpp | 10 ++++++---- gfx/layers/composite/ContainerLayerComposite.cpp | 8 +++++--- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h index eff8beed1051..7ccefb864e25 100644 --- a/gfx/layers/Layers.h +++ b/gfx/layers/Layers.h @@ -1798,10 +1798,13 @@ public: // 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; diff --git a/gfx/layers/composite/AsyncCompositionManager.cpp b/gfx/layers/composite/AsyncCompositionManager.cpp index 8397a9c7cdf6..7b76d0214263 100644 --- a/gfx/layers/composite/AsyncCompositionManager.cpp +++ b/gfx/layers/composite/AsyncCompositionManager.cpp @@ -886,10 +886,11 @@ AsyncCompositionManager::ApplyAsyncContentTransformToTree(Layer *aLayer, if (RefPtr 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; @@ -1069,10 +1070,11 @@ AsyncCompositionManager::ApplyAsyncContentTransformToTree(Layer *aLayer, 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; } diff --git a/gfx/layers/composite/ContainerLayerComposite.cpp b/gfx/layers/composite/ContainerLayerComposite.cpp index 6b3db82dc9fc..c40b3c19f8a8 100644 --- a/gfx/layers/composite/ContainerLayerComposite.cpp +++ b/gfx/layers/composite/ContainerLayerComposite.cpp @@ -299,10 +299,11 @@ RenderMinimap(ContainerT* aContainer, } 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); @@ -461,7 +462,8 @@ RenderLayers(ContainerT* aContainer, LayerManagerComposite* aManager, 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();