Bug 1375484 - ScrollSelectionIntoViewEvent should be called during refresh driver tick, r=ehsan

dom/base/Selection.cpp

@@ -59,7 +59,7 @@
 #include "nsCopySupport.h"
 #include "nsIClipboard.h"
 #include "nsIFrameInlines.h"
-
+#include "nsRefreshDriver.h"
 #include "nsIBidiKeyboard.h"
 
 #include "nsError.h"
@@ -3490,25 +3490,17 @@ Selection::PostScrollSelectionIntoViewEvent(
   // processed before we scroll. This will insure that we scroll to the
   // correct place on screen.
   mScrollEvent.Revoke();
+  nsPresContext* presContext = GetPresContext();
+  NS_ENSURE_STATE(presContext);
+  nsRefreshDriver* refreshDriver = presContext->RefreshDriver();
+  NS_ENSURE_STATE(refreshDriver);
 
   RefPtr<ScrollSelectionIntoViewEvent> ev =
     new ScrollSelectionIntoViewEvent(this, aRegion, aVertical, aHorizontal,
                                      aFlags);
   mScrollEvent = ev;
-  nsresult rv;
-  nsIDocument* doc = GetParentObject();
-  if (doc) {
-    rv = doc->Dispatch("ScrollSelectionIntoViewEvent",
-                       TaskCategory::Other,
-                       ev.forget());
-  } else {
-    rv = NS_DispatchToCurrentThread(ev);
-  }
-
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    mScrollEvent = nullptr; // no need to hold on to the event
-  }
-  return rv;
+  refreshDriver->AddPendingSelectionScroll(ev);
+  return NS_OK;
 }
 
 NS_IMETHODIMP

dom/html/test/forms/test_input_textarea_set_value_no_scroll.html

@@ -39,23 +39,25 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=829606
     setTimeout(function() {
       synthesizeKey('f', {});
 
-      var selectionAtTheEndSnapshot = snapshotWindow(window);
-      results = compareSnapshots(baseSnapshot, selectionAtTheEndSnapshot, false);
-      ok(results[0], "after appending a character, string should have changed");
+      requestAnimationFrame(function() {
+        var selectionAtTheEndSnapshot = snapshotWindow(window);
+        results = compareSnapshots(baseSnapshot, selectionAtTheEndSnapshot, false);
+        ok(results[0], "after appending a character, string should have changed");
 
-      element.value = element.value;
-      var tmpSnapshot = snapshotWindow(window);
+        element.value = element.value;
+        var tmpSnapshot = snapshotWindow(window);
 
-      results = compareSnapshots(baseSnapshot, tmpSnapshot, false);
-      ok(results[0], "re-settig the value should change nothing");
+        results = compareSnapshots(baseSnapshot, tmpSnapshot, false);
+        ok(results[0], "re-settig the value should change nothing");
 
-      results = compareSnapshots(selectionAtTheEndSnapshot, tmpSnapshot, true);
-      ok(results[0], "re-settig the value should change nothing");
+        results = compareSnapshots(selectionAtTheEndSnapshot, tmpSnapshot, true);
+        ok(results[0], "re-settig the value should change nothing");
 
-      element.selectionStart = element.selectionEnd = 0;
-      element.blur();
+        element.selectionStart = element.selectionEnd = 0;
+        element.blur();
 
-      gTestRunner.next();
+        gTestRunner.next();
+      });
     }, 0);
   }
 

editor/libeditor/tests/test_bug586662.html

@@ -43,12 +43,14 @@ SimpleTest.waitForFocus(function() {
         // Make sure that typing something into the textarea will cause the
         // page to scroll down
         synthesizeKey("a", {}, win);
-        setTimeout(function() {
-          isnot(win.scrollY, 0, "Page is scrolled down again");
+        requestAnimationFrame(function() {
+          setTimeout(function() {
+            isnot(win.scrollY, 0, "Page is scrolled down again");
 
-          win.close();
-          SimpleTest.finish();
-        }, 0);
+            win.close();
+            SimpleTest.finish();
+          }, 0);
+        });
       }, 0);
     }, 0);
   }, win);

layout/base/nsRefreshDriver.cpp

@@ -46,6 +46,7 @@
 #include "GeckoProfiler.h"
 #include "nsNPAPIPluginInstance.h"
 #include "mozilla/dom/Performance.h"
+#include "mozilla/dom/Selection.h"
 #include "mozilla/dom/WindowBinding.h"
 #include "mozilla/GeckoRestyleManager.h"
 #include "mozilla/RestyleManager.h"
@@ -1189,8 +1190,9 @@ nsRefreshDriver::nsRefreshDriver(nsPresContext* aPresContext)
 nsRefreshDriver::~nsRefreshDriver()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(ObserverCount() == 0,
-             "observers should have unregistered");
+  MOZ_ASSERT(ObserverCount() == mPendingSelectionScrolls.Length(),
+             "observers, except pending selection scrolls, "
+             "should have been unregistered");
   MOZ_ASSERT(!mActiveTimer, "timer should be gone");
   MOZ_ASSERT(!mPresContext,
              "Should have called Disconnect() and decremented "
@@ -1426,6 +1428,7 @@ nsRefreshDriver::ObserverCount() const
   sum += mFrameRequestCallbackDocs.Length();
   sum += mThrottledFrameRequestCallbackDocs.Length();
   sum += mViewManagerFlushIsPending;
+  sum += mPendingSelectionScrolls.Length();
   return sum;
 }
 
@@ -1813,6 +1816,12 @@ nsRefreshDriver::Tick(int64_t aNowEpoch, TimeStamp aNowTime)
     nsLayoutUtils::UpdateDisplayPortMarginsFromPendingMessages();
   }
 
+  AutoTArray<nsCOMPtr<nsIRunnable>, 16> pendingSelectionScrolls;
+  pendingSelectionScrolls.SwapElements(mPendingSelectionScrolls);
+  for (uint32_t i = 0; i < pendingSelectionScrolls.Length(); ++i) {
+    pendingSelectionScrolls[i]->Run();
+  }
+
   /*
    * The timer holds a reference to |this| while calling |Notify|.
    * However, implementations of |WillRefresh| are permitted to destroy

layout/base/nsRefreshDriver.h

@@ -192,6 +192,12 @@ public:
     return mLayoutFlushObservers.Contains(aShell);
   }
 
+  void AddPendingSelectionScroll(nsIRunnable* aRunnable)
+  {
+    mPendingSelectionScrolls.AppendElement(aRunnable);
+    EnsureTimerStarted();
+  }
+
   /**
    * Remember whether our presshell's view manager needs a flush
    */
@@ -450,6 +456,7 @@ private:
   ObserverArray mObservers[3];
   RequestTable mRequests;
   ImageStartTable mStartTable;
+  AutoTArray<nsCOMPtr<nsIRunnable>, 16> mPendingSelectionScrolls;
 
   struct PendingEvent {
     nsCOMPtr<nsINode> mTarget;

layout/forms/test/test_bug231389.html

@@ -42,11 +42,11 @@ addLoadEvent(function() {
   is(area.scrollTop, 0, "The textarea should not be scrolled initially");
   area.selectionStart = pos;
   area.selectionEnd = pos;
-  setTimeout(function() {
+  requestAnimationFrame(function() {
     isnot(area.scrollTop, 0, "The textarea's insertion point should be scrolled into view");
 
     SimpleTest.finish();
-  }, 0);
+  });
 });
 
 </script>