Bug 1488871 - Don't flush layout in AsyncScrollPortEvent::Run. r=mats

I'd claim that we don't need it because, in order to enqueue the event, we
already need to have overflowed the event in a normal reflow.

For now this should not break anything (or anything that wasn't already racy
depending on when we paint).

The only reason the flush is there is according to roc is to decide whether to
fire the event, and because it needs the layout information:

  https://bugzilla.mozilla.org/show_bug.cgi?id=771822#c4

In practice, however, all the layout information we need we have already
computed by the time we post the event.

We don't expose the rects via the event details, which is what could get
out-of-date, so this patch could only mean that we fire the event slightly more
often in cases where people remove stuff from the DOM, right after we do layout
and the content has overflowed. But that's actually pretty unlikely.

This event in general is pretty problematic because it exposes when we do
layout and when we paint, which is not great. Its test coverage is also pretty
low (test_overflow_event.html, which of course still passes without this).

I still want to do this change first since it's trivial to back out if needed.

Then I'd want to change how it fires to match the scrolled area change event
(which would allow us to remove the WillPaintObserver stuff), after verifying
that chrome consumers are still fine with that, and then put behind a pref and
hide it from content, while we leave time for chrome consumers to migrate away
from it, and allow us to revert if something breaks.

Differential Revision: https://phabricator.services.mozilla.com/D5082

--HG--
extra : moz-landing-system : lando

layout/generic/nsGfxScrollFrame.cpp

@@ -4580,6 +4580,12 @@ nsresult ScrollFrameHelper::FireScrollPortEvent() {
   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;
 
@@ -5140,10 +5146,6 @@ void ScrollFrameHelper::PostScrollEvent(bool aDelayed) {
 
 NS_IMETHODIMP
 ScrollFrameHelper::AsyncScrollPortEvent::Run() {
-  if (mHelper) {
-    mHelper->mOuter->PresContext()->Document()->FlushPendingNotifications(
-        FlushType::InterruptibleLayout);
-  }
   return mHelper ? mHelper->FireScrollPortEvent() : NS_OK;
 }