Bug 1478776 - Part 7: Tune scroll events to only fire when the *relative* offset changes. r=botond
authorJan Henning <jh+bugzilla@buttercookie.de>
Thu, 20 Dec 2018 21:35:38 +0000
changeset 451698 6f2e2faa3321fb36ac285310855c4bd3e25e8657
parent 451697 63749e66f2665bfb64988809fbd541874084cfe1
child 451699 767281b6922eda01f577939b69f988ab6efc9866
push id35251
push userccoroiu@mozilla.com
push dateFri, 21 Dec 2018 21:54:30 +0000
treeherdermozilla-central@74101900e7d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1478776
milestone66.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 1478776 - Part 7: Tune scroll events to only fire when the *relative* offset changes. r=botond Internally, Gecko stores and updates the *absolute* offset between the visual viewport and the page, however the spec demands that the scroll event be fired whenever the *relative* offset between visual and layout viewport changes. Differential Revision: https://phabricator.services.mozilla.com/D14044
dom/base/VisualViewport.cpp
dom/base/VisualViewport.h
gfx/layers/apz/test/mochitest/helper_basic_pan.html
gfx/layers/apz/util/APZCCallbackHelper.cpp
layout/base/PresShell.cpp
layout/base/nsIPresShell.h
layout/generic/nsGfxScrollFrame.cpp
--- a/dom/base/VisualViewport.cpp
+++ b/dom/base/VisualViewport.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "VisualViewport.h"
 
 #include "mozilla/EventDispatcher.h"
+#include "mozilla/ToString.h"
 #include "nsIScrollableFrame.h"
 #include "nsIDocShell.h"
 #include "nsPresContext.h"
 #include "nsRefreshDriver.h"
 
 #define VVP_LOG(...)
 // #define VVP_LOG(...) printf_stderr("VVP: " __VA_ARGS__)
 
@@ -170,47 +171,67 @@ void VisualViewport::FireResizeEvent() {
   WidgetEvent event(true, eResize);
   event.mFlags.mBubbles = false;
   event.mFlags.mCancelable = false;
   EventDispatcher::Dispatch(this, GetPresContext(), &event);
 }
 
 /* ================= Scroll event handling ================= */
 
-void VisualViewport::PostScrollEvent() {
-  VVP_LOG("%p: PostScrollEvent\n", this);
+void VisualViewport::PostScrollEvent(const nsPoint& aPrevRelativeOffset) {
+  VVP_LOG("%p: PostScrollEvent, prevRelativeOffset %s\n", this,
+          ToString(aPrevRelativeOffset).c_str());
   if (mScrollEvent) {
     return;
   }
 
   // The event constructor will register itself with the refresh driver.
   if (nsPresContext* presContext = GetPresContext()) {
-    mScrollEvent = new VisualViewportScrollEvent(this, presContext);
+    mScrollEvent =
+        new VisualViewportScrollEvent(this, presContext, aPrevRelativeOffset);
     VVP_LOG("%p: PostScrollEvent, created new event\n", this);
   }
 }
 
 VisualViewport::VisualViewportScrollEvent::VisualViewportScrollEvent(
-    VisualViewport* aViewport, nsPresContext* aPresContext)
+    VisualViewport* aViewport, nsPresContext* aPresContext,
+    const nsPoint& aPrevRelativeOffset)
     : Runnable("VisualViewport::VisualViewportScrollEvent"),
-      mViewport(aViewport) {
+      mViewport(aViewport),
+      mPrevRelativeOffset(aPrevRelativeOffset) {
   aPresContext->RefreshDriver()->PostVisualViewportScrollEvent(this);
 }
 
 NS_IMETHODIMP
 VisualViewport::VisualViewportScrollEvent::Run() {
   if (mViewport) {
     mViewport->FireScrollEvent();
   }
   return NS_OK;
 }
 
 void VisualViewport::FireScrollEvent() {
   MOZ_ASSERT(mScrollEvent);
+  nsPoint prevRelativeOffset = mScrollEvent->PrevRelativeOffset();
   mScrollEvent->Revoke();
   mScrollEvent = nullptr;
 
-  VVP_LOG("%p, FireScrollEvent, fire VisualViewport scroll\n", this);
-  WidgetGUIEvent event(true, eScroll, nullptr);
-  event.mFlags.mBubbles = false;
-  event.mFlags.mCancelable = false;
-  EventDispatcher::Dispatch(this, GetPresContext(), &event);
+  nsIPresShell* presShell = GetPresShell();
+  // Check whether the relative visual viewport offset actually changed - maybe
+  // both visual and layout viewport scrolled together and there was no change
+  // after all.
+  if (presShell) {
+    nsPoint curRelativeOffset =
+        presShell->GetVisualViewportOffsetRelativeToLayoutViewport();
+    VVP_LOG(
+        "%p: FireScrollEvent, curRelativeOffset %s, "
+        "prevRelativeOffset %s\n",
+        this, ToString(curRelativeOffset).c_str(),
+        ToString(prevRelativeOffset).c_str());
+    if (curRelativeOffset != prevRelativeOffset) {
+      VVP_LOG("%p, FireScrollEvent, fire VisualViewport scroll\n", this);
+      WidgetGUIEvent event(true, eScroll, nullptr);
+      event.mFlags.mBubbles = false;
+      event.mFlags.mCancelable = false;
+      EventDispatcher::Dispatch(this, GetPresContext(), &event);
+    }
+  }
 }
--- a/dom/base/VisualViewport.h
+++ b/dom/base/VisualViewport.h
@@ -30,17 +30,17 @@ class VisualViewport final : public mozi
   double Scale() const;
   IMPL_EVENT_HANDLER(resize)
   IMPL_EVENT_HANDLER(scroll)
 
   virtual JSObject* WrapObject(JSContext* aCx,
                                JS::Handle<JSObject*> aGivenProto) override;
 
   void PostResizeEvent();
-  void PostScrollEvent();
+  void PostScrollEvent(const nsPoint& aPrevRelativeOffset);
 
   // These two events are modelled after the ScrollEvent class in
   // nsGfxScrollFrame.h.
   class VisualViewportResizeEvent : public Runnable {
    public:
     NS_DECL_NSIRUNNABLE
     VisualViewportResizeEvent(VisualViewport* aViewport,
                               nsPresContext* aPresContext);
@@ -49,21 +49,33 @@ class VisualViewport final : public mozi
    private:
     VisualViewport* mViewport;
   };
 
   class VisualViewportScrollEvent : public Runnable {
    public:
     NS_DECL_NSIRUNNABLE
     VisualViewportScrollEvent(VisualViewport* aViewport,
-                              nsPresContext* aPresContext);
+                              nsPresContext* aPresContext,
+                              const nsPoint& aPrevRelativeOffset);
     void Revoke() { mViewport = nullptr; }
+    nsPoint PrevRelativeOffset() const { return mPrevRelativeOffset; }
 
    private:
     VisualViewport* mViewport;
+    // The VisualViewport "scroll" event is supposed to be fired only when the
+    // *relative* offset between visual and layout viewport changes. The two
+    // viewports are updated independently from each other, though, so the only
+    // thing we can do is note the fact that one of the inputs into the relative
+    // visual viewport offset changed and then check the offset again at the
+    // next refresh driver tick, just before the event is going to fire.
+    // Hopefully, at this point both visual and layout viewport positions have
+    // been updated, so that we're able to tell whether the relative offset did
+    // in fact change or not.
+    const nsPoint mPrevRelativeOffset;
   };
 
  private:
   virtual ~VisualViewport();
 
   CSSSize VisualViewportSize() const;
   CSSPoint VisualViewportOffset() const;
   CSSPoint LayoutViewportOffset() const;
--- a/gfx/layers/apz/test/mochitest/helper_basic_pan.html
+++ b/gfx/layers/apz/test/mochitest/helper_basic_pan.html
@@ -50,17 +50,17 @@ function* test(testDriver) {
   // changes. Even when they're both scrolling together, we may update their
   // positions independently, though, leading to some jitter in the offset and
   // triggering the event after all.
   // At least for the case here, where both viewports are the same size and we
   // have a freshly loaded page, we should however be able to keep the offset at
   // a constant zero and therefore not cause any visual viewport scroll events
   // to fire.
   visScrEvt.unregister();
-  todo_is(visScrEvt.count, 0, "Got no visual viewport scroll events");
+  is(visScrEvt.count, 0, "Got no visual viewport scroll events");
   visScrEvtInternal.unregister();
   // Our internal visual viewport scroll event on the other hand only cares
   // about the absolute offset of the visual viewport and should therefore
   // definitively fire.
   todo(visScrEvtInternal.count > 0, "Got some mozvisualscroll events");
 }
 
 waitUntilApzStable()
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -170,17 +170,18 @@ static ScreenMargin ScrollFrame(nsIConte
   nsIScrollableFrame* sf =
       nsLayoutUtils::FindScrollableFrameFor(aRequest.GetScrollId());
   if (sf) {
     sf->ResetScrollInfoIfGeneration(aRequest.GetScrollGeneration());
     sf->SetScrollableByAPZ(!aRequest.IsScrollInfoLayer());
     if (sf->IsRootScrollFrameOfDocument()) {
       if (nsCOMPtr<nsIPresShell> shell = GetPresShell(aContent)) {
         shell->SetVisualViewportOffset(
-            CSSPoint::ToAppUnits(aRequest.GetScrollOffset()));
+            CSSPoint::ToAppUnits(aRequest.GetScrollOffset()),
+            shell->GetVisualViewportOffsetRelativeToLayoutViewport());
       }
     }
   }
   bool scrollUpdated = false;
   ScreenMargin displayPortMargins = aRequest.GetDisplayPortMargins();
   CSSPoint apzScrollOffset = aRequest.GetScrollOffset();
   CSSPoint actualScrollOffset = ScrollFrameTo(sf, aRequest, scrollUpdated);
 
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -10037,22 +10037,23 @@ void nsIPresShell::SetVisualViewportSize
     MarkFixedFramesForReflow(nsIPresShell::eResize);
 
     if (auto* window = nsGlobalWindowInner::Cast(mDocument->GetInnerWindow())) {
       window->VisualViewport()->PostResizeEvent();
     }
   }
 }
 
-void nsIPresShell::SetVisualViewportOffset(const nsPoint& aScrollOffset) {
+void nsIPresShell::SetVisualViewportOffset(const nsPoint& aScrollOffset,
+                                           const nsPoint& aPrevRelativeOffset) {
   if (mVisualViewportOffset != aScrollOffset) {
     mVisualViewportOffset = aScrollOffset;
 
     if (auto* window = nsGlobalWindowInner::Cast(mDocument->GetInnerWindow())) {
-      window->VisualViewport()->PostScrollEvent();
+      window->VisualViewport()->PostScrollEvent(aPrevRelativeOffset);
     }
   }
 }
 
 nsPoint nsIPresShell::GetVisualViewportOffsetRelativeToLayoutViewport() const {
   nsPoint result;
   if (nsIScrollableFrame* sf = GetRootScrollFrameAsScrollable()) {
     result = GetVisualViewportOffset() - sf->GetScrollPosition();
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -1642,17 +1642,18 @@ class nsIPresShell : public nsStubDocume
   void SetVisualViewportSize(nscoord aWidth, nscoord aHeight);
   bool IsVisualViewportSizeSet() { return mVisualViewportSizeSet; }
   nsSize GetVisualViewportSize() {
     NS_ASSERTION(mVisualViewportSizeSet,
                  "asking for visual viewport size when its not set?");
     return mVisualViewportSize;
   }
 
-  void SetVisualViewportOffset(const nsPoint& aScrollOffset);
+  void SetVisualViewportOffset(const nsPoint& aScrollOffset,
+                               const nsPoint& aPrevRelativeOffset);
 
   nsPoint GetVisualViewportOffset() const { return mVisualViewportOffset; }
 
   nsPoint GetVisualViewportOffsetRelativeToLayoutViewport() const;
 
   virtual void WindowSizeMoveDone() = 0;
   virtual void SysColorChanged() = 0;
   virtual void ThemeChanged() = 0;
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -69,16 +69,17 @@
 #include "nsPluginFrame.h"
 #include "nsSliderFrame.h"
 #include "mozilla/layers/APZCCallbackHelper.h"
 #include "mozilla/layers/AxisPhysicsModel.h"
 #include "mozilla/layers/AxisPhysicsMSDModel.h"
 #include "mozilla/layers/LayerTransactionChild.h"
 #include "mozilla/layers/ScrollLinkedEffectDetector.h"
 #include "mozilla/Unused.h"
+#include "VisualViewport.h"
 #include "LayersLogging.h"  // for Stringify
 #include <algorithm>
 #include <cstdlib>  // for std::abs(int/long)
 #include <cmath>    // for std::abs(float/double)
 
 #define PAINT_SKIP_LOG(...)
 // #define PAINT_SKIP_LOG(...) printf_stderr("PSKIP: " __VA_ARGS__)
 
@@ -2670,16 +2671,19 @@ void ScrollFrameHelper::ScrollToImpl(nsP
       std::max(visualViewportSize.width / std::max(sHorzScrollFraction, 1),
                AppUnitsPerCSSPixel());
   nscoord vertAllowance =
       std::max(visualViewportSize.height / std::max(sVertScrollFraction, 1),
                AppUnitsPerCSSPixel());
   if (dist.x >= horzAllowance || dist.y >= vertAllowance) {
     needFrameVisibilityUpdate = true;
   }
+  nsPoint prevVVRelativeOffset =
+      presContext->PresShell()
+          ->GetVisualViewportOffsetRelativeToLayoutViewport();
 
   // notify the listeners.
   for (uint32_t i = 0; i < mListeners.Length(); i++) {
     mListeners[i]->ScrollPositionWillChange(pt.x, pt.y);
   }
 
   nsRect oldDisplayPort;
   nsIContent* content = mOuter->GetContent();
@@ -2731,17 +2735,17 @@ void ScrollFrameHelper::ScrollToImpl(nsP
     content->SetProperty(nsGkAtoms::apzCallbackTransform, new CSSPoint(),
                          nsINode::DeleteProperty<CSSPoint>);
 
     // Similarly, update the main thread's view of the visual viewport
     // offset. Otherwise, if we perform calculations that depend on this
     // offset (e.g. by using nsIDOMWindowUtils.getVisualViewportOffset()
     // in chrome JS code) before it's updated by the next APZ repaint,
     // we could get incorrect results.
-    presContext->PresShell()->SetVisualViewportOffset(pt);
+    presContext->PresShell()->SetVisualViewportOffset(pt, prevVVRelativeOffset);
   }
 
   ScrollVisual();
 
   bool schedulePaint = true;
   if (nsLayoutUtils::AsyncPanZoomEnabled(mOuter) &&
       !nsLayoutUtils::ShouldDisableApzForElement(content) &&
       gfxPrefs::APZPaintSkipping()) {
@@ -2851,16 +2855,25 @@ void ScrollFrameHelper::ScrollToImpl(nsP
       return;
     }
   }
 
   presContext->RecordInteractionTime(
       nsPresContext::InteractionType::eScrollInteraction, TimeStamp::Now());
 
   PostScrollEvent();
+  // If this is a viewport scroll, this could affect the relative offset
+  // between layout and visual viewport, so we might have to fire a visual
+  // viewport scroll event as well.
+  if (mIsRoot) {
+    if (auto* window = nsGlobalWindowInner::Cast(
+            mOuter->PresContext()->Document()->GetInnerWindow())) {
+      window->VisualViewport()->PostScrollEvent(prevVVRelativeOffset);
+    }
+  }
 
   // notify the listeners.
   for (uint32_t i = 0; i < mListeners.Length(); i++) {
     mListeners[i]->ScrollPositionDidChange(pt.x, pt.y);
   }
 
   nsCOMPtr<nsIDocShell> docShell = presContext->GetDocShell();
   if (docShell) {