Bug 1421825 - Fix crash and re-enable crashtest. r=jrmuizel
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 10 Jul 2018 09:37:40 -0400
changeset 425698 2052715b9faf6a3f21ec06334adac423d8e9164c
parent 425697 68a6a71686fad5fb4b46c993e8b3a0c80e4d743c
child 425699 2b1bc2f3a4874d742ecbadc2855a211b6cb06d02
push id34262
push usercsabou@mozilla.com
push dateTue, 10 Jul 2018 21:51:50 +0000
treeherdermozilla-central@70f901964f97 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
--- 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;
   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) {
     FrameMetrics::ViewID scrollId = asr->GetViewId();
     if (Maybe<size_t> index = aOwner.HasMetadataFor(scrollId)) {
     } else {
       Maybe<ScrollMetadata> metadata = asr->mScrollableFrame->ComputeScrollMetadata(
           aOwner.GetManager(), aItem->ReferenceFrame(),
           ContainerLayerParameters(), nullptr);
       MOZ_ASSERT(metadata->GetMetrics().GetScrollId() == scrollId);
+    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