Bug 1421825 - Fix crash and re-enable crashtest. r?jrmuizel draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 10 Jul 2018 09:37:40 -0400
changeset 816060 e7cc401e2a6ecd5b610f0062b78bb10700c3622c
parent 815985 a675c5d7eb76887a3e4b24548d621c9cc05a1545
push id115735
push userkgupta@mozilla.com
push dateTue, 10 Jul 2018 14:46:46 +0000
reviewersjrmuizel
bugs1421825
milestone63.0a1
Bug 1421825 - Fix crash and re-enable crashtest. r?jrmuizel In some cases we get a gecko display list that looks like this: WrapList with asr(<A>, <B>) Item with asr (<B>) and clipchain(<something> [A]) In this case, we would initialize the WebRenderLayerScrollData for the nested item using a stop-at ancestor of A (because that was the leafmost ASR from the containing WrapList) but the item itself has an ASR of B, which is an ancestor of A. So when walking up from B we'd never hit the stop-at ancestor, and so we'd end up duplicating metrics from the containing WRLSD onto the nested WRLSD. This generated an assertion failure in the APZ code. This patch detects this scenario and skips adding metrics on the nested WRLSD. This produces an APZ tree equivalent to what the non-WebRender path would produce. MozReview-Commit-ID: 8eo6pzXXKBd
gfx/layers/wr/WebRenderScrollData.cpp
layout/painting/crashtests/crashtests.list
--- a/gfx/layers/wr/WebRenderScrollData.cpp
+++ b/gfx/layers/wr/WebRenderScrollData.cpp
@@ -45,31 +45,39 @@ WebRenderLayerScrollData::Initialize(Web
                                      const Maybe<gfx::Matrix4x4>& aAncestorTransform)
 {
   MOZ_ASSERT(aDescendantCount >= 0); // Ensure value is valid
   MOZ_ASSERT(mDescendantCount == -1); // Don't allow re-setting an already set value
   mDescendantCount = aDescendantCount;
 
   MOZ_ASSERT(aItem);
   aItem->UpdateScrollData(&aOwner, this);
-  for (const ActiveScrolledRoot* asr = aItem->GetActiveScrolledRoot();
-       asr && asr != aStopAtAsr;
-       asr = asr->mParent) {
+
+  const ActiveScrolledRoot* asr = aItem->GetActiveScrolledRoot();
+  if (ActiveScrolledRoot::IsAncestor(asr, aStopAtAsr)) {
+    // If the item's ASR is an ancestor of the stop-at ASR, then we don't need
+    // any more metrics information because we'll end up duplicating what the
+    // ancestor WebRenderLayerScrollData already has.
+    asr = nullptr;
+  }
+
+  while (asr && asr != aStopAtAsr) {
     MOZ_ASSERT(aOwner.GetManager());
     FrameMetrics::ViewID scrollId = asr->GetViewId();
     if (Maybe<size_t> index = aOwner.HasMetadataFor(scrollId)) {
       mScrollIds.AppendElement(index.ref());
     } else {
       Maybe<ScrollMetadata> metadata = asr->mScrollableFrame->ComputeScrollMetadata(
           aOwner.GetManager(), aItem->ReferenceFrame(),
           ContainerLayerParameters(), nullptr);
       MOZ_ASSERT(metadata);
       MOZ_ASSERT(metadata->GetMetrics().GetScrollId() == scrollId);
       mScrollIds.AppendElement(aOwner.AddMetadata(metadata.ref()));
     }
+    asr = asr->mParent;
   }
 
   // aAncestorTransform, if present, is the transform from an ancestor
   // nsDisplayTransform that was stored on the stacking context in order to
   // propagate it downwards in the tree. (i.e. |aItem| is a strict descendant of
   // the nsDisplayTranform which produced aAncestorTransform). We store this
   // separately from mTransform because in the case where we have multiple
   // scroll metadata on this layer item, the mAncestorTransform is associated
--- a/layout/painting/crashtests/crashtests.list
+++ b/layout/painting/crashtests/crashtests.list
@@ -3,14 +3,14 @@ skip-if(Android) load 1407470-1.html
 load 1413073-1.html
 load 1413073-2.html
 load 1405881-1.html
 load 1418177-1.html
 load 1418722-1.html
 load 1419917.html
 load 1425271-1.html
 load 1428906-1.html
-skip-if(webrender) load 1430589-1.html # bug 1421825 for webrender
+load 1430589-1.html
 load 1454105-1.html
 load 1455944-1.html
 load 1465305-1.html
 load 1468124-1.html
 load 1469472.html