Bug 1465935 - Handle another edge case with hit-testing inside fixed-pos items. r=mstange
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 07 Jun 2018 13:06:33 -0400
changeset 478602 ada5a84764728f3d16d60f65052cf56f84aabd51
parent 478601 b04f4c9b15ab09b09cd38494db420f201ac8587a
child 478603 5b7b36cff4fc4a74deb69b1eba18842a4912cd2a
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstange
bugs1465935
milestone62.0a1
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 1465935 - Handle another edge case with hit-testing inside fixed-pos items. r=mstange Transforms are containing blocks for fixed-pos items, so if a fixed-pos item is inside a scrolled transform, then it should use that scrollframe as the scroll target for hit-testing. This patch handles this case for WebRender by stashing the appropriate ASR on the nsDisplayFixedPosition item and using it instead of the presShell's root scrollframe in this scenario. The patch also adds a mochitest (which is basically a mochitested version of the reftest in fixed-pos-scrolled-clip-3.html, with a hit-test check to ensure that it's hitting the right scrollframe). MozReview-Commit-ID: 7YQAeOiMMuP
gfx/layers/apz/test/mochitest/helper_hittest_fixed_in_scrolled_transform.html
gfx/layers/apz/test/mochitest/mochitest.ini
gfx/layers/apz/test/mochitest/test_group_hittest.html
layout/generic/nsFrame.cpp
layout/painting/nsDisplayList.cpp
layout/painting/nsDisplayList.h
new file mode 100644
--- /dev/null
+++ b/gfx/layers/apz/test/mochitest/helper_hittest_fixed_in_scrolled_transform.html
@@ -0,0 +1,87 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Hit-testing on the special setup from fixed-pos-scrolled-clip-3.html</title>
+  <script type="application/javascript" src="apz_test_utils.js"></script>
+  <script type="application/javascript" src="apz_test_native_event_utils.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/paint_listener.js"></script>
+  <meta name="viewport" content="width=device-width"/>
+<style>
+body {
+  margin: 0;
+  height: 4000px;
+}
+
+.transform {
+  transform: translate(10px, 10px);
+  width: 500px;
+}
+
+.subframe {
+  height: 600px;
+  overflow: auto;
+  box-shadow: 0 0 0 2px black;
+}
+
+.scrolled {
+  height: 4000px;
+  position: relative;
+}
+
+.absoluteClip {
+  position: absolute;
+  top: 300px;
+  left: 100px;
+  width: 200px;
+  height: 200px;
+  background: red;
+  clip: rect(auto auto auto auto);
+}
+
+.fixed {
+  position: fixed;
+  top: 0;
+  right: 0;
+  bottom: 0;
+  left: 0;
+  background: linear-gradient(lime, lime) black 0 100px no-repeat;
+  background-size: 100% 200px;
+}
+</style>
+</head>
+<body>
+<!-- This is lifted from layout/reftests/async-scrolling/fixed-pos-scrolled-clip-3.html -->
+<div class="transform">
+  <div class="subframe">
+    <div class="scrolled">
+      <div class="absoluteClip">
+        <div class="fixed"></div>
+      </div>
+    </div>
+  </div>
+</div>
+</body>
+<script type="application/javascript">
+
+function* test(testDriver) {
+  var config = getHitTestConfig();
+  var utils = config.utils;
+
+  // layerize the scrollable frame
+  var subframe = document.querySelector('.subframe');
+  utils.setDisplayPortForElement(0, 0, 800, 2000, subframe, 1);
+  yield waitForAllPaints(testDriver);
+
+  var target = document.querySelector('.absoluteClip');
+  checkHitResult(hitTest(centerOf(target)),
+                 APZHitResultFlags.VISIBLE,
+                 utils.getViewId(subframe),
+                 "fixed item inside a scrolling transform");
+
+  subtestDone();
+}
+
+waitUntilApzStable().then(runContinuation(test));
+
+</script>
+</html>
--- a/gfx/layers/apz/test/mochitest/mochitest.ini
+++ b/gfx/layers/apz/test/mochitest/mochitest.ini
@@ -21,16 +21,17 @@
     helper_drag_click.html
     helper_drag_scroll.html
     helper_iframe_pan.html
     helper_iframe1.html
     helper_iframe2.html
     helper_hittest_backface_hidden.html
     helper_hittest_basic.html
     helper_hittest_checkerboard.html
+    helper_hittest_fixed_in_scrolled_transform.html
     helper_hittest_float_bug1434846.html
     helper_hittest_float_bug1443518.html
     helper_key_scroll.html
     helper_long_tap.html
     helper_override_root.html
     helper_override_subdoc.html
     helper_overscroll_behavior_bug1425573.html
     helper_overscroll_behavior_bug1425603.html
--- a/gfx/layers/apz/test/mochitest/test_group_hittest.html
+++ b/gfx/layers/apz/test/mochitest/test_group_hittest.html
@@ -23,16 +23,17 @@ var prefs = [
   // APZ as a MouseInput so the hit result is recorded.
   ["test.events.async.enabled", true],
   // Turns on APZTestData logging which we use to obtain the hit test results.
   ["apz.test.logging_enabled", true]
 ];
 
 var subtests = [
   {'file': 'helper_hittest_basic.html', 'prefs': prefs},
+  {'file': 'helper_hittest_fixed_in_scrolled_transform.html', 'prefs': prefs},
   {'file': 'helper_hittest_float_bug1434846.html', 'prefs': prefs},
   {'file': 'helper_hittest_float_bug1443518.html', 'prefs': prefs},
   {'file': 'helper_hittest_checkerboard.html', 'prefs': prefs},
   {'file': 'helper_hittest_backface_hidden.html', 'prefs': prefs}
 ];
 
 if (isApzEnabled()) {
   SimpleTest.waitForExplicitFinish();
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3394,20 +3394,25 @@ nsIFrame::BuildDisplayListForStackingCon
   if (useFixedPosition) {
     if (clipCapturedBy == ContainerItemType::eFixedPosition) {
       clipState.Restore();
     }
     // The ASR for the fixed item should be the ASR of our containing block,
     // which has been set as the builder's current ASR, unless this frame is
     // invisible and we hadn't saved display item data for it. In that case,
     // we need to take the containerItemASR since we might have fixed children.
+    // For WebRender, we want to the know what |containerItemASR| is for the
+    // case where the fixed-pos item is not a "real" fixed-pos item (e.g. it's
+    // nested inside a scrolling transform), so we stash that on the display
+    // item as well.
     const ActiveScrolledRoot* fixedASR =
       ActiveScrolledRoot::PickAncestor(containerItemASR, aBuilder->CurrentActiveScrolledRoot());
     resultList.AppendToTop(
-        MakeDisplayItem<nsDisplayFixedPosition>(aBuilder, this, &resultList, fixedASR));
+        MakeDisplayItem<nsDisplayFixedPosition>(aBuilder, this, &resultList,
+          fixedASR, containerItemASR));
     if (aCreatedContainerItem) {
       *aCreatedContainerItem = true;
     }
   } else if (useStickyPosition) {
     // For position:sticky, the clip needs to be applied both to the sticky
     // container item and to the contents. The container item needs the clip
     // because a scrolled clip needs to move independently from the sticky
     // contents, and the contents need the clip so that they have finite
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7334,32 +7334,35 @@ nsDisplayResolution::BuildLayer(nsDispla
   layer->AsContainerLayer()->SetScaleToResolution(
       presShell->ScaleToResolution(), presShell->GetResolution());
   return layer.forget();
 }
 
 nsDisplayFixedPosition::nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder,
                                                nsIFrame* aFrame,
                                                nsDisplayList* aList,
-                                               const ActiveScrolledRoot* aActiveScrolledRoot)
+                                               const ActiveScrolledRoot* aActiveScrolledRoot,
+                                               const ActiveScrolledRoot* aContainerASR)
   : nsDisplayOwnLayer(aBuilder, aFrame, aList, aActiveScrolledRoot)
   , mIndex(0)
   , mIsFixedBackground(false)
+  , mContainerASR(aContainerASR)
 {
   MOZ_COUNT_CTOR(nsDisplayFixedPosition);
   Init(aBuilder);
 }
 
 nsDisplayFixedPosition::nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder,
                                                nsIFrame* aFrame,
                                                nsDisplayList* aList,
                                                uint32_t aIndex)
   : nsDisplayOwnLayer(aBuilder, aFrame, aList, aBuilder->CurrentActiveScrolledRoot())
   , mIndex(aIndex)
   , mIsFixedBackground(true)
+  , mContainerASR(nullptr) // XXX maybe this should be something?
 {
   MOZ_COUNT_CTOR(nsDisplayFixedPosition);
   Init(aBuilder);
 }
 
 void
 nsDisplayFixedPosition::Init(nsDisplayListBuilder* aBuilder)
 {
@@ -7422,47 +7425,62 @@ nsDisplayFixedPosition::BuildLayer(nsDis
   anchorRect.MoveTo(viewportFrame->GetOffsetToCrossDoc(ReferenceFrame()));
 
   nsLayoutUtils::SetFixedPositionLayerData(layer,
       viewportFrame, anchorRect, fixedFrame, presContext, aContainerParameters);
 
   return layer.forget();
 }
 
+ViewID
+nsDisplayFixedPosition::GetScrollTargetId()
+{
+  if (mContainerASR && !nsLayoutUtils::IsReallyFixedPos(mFrame)) {
+    return mContainerASR->GetViewId();
+  }
+  return nsLayoutUtils::ScrollIdForRootScrollFrame(mFrame->PresContext());
+}
+
 bool
 nsDisplayFixedPosition::CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                                 mozilla::wr::IpcResourceUpdateQueue& aResources,
                                                 const StackingContextHelper& aSc,
                                                 mozilla::layers::WebRenderLayerManager* aManager,
                                                 nsDisplayListBuilder* aDisplayListBuilder)
 {
   // We install this RAII scrolltarget tracker so that any
   // nsDisplayCompositorHitTestInfo items inside this fixed-pos item (and that
   // share the same ASR as this item) use the correct scroll target. That way
   // attempts to scroll on those items will scroll the root scroll frame.
   mozilla::wr::DisplayListBuilder::FixedPosScrollTargetTracker tracker(
       aBuilder,
       GetActiveScrolledRoot(),
-      nsLayoutUtils::ScrollIdForRootScrollFrame(Frame()->PresContext()));
+      GetScrollTargetId());
   return nsDisplayOwnLayer::CreateWebRenderCommands(aBuilder, aResources, aSc,
       aManager, aDisplayListBuilder);
 }
 
 bool
 nsDisplayFixedPosition::UpdateScrollData(mozilla::layers::WebRenderScrollData* aData,
                                          mozilla::layers::WebRenderLayerScrollData* aLayerData)
 {
   if (aLayerData) {
-    FrameMetrics::ViewID id = nsLayoutUtils::ScrollIdForRootScrollFrame(
-        Frame()->PresContext());
-    aLayerData->SetFixedPositionScrollContainerId(id);
+    aLayerData->SetFixedPositionScrollContainerId(GetScrollTargetId());
   }
   return nsDisplayOwnLayer::UpdateScrollData(aData, aLayerData) | true;
 }
 
+void
+nsDisplayFixedPosition::WriteDebugInfo(std::stringstream& aStream)
+{
+  aStream << nsPrintfCString(" (containerASR %s) (scrolltarget %" PRIu64 ")",
+      ActiveScrolledRoot::ToString(mContainerASR).get(),
+      GetScrollTargetId()).get();
+}
+
 TableType
 GetTableTypeFromFrame(nsIFrame* aFrame)
 {
   if (aFrame->IsTableFrame()) {
     return TableType::TABLE;
   }
 
   if (aFrame->IsTableColFrame()) {
--- a/layout/painting/nsDisplayList.h
+++ b/layout/painting/nsDisplayList.h
@@ -5948,17 +5948,18 @@ private:
   // GetActiveScrolledRoot(), or it may be a descendant of that.
   const ActiveScrolledRoot* mContainerASR;
 };
 
 class nsDisplayFixedPosition : public nsDisplayOwnLayer {
 public:
   nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                          nsDisplayList* aList,
-                         const ActiveScrolledRoot* aActiveScrolledRoot);
+                         const ActiveScrolledRoot* aActiveScrolledRoot,
+                         const ActiveScrolledRoot* aContainerASR);
   nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder,
                          const nsDisplayFixedPosition& aOther)
     : nsDisplayOwnLayer(aBuilder, aOther)
     , mAnimatedGeometryRootForScrollMetadata(aOther.mAnimatedGeometryRootForScrollMetadata)
     , mIndex(aOther.mIndex)
     , mIsFixedBackground(aOther.mIsFixedBackground)
   {
     MOZ_COUNT_CTOR(nsDisplayFixedPosition);
@@ -6012,25 +6013,29 @@ public:
   virtual bool CreateWebRenderCommands(mozilla::wr::DisplayListBuilder& aBuilder,
                                        mozilla::wr::IpcResourceUpdateQueue& aResources,
                                        const StackingContextHelper& aSc,
                                        mozilla::layers::WebRenderLayerManager* aManager,
                                        nsDisplayListBuilder* aDisplayListBuilder) override;
   virtual bool UpdateScrollData(mozilla::layers::WebRenderScrollData* aData,
                                 mozilla::layers::WebRenderLayerScrollData* aLayerData) override;
 
+  virtual void WriteDebugInfo(std::stringstream& aStream) override;
+
 protected:
   // For background-attachment:fixed
   nsDisplayFixedPosition(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                          nsDisplayList* aList, uint32_t aIndex);
   void Init(nsDisplayListBuilder* aBuilder);
+  ViewID GetScrollTargetId();
 
   RefPtr<AnimatedGeometryRoot> mAnimatedGeometryRootForScrollMetadata;
   uint32_t mIndex;
   bool mIsFixedBackground;
+  const ActiveScrolledRoot* mContainerASR;
 };
 
 class nsDisplayTableFixedPosition : public nsDisplayFixedPosition
 {
 public:
   static nsDisplayTableFixedPosition* CreateForFixedBackground(nsDisplayListBuilder* aBuilder,
                                                                nsIFrame* aFrame,
                                                                nsDisplayBackgroundImage* aImage,