Bug 1559690 - Make scroll port events use the same mechanism as scroll events. r=hiro
☠☠ backed out by 918d7c84da37 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 22 Jun 2019 18:03:25 +0200
changeset 542988 fdb96c16d976a045ee5a96d2c6b866e60eded1a9
parent 542987 ae4b3c9f8ee55b431b4b9f7a7f454c27416a82bb
child 542989 9f45982e7800871078d5b707bcfcc6c12000bbc0
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1559690
milestone69.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 1559690 - Make scroll port events use the same mechanism as scroll events. r=hiro This prevents them from being called from layout and is more similar to what WillPaintObservers did. Differential Revision: https://phabricator.services.mozilla.com/D35604
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -121,53 +121,55 @@ static uint32_t GetOverflowChange(const 
  * categories of refresh observers (FlushType::Style, FlushType::Layout,
  * and FlushType::Display) are run at the desired time in a refresh driver
  * tick. They behave similarly to refresh observers in that their presence
  * causes the refresh driver to tick.
  *
  * ScrollEvents are one-shot runnables; the refresh driver drops them after
  * running them.
  */
-class ScrollFrameHelper::ScrollEvent : public Runnable {
+class GenericScrollEvent : public Runnable {
  public:
-  NS_DECL_NSIRUNNABLE
-  explicit ScrollEvent(ScrollFrameHelper* aHelper, bool aDelayed);
   void Revoke() { mHelper = nullptr; }
 
- private:
+ protected:
+  GenericScrollEvent(const char* aTag, ScrollFrameHelper* aHelper,
+                     bool aDelayed = false)
+      : Runnable(aTag), mHelper(aHelper) {
+    mHelper->mOuter->PresContext()->RefreshDriver()->PostScrollEvent(this,
+                                                                     aDelayed);
+  }
+
   ScrollFrameHelper* mHelper;
 };
 
-class ScrollFrameHelper::ScrollEndEvent : public Runnable {
- public:
-  NS_DECL_NSIRUNNABLE
-  explicit ScrollEndEvent(ScrollFrameHelper* aHelper);
-  void Revoke() { mHelper = nullptr; }
-
- private:
-  ScrollFrameHelper* mHelper;
-};
-
-class ScrollFrameHelper::AsyncScrollPortEvent : public Runnable {
- public:
-  NS_DECL_NSIRUNNABLE
-  explicit AsyncScrollPortEvent(ScrollFrameHelper* helper)
-      : Runnable("ScrollFrameHelper::AsyncScrollPortEvent"),
-        mHelper(helper) {}
-  void Revoke() { mHelper = nullptr; }
-
- private:
-  ScrollFrameHelper* mHelper;
-};
+#define DEFINE_SCROLL_EVENT(name_)                                    \
+  class ScrollFrameHelper::name_ final : public GenericScrollEvent {  \
+   public:                                                            \
+    explicit name_(ScrollFrameHelper* aHelper, bool aDelayed = false) \
+        : GenericScrollEvent("ScrollFrameHelper::" #name_, aHelper,   \
+                             aDelayed) {}                             \
+    NS_IMETHODIMP Run() final {                                       \
+      if (mHelper) {                                                  \
+        mHelper->Fire##name_();                                       \
+      }                                                               \
+      return NS_OK;                                                   \
+    }                                                                 \
+  };
+
+DEFINE_SCROLL_EVENT(ScrollEvent);
+DEFINE_SCROLL_EVENT(ScrollEndEvent);
+DEFINE_SCROLL_EVENT(ScrollPortEvent);
 
 class ScrollFrameHelper::ScrolledAreaEvent : public Runnable {
  public:
   NS_DECL_NSIRUNNABLE
   explicit ScrolledAreaEvent(ScrollFrameHelper* helper)
       : Runnable("ScrollFrameHelper::ScrolledAreaEvent"), mHelper(helper) {}
+
   void Revoke() { mHelper = nullptr; }
 
  private:
   ScrollFrameHelper* mHelper;
 };
 
 //----------------------------------------------------------------------
 
@@ -2113,16 +2115,19 @@ ScrollFrameHelper::ScrollFrameHelper(nsC
 
 ScrollFrameHelper::~ScrollFrameHelper() {
   if (mScrollEvent) {
     mScrollEvent->Revoke();
   }
   if (mScrollEndEvent) {
     mScrollEndEvent->Revoke();
   }
+  if (mScrollPortEvent) {
+    mScrollPortEvent->Revoke();
+  }
 }
 
 /*
  * Callback function from AsyncSmoothMSDScroll, used in
  * ScrollFrameHelper::ScrollTo
  */
 void ScrollFrameHelper::AsyncSmoothMSDScrollCallback(
     ScrollFrameHelper* aInstance, mozilla::TimeDuration aDeltaTime) {
@@ -4633,37 +4638,32 @@ auto ScrollFrameHelper::GetPageLoadingSt
     loadCompleted = cv->GetLoadCompleted();
     stopped = cv->GetIsStopped();
   }
   return loadCompleted
              ? (stopped ? LoadingState::Stopped : LoadingState::Loaded)
              : LoadingState::Loading;
 }
 
-nsresult ScrollFrameHelper::FireScrollPortEvent() {
-  mAsyncScrollPortEvent.Forget();
+void ScrollFrameHelper::FireScrollPortEvent() {
+  mScrollPortEvent->Revoke();
+  mScrollPortEvent = nullptr;
 
   // Keep this in sync with PostOverflowEvent().
   nsSize scrollportSize = mScrollPort.Size();
   nsSize childSize = GetScrolledRect().Size();
 
-  // TODO(emilio): why do we need the whole WillPaintObserver infrastructure and
-  // can't use AddScriptRunner & co? I guess it made sense when we used
-  // WillPaintObserver for scroll events too, or when this used to flush.
-  //
-  // Should we remove this?
-
   bool newVerticalOverflow = childSize.height > scrollportSize.height;
   bool vertChanged = mVerticalOverflow != newVerticalOverflow;
 
   bool newHorizontalOverflow = childSize.width > scrollportSize.width;
   bool horizChanged = mHorizontalOverflow != newHorizontalOverflow;
 
   if (!vertChanged && !horizChanged) {
-    return NS_OK;
+    return;
   }
 
   // If both either overflowed or underflowed then we dispatch only one
   // DOM event.
   bool both = vertChanged && horizChanged &&
               newVerticalOverflow == newHorizontalOverflow;
   InternalScrollPortEvent::OrientType orient;
   if (both) {
@@ -4687,18 +4687,18 @@ nsresult ScrollFrameHelper::FireScrollPo
   InternalScrollPortEvent event(
       true,
       (orient == InternalScrollPortEvent::eHorizontal ? mHorizontalOverflow
                                                       : mVerticalOverflow)
           ? eScrollPortOverflow
           : eScrollPortUnderflow,
       nullptr);
   event.mOrient = orient;
-  return EventDispatcher::Dispatch(mOuter->GetContent(), mOuter->PresContext(),
-                                   &event);
+  EventDispatcher::Dispatch(mOuter->GetContent(), mOuter->PresContext(),
+                            &event);
 }
 
 void ScrollFrameHelper::PostScrollEndEvent() {
   if (mScrollEndEvent) {
     return;
   }
 
   // The ScrollEndEvent constructor registers itself with the refresh driver.
@@ -5123,44 +5123,16 @@ void ScrollFrameHelper::CurPosAttributeC
                        isSmooth ? ScrollMode::Smooth : ScrollMode::Instant,
                        nsGkAtoms::scrollbars, &allowedRange);
   }
   // 'this' might be destroyed here
 }
 
 /* ============= Scroll events ========== */
 
-ScrollFrameHelper::ScrollEvent::ScrollEvent(ScrollFrameHelper* aHelper,
-                                            bool aDelayed)
-    : Runnable("ScrollFrameHelper::ScrollEvent"), mHelper(aHelper) {
-  mHelper->mOuter->PresContext()->RefreshDriver()->PostScrollEvent(this,
-                                                                   aDelayed);
-}
-
-NS_IMETHODIMP
-ScrollFrameHelper::ScrollEvent::Run() {
-  if (mHelper) {
-    mHelper->FireScrollEvent();
-  }
-  return NS_OK;
-}
-
-ScrollFrameHelper::ScrollEndEvent::ScrollEndEvent(ScrollFrameHelper* aHelper)
-    : Runnable("ScrollFrameHelper::ScrollEndEvent"), mHelper(aHelper) {
-  mHelper->mOuter->PresContext()->RefreshDriver()->PostScrollEvent(this);
-}
-
-NS_IMETHODIMP
-ScrollFrameHelper::ScrollEndEvent::Run() {
-  if (mHelper) {
-    mHelper->FireScrollEndEvent();
-  }
-  return NS_OK;
-}
-
 void ScrollFrameHelper::FireScrollEvent() {
   nsIContent* content = mOuter->GetContent();
   nsPresContext* prescontext = mOuter->PresContext();
 #ifdef MOZ_GECKO_PROFILER
   nsCOMPtr<nsIDocShell> docShell = prescontext->GetDocShell();
   AUTO_PROFILER_TRACING_DOCSHELL("Paint", "FireScrollEvent", GRAPHICS,
                                  docShell);
 #endif
@@ -5204,21 +5176,16 @@ void ScrollFrameHelper::PostScrollEvent(
   if (mScrollEvent) {
     return;
   }
 
   // The ScrollEvent constructor registers itself with the refresh driver.
   mScrollEvent = new ScrollEvent(this, aDelayed);
 }
 
-NS_IMETHODIMP
-ScrollFrameHelper::AsyncScrollPortEvent::Run() {
-  return mHelper ? mHelper->FireScrollPortEvent() : NS_OK;
-}
-
 bool nsXULScrollFrame::AddHorizontalScrollbar(nsBoxLayoutState& aState,
                                               bool aOnBottom) {
   if (!mHelper.mHScrollbarBox) {
     return true;
   }
 
   return AddRemoveScrollbar(aState, aOnBottom, true, true);
 }
@@ -5353,17 +5320,17 @@ void nsXULScrollFrame::LayoutScrollArea(
     // REVIEW: Have we accounted for both?
     ClampAndSetBounds(aState, childRect, aScrollPosition, true);
   }
 
   aState.SetLayoutFlags(oldflags);
 }
 
 void ScrollFrameHelper::PostOverflowEvent() {
-  if (mAsyncScrollPortEvent.IsPending()) {
+  if (mScrollPortEvent) {
     return;
   }
 
   // Keep this in sync with FireScrollPortEvent().
   nsSize scrollportSize = mScrollPort.Size();
   nsSize childSize = GetScrolledRect().Size();
 
   bool newVerticalOverflow = childSize.height > scrollportSize.height;
@@ -5371,18 +5338,17 @@ void ScrollFrameHelper::PostOverflowEven
 
   bool newHorizontalOverflow = childSize.width > scrollportSize.width;
   bool horizChanged = mHorizontalOverflow != newHorizontalOverflow;
 
   if (!vertChanged && !horizChanged) {
     return;
   }
 
-  mAsyncScrollPortEvent = new AsyncScrollPortEvent(this);
-  nsContentUtils::AddScriptRunner(mAsyncScrollPortEvent.get());
+  mScrollPortEvent = new ScrollPortEvent(this);
 }
 
 nsIFrame* ScrollFrameHelper::GetFrameForDir() const {
   nsIFrame* frame = mOuter;
   // XXX This is a bit on the slow side.
   if (mIsRoot) {
     // https://drafts.csswg.org/css-writing-modes-4/#principal-flow
     // If we're the root scrollframe, we need the root element's style data.
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -67,20 +67,20 @@ class ScrollFrameHelper : public nsIRefl
   // reload our child frame list.
   // We need this if a scrollbar frame is recreated.
   void ReloadChildFrames();
 
   nsresult CreateAnonymousContent(
       nsTArray<nsIAnonymousContentCreator::ContentInfo>& aElements);
   void AppendAnonymousContentTo(nsTArray<nsIContent*>& aElements,
                                 uint32_t aFilter);
-  nsresult FireScrollPortEvent();
+  void PostOverflowEvent();
+  void FireScrollPortEvent();
   void PostScrollEndEvent();
   void FireScrollEndEvent();
-  void PostOverflowEvent();
   using PostDestroyData = nsIFrame::PostDestroyData;
   void Destroy(PostDestroyData& aPostDestroyData);
 
   void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                         const nsDisplayListSet& aLists);
 
   // Add display items for the top-layer (which includes things like
   // the fullscreen element, its backdrop, and text selection carets)
@@ -516,22 +516,22 @@ class ScrollFrameHelper : public nsIRefl
   // owning references to the nsIAnonymousContentCreator-built content
   nsCOMPtr<mozilla::dom::Element> mHScrollbarContent;
   nsCOMPtr<mozilla::dom::Element> mVScrollbarContent;
   nsCOMPtr<mozilla::dom::Element> mScrollCornerContent;
   nsCOMPtr<mozilla::dom::Element> mResizerContent;
 
   class ScrollEvent;
   class ScrollEndEvent;
-  class AsyncScrollPortEvent;
+  class ScrollPortEvent;
   class ScrolledAreaEvent;
 
   RefPtr<ScrollEvent> mScrollEvent;
   RefPtr<ScrollEndEvent> mScrollEndEvent;
-  nsRevocableEventPtr<AsyncScrollPortEvent> mAsyncScrollPortEvent;
+  RefPtr<ScrollPortEvent> mScrollPortEvent;
   nsRevocableEventPtr<ScrolledAreaEvent> mScrolledAreaEvent;
   nsIFrame* mHScrollbarBox;
   nsIFrame* mVScrollbarBox;
   nsIFrame* mScrolledFrame;
   nsIFrame* mScrollCornerBox;
   nsIFrame* mResizerBox;
   nsContainerFrame* mOuter;
   const nsIFrame* mReferenceFrameDuringPainting;