Bug 1375484 - ScrollSelectionIntoViewEvent should be called during refresh driver tick, r=ehsan
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Sun, 25 Jun 2017 00:38:42 +0300
changeset 365845 a3d1fb7eaac4b5df5534d54bddd248e8d24896ec
parent 365844 e6eef067a1981476b632107cfa0870b521bcb7e0
child 365846 429613574395423696756adc14c2dd6fb91a2ee5
push id91856
push useropettay@mozilla.com
push dateSat, 24 Jun 2017 21:40:27 +0000
treeherdermozilla-inbound@a3d1fb7eaac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1375484
milestone56.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 1375484 - ScrollSelectionIntoViewEvent should be called during refresh driver tick, r=ehsan
dom/base/Selection.cpp
dom/html/test/forms/test_input_textarea_set_value_no_scroll.html
editor/libeditor/tests/test_bug586662.html
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/forms/test/test_bug231389.html
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -54,17 +54,17 @@
 #include "nsIDocument.h"
 
 #include "nsISelectionController.h"//for the enums
 #include "nsAutoCopyListener.h"
 #include "SelectionChangeListener.h"
 #include "nsCopySupport.h"
 #include "nsIClipboard.h"
 #include "nsIFrameInlines.h"
-
+#include "nsRefreshDriver.h"
 #include "nsIBidiKeyboard.h"
 
 #include "nsError.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/ShadowRoot.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/SelectionBinding.h"
 #include "mozilla/AsyncEventDispatcher.h"
@@ -3485,35 +3485,27 @@ Selection::PostScrollSelectionIntoViewEv
                                          nsIPresShell::ScrollAxis aVertical,
                                          nsIPresShell::ScrollAxis aHorizontal)
 {
   // If we've already posted an event, revoke it and place a new one at the
   // end of the queue to make sure that any new pending reflow events are
   // 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
 Selection::ScrollIntoView(SelectionRegion aRegion, bool aIsSynchronous,
                           int16_t aVPercent, int16_t aHPercent)
 {
   ErrorResult result;
   ScrollIntoView(aRegion, aIsSynchronous, aVPercent, aHPercent, result);
--- a/dom/html/test/forms/test_input_textarea_set_value_no_scroll.html
+++ b/dom/html/test/forms/test_input_textarea_set_value_no_scroll.html
@@ -34,33 +34,35 @@ https://bugzilla.mozilla.org/show_bug.cg
     var results = compareSnapshots(baseSnapshot, snapshotWindow(window), true);
     ok(results[0], "sanity check: screenshots should be the same");
 
     element.selectionStart = element.selectionEnd = element.value.length;
 
     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);
   }
 
   // This test checks that when a textarea has a long list of values and the
   // textarea's value is then changed, the values are shown correctly.
   function testCorrectUpdateOnScroll()
   {
     var textarea = document.createElement('textarea');
--- a/editor/libeditor/tests/test_bug586662.html
+++ b/editor/libeditor/tests/test_bug586662.html
@@ -38,22 +38,24 @@ SimpleTest.waitForFocus(function() {
       isnot(win.scrollY, 0, "Page is scrolled down");
       // Scroll back up
       win.scrollTo(0, 0);
       setTimeout(function() {
         is(win.scrollY, 0, "Page is scrolled back up");
         // 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);
 });
 
    </script>
   </pre>
 </body>
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -41,16 +41,17 @@
 #include "jsapi.h"
 #include "nsContentUtils.h"
 #include "mozilla/PendingAnimationTracker.h"
 #include "mozilla/Preferences.h"
 #include "nsViewManager.h"
 #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"
 #include "mozilla/RestyleManagerInlines.h"
 #include "Layers.h"
 #include "imgIContainer.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "nsDocShell.h"
@@ -1184,18 +1185,19 @@ nsRefreshDriver::nsRefreshDriver(nsPresC
   mNextRecomputeVisibilityTick = mMostRecentTick;
 
   ++sRefreshDriverCount;
 }
 
 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 "
              "sRefreshDriverCount!");
 
   if (mRootRefresh) {
     mRootRefresh->RemoveRefreshObserver(this, FlushType::Style);
     mRootRefresh = nullptr;
@@ -1421,16 +1423,17 @@ nsRefreshDriver::ObserverCount() const
   // layout changes can affect media queries on child documents, triggering
   // style changes, etc.
   sum += mStyleFlushObservers.Length();
   sum += mLayoutFlushObservers.Length();
   sum += mPendingEvents.Length();
   sum += mFrameRequestCallbackDocs.Length();
   sum += mThrottledFrameRequestCallbackDocs.Length();
   sum += mViewManagerFlushIsPending;
+  sum += mPendingSelectionScrolls.Length();
   return sum;
 }
 
 uint32_t
 nsRefreshDriver::ImageRequestCount() const
 {
   uint32_t count = 0;
   for (auto iter = mStartTable.ConstIter(); !iter.Done(); iter.Next()) {
@@ -1808,16 +1811,22 @@ nsRefreshDriver::Tick(int64_t aNowEpoch,
 
   // We want to process any pending APZ metrics ahead of their positions
   // in the queue. This will prevent us from spending precious time
   // painting a stale displayport.
   if (gfxPrefs::APZPeekMessages()) {
     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
    * the pres context, which will cause our |mPresContext| to become
    * null.  If this happens, we must stop notifying observers.
    */
   for (uint32_t i = 0; i < ArrayLength(mObservers); ++i) {
     ObserverArray::EndLimitedIterator etor(mObservers[i]);
--- a/layout/base/nsRefreshDriver.h
+++ b/layout/base/nsRefreshDriver.h
@@ -187,16 +187,22 @@ public:
   }
   void RemoveLayoutFlushObserver(nsIPresShell* aShell) {
     mLayoutFlushObservers.RemoveElement(aShell);
   }
   bool IsLayoutFlushObserver(nsIPresShell* aShell) {
     return mLayoutFlushObservers.Contains(aShell);
   }
 
+  void AddPendingSelectionScroll(nsIRunnable* aRunnable)
+  {
+    mPendingSelectionScrolls.AppendElement(aRunnable);
+    EnsureTimerStarted();
+  }
+
   /**
    * Remember whether our presshell's view manager needs a flush
    */
   void ScheduleViewManagerFlush();
   void RevokeViewManagerFlush() {
     mViewManagerFlushIsPending = false;
   }
   bool ViewManagerFlushIsPending() {
@@ -445,16 +451,17 @@ private:
   mozilla::TimeStamp mTickStart;
   mozilla::TimeStamp mNextThrottledFrameRequestTick;
   mozilla::TimeStamp mNextRecomputeVisibilityTick;
 
   // separate arrays for each flush type we support
   ObserverArray mObservers[3];
   RequestTable mRequests;
   ImageStartTable mStartTable;
+  AutoTArray<nsCOMPtr<nsIRunnable>, 16> mPendingSelectionScrolls;
 
   struct PendingEvent {
     nsCOMPtr<nsINode> mTarget;
     nsCOMPtr<nsIDOMEvent> mEvent;
   };
 
   AutoTArray<nsIPresShell*, 16> mStyleFlushObservers;
   AutoTArray<nsIPresShell*, 16> mLayoutFlushObservers;
--- a/layout/forms/test/test_bug231389.html
+++ b/layout/forms/test/test_bug231389.html
@@ -37,19 +37,19 @@ SimpleTest.waitForExplicitFinish();
 addLoadEvent(function() {
   var area = document.getElementById("area");
   var val = area.value;
   var pos = val.indexOf("purposes");
 
   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>
 </pre>
 </body>
 </html>