Bug 1149555 - Update resize event firing to follow the specs, dispatch right before rAF callbacks, r=dbaron
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Tue, 05 May 2015 17:56:01 +0300
changeset 273822 f58f60cd772cb25e395c7e2effc125897c45cc98
parent 273731 e3d3f273cf88a894ae25f096ab85f68e49b1982a
child 273823 80b09ba3d52e42c43cb831df5dd192680e5ca663
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1149555
milestone40.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 1149555 - Update resize event firing to follow the specs, dispatch right before rAF callbacks, r=dbaron
browser/base/content/test/general/browser_windowopen_reflows.js
browser/base/content/test/social/browser_social_chatwindow_resize.js
dom/base/test/test_reentrant_flush.html
dom/tests/mochitest/bugs/test_sizetocontent_clamp.html
layout/base/nsIPresShell.h
layout/base/nsPresShell.cpp
layout/base/nsPresShell.h
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/reftests/font-face/ex-unit-1-dynamic.html
--- a/browser/base/content/test/general/browser_windowopen_reflows.js
+++ b/browser/base/content/test/general/browser_windowopen_reflows.js
@@ -18,16 +18,19 @@ const EXPECTED_REFLOWS = [
     "gBrowserInit._delayedStartup@chrome://browser/content/browser.js|",
 
   // Focusing the content area causes a reflow.
   "gBrowserInit._delayedStartup@chrome://browser/content/browser.js|",
 
   // Sometimes sessionstore collects data during this test, which causes a sync reflow
   // (https://bugzilla.mozilla.org/show_bug.cgi?id=892154 will fix this)
   "ssi_getWindowDimension@resource:///modules/sessionstore/SessionStore.jsm",
+
+  // We may get a resize event, see bug 1149555.
+  "PreviewController.prototype.wasResizedSinceLastPreview@resource:///modules/WindowsPreviewPerTab.jsm"
 ];
 
 if (Services.appinfo.OS == "WINNT" || Services.appinfo.OS == "Darwin") {
   // TabsInTitlebar._update causes a reflow on OS X and Windows trying to do calculations
   // since layout info is already dirty. This doesn't seem to happen before
   // MozAfterPaint on Linux.
   EXPECTED_REFLOWS.push("rect@chrome://browser/content/browser.js|" +
                           "TabsInTitlebar._update@chrome://browser/content/browser.js|" +
--- a/browser/base/content/test/social/browser_social_chatwindow_resize.js
+++ b/browser/base/content/test/social/browser_social_chatwindow_resize.js
@@ -67,18 +67,18 @@ var tests = {
         [chatWidth-2, 1, "to < 1 chat width - only last should be visible."],
         [chatWidth+2, 1, "2 pixels more then one fully exposed (not counting popup) - still only 1."],
         [chatWidth+popupWidth+2, 1, "2 pixels more than one fully exposed (including popup) - still only 1."],
         [chatWidth*2-2, 1, "second not showing by 2 pixels (not counting popup) - only 1 exposed."],
         [chatWidth*2+popupWidth-2, 1, "second not showing by 2 pixelx (including popup) - only 1 exposed."],
         [chatWidth*2+popupWidth+2, 2, "big enough to fit 2 - nub remains visible as first is still hidden"],
         [chatWidth*3+popupWidth-2, 2, "one smaller than the size necessary to display all three - first still hidden"],
         [chatWidth*3+popupWidth+2, 3, "big enough to fit all - all exposed (which removes the nub)"],
-        [chatWidth*3+2, 3, "now the nub is hidden we can resize back down to chatWidth*3 before overflow."],
-        [chatWidth*3-2, 2, "2 pixels less and the first is again collapsed (and the nub re-appears)"],
+        [chatWidth*3+4, 3, "now the nub is hidden we can resize back down to chatWidth*3 before overflow."],
+        [chatWidth*3-4, 2, "4 pixels less and the first is again collapsed (and the nub re-appears)"],
         [chatWidth*2+popupWidth+2, 2, "back down to just big enough to fit 2"],
         [chatWidth*2+popupWidth-2, 1, "back down to just not enough to fit 2"],
         [chatWidth*3+popupWidth+2, 3, "now a large jump to make all 3 visible (ie, affects 2)"],
         [chatWidth*1.5, 1, "and a large jump back down to 1 visible (ie, affects 2)"],
       ], function() {
         closeAllChats();
         next();
       });
--- a/dom/base/test/test_reentrant_flush.html
+++ b/dom/base/test/test_reentrant_flush.html
@@ -43,19 +43,18 @@ addLoadEvent(function() {
   win.addEventListener("resize", handleResize, false);
   SpecialPowers.setFullZoom(win, 2);
 
   is(resizeHandlerRan, false,
      "Resize handler should not have run yet for this test to be valid");
 
   // Now flush out layout on the subdocument, to trigger the resize handler
   is(bod.getBoundingClientRect().width, 50, "Width of body should still be 50px");
-
-  is(resizeHandlerRan, true, "Resize handler should have run");
-
-  win.removeEventListener("resize", handleResize, false);
-
-  SimpleTest.finish();
+  window.requestAnimationFrame(function() {
+    is(resizeHandlerRan, true, "Resize handler should have run");
+    win.removeEventListener("resize", handleResize, false);
+    SimpleTest.finish();
+  });
 });
 </script>
 </pre>
 </body>
 </html>
--- a/dom/tests/mochitest/bugs/test_sizetocontent_clamp.html
+++ b/dom/tests/mochitest/bugs/test_sizetocontent_clamp.html
@@ -32,33 +32,31 @@ var epsilon =  navigator.platform.indexO
 // outer window chrome.
 var isWin8 = (navigator.userAgent.indexOf("Windows NT 6.2") != -1);
 
 var innerWidthMin = (isWin8 ? 120 : 100);
 var innerWidthMax = (isWin8 ? 125 : 100);
 
 function test() {
   var w = window.open('data:text/html,null', null, 'width=300,height=300');
-  var nbResize = 0;
 
   SimpleTest.waitForFocus(function() {
     w.onresize = function() {
-      nbResize++;
-
-      if (nbResize == 1) {
+      if (!(w.innerWidth + epsilon >= innerWidthMin &&
+            w.innerWidth - epsilon <= innerWidthMax)) {
+        // We need still another resize event.
         return;
       }
-
-      ok(w.innerWidth + epsilon >= innerWidthMin && w.innerWidth - epsilon <= innerWidthMax,
-         "innerWidth should be between " + innerWidthMin + " and " + innerWidthMax);
-      ok(w.innerHeight + epsilon >= 100 && w.innerHeight - epsilon <= 100,
-         "innerHeight should be around 100");
-
-      // It's not clear why 2 events are coming...
-      is(nbResize, 2, "We should get 2 events.");
+      if (!(w.innerHeight + epsilon >= 100 &&
+        w.innerHeight - epsilon <= 100)) {
+        // ditto
+        return;
+      }
+      ok(true, "innerWidth should be between " + innerWidthMin + " and " + innerWidthMax);
+      ok(true, "innerHeight should be around 100");
 
       w.close();
 
       SimpleTest.waitForFocus(function() {
         SimpleTest.finish();
       });
     };
     w.sizeToContent();
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -133,20 +133,20 @@ typedef struct CapturingContentInfo {
   // capture should only be allowed during a mousedown event
   bool mAllowed;
   bool mPointerLock;
   bool mRetargetToElement;
   bool mPreventDrag;
   mozilla::StaticRefPtr<nsIContent> mContent;
 } CapturingContentInfo;
 
-// d910f009-d209-74c1-6b04-30c83c051c78
+// b7b89561-4f03-44b3-9afa-b47e7f313ffb
 #define NS_IPRESSHELL_IID \
-  { 0x025264c6, 0x0b12, 0x4804, \
-    { 0xa3, 0x3e, 0xb7, 0x73, 0xf2, 0x19, 0x48, 0x90 } }
+  { 0xb7b89561, 0x4f03, 0x44b3, \
+    { 0x9a, 0xfa, 0xb4, 0x7e, 0x7f, 0x31, 0x3f, 0xfb } }
 
 // debug VerifyReflow flags
 #define VERIFY_REFLOW_ON                    0x01
 #define VERIFY_REFLOW_NOISY                 0x02
 #define VERIFY_REFLOW_ALL                   0x04
 #define VERIFY_REFLOW_DUMP_COMMANDS         0x08
 #define VERIFY_REFLOW_NOISY_RC              0x10
 #define VERIFY_REFLOW_REALLY_NOISY_RC       0x20
@@ -1545,16 +1545,18 @@ public:
   virtual void EnsureImageInVisibleList(nsIImageLoadingContent* aImage) = 0;
 
   // Removes the image from the list of visible images if it is present there.
   virtual void RemoveImageFromVisibleList(nsIImageLoadingContent* aImage) = 0;
 
   // Whether we should assume all images are visible.
   virtual bool AssumeAllImagesVisible() = 0;
 
+  virtual void FireResizeEvent() = 0;
+
   /**
    * Refresh observer management.
    */
 protected:
   virtual bool AddRefreshObserverExternal(nsARefreshObserver* aObserver,
                                           mozFlushType aFlushType);
   bool AddRefreshObserverInternal(nsARefreshObserver* aObserver,
                                   mozFlushType aFlushType);
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -1211,31 +1211,28 @@ PresShell::Destroy()
     if (mDocument->HasAnimationController()) {
       mDocument->GetAnimationController()->NotifyRefreshDriverDestroying(rd);
     }
   }
 
   // Revoke any pending events.  We need to do this and cancel pending reflows
   // before we destroy the frame manager, since apparently frame destruction
   // sometimes spins the event queue when plug-ins are involved(!).
+  if (mResizeEventPending) {
+    rd->RemoveResizeEventFlushObserver(this);
+  }
   rd->RemoveLayoutFlushObserver(this);
   if (mHiddenInvalidationObserverRefreshDriver) {
     mHiddenInvalidationObserverRefreshDriver->RemovePresShellToInvalidateIfHidden(this);
   }
 
   if (rd->PresContext() == GetPresContext()) {
     rd->RevokeViewManagerFlush();
   }
 
-  mResizeEvent.Revoke();
-  if (mAsyncResizeTimerIsActive) {
-    mAsyncResizeEventTimer->Cancel();
-    mAsyncResizeTimerIsActive = false;
-  }
-
   CancelAllPendingReflows();
   CancelPostedReflowCallbacks();
 
   // Destroy the frame manager. This will destroy the frame hierarchy
   mFrameConstructor->WillDestroyFrameTree();
 
   // Destroy all frame properties (whose destruction was suppressed
   // while destroying the frame tree, but which might contain more
@@ -1993,22 +1990,16 @@ PresShell::Initialize(nscoord aWidth, ns
 void
 PresShell::sPaintSuppressionCallback(nsITimer *aTimer, void* aPresShell)
 {
   nsRefPtr<PresShell> self = static_cast<PresShell*>(aPresShell);
   if (self)
     self->UnsuppressPainting();
 }
 
-void
-PresShell::AsyncResizeEventCallback(nsITimer* aTimer, void* aPresShell)
-{
-  static_cast<PresShell*>(aPresShell)->FireResizeEvent();
-}
-
 nsresult
 PresShell::ResizeReflowOverride(nscoord aWidth, nscoord aHeight)
 {
   mViewportOverridden = true;
   return ResizeReflowIgnoreOverride(aWidth, aHeight);
 }
 
 nsresult
@@ -2083,63 +2074,40 @@ PresShell::ResizeReflowIgnoreOverride(ns
   }
 
   rootFrame = mFrameConstructor->GetRootFrame();
   if (aHeight == NS_UNCONSTRAINEDSIZE && rootFrame) {
     mPresContext->SetVisibleArea(
       nsRect(0, 0, aWidth, rootFrame->GetRect().height));
   }
 
-  if (!mIsDestroying && !mResizeEvent.IsPending() &&
-      !mAsyncResizeTimerIsActive) {
-    if (mInResize) {
-      if (!mAsyncResizeEventTimer) {
-        mAsyncResizeEventTimer = do_CreateInstance("@mozilla.org/timer;1");
-      }
-      if (mAsyncResizeEventTimer) {
-        mAsyncResizeTimerIsActive = true;
-        mAsyncResizeEventTimer->InitWithFuncCallback(AsyncResizeEventCallback,
-                                                     this, 15,
-                                                     nsITimer::TYPE_ONE_SHOT);
-      }
-    } else {
-      nsRefPtr<nsRunnableMethod<PresShell> > resizeEvent =
-        NS_NewRunnableMethod(this, &PresShell::FireResizeEvent);
-      if (NS_SUCCEEDED(NS_DispatchToCurrentThread(resizeEvent))) {
-        mResizeEvent = resizeEvent;
-        mDocument->SetNeedStyleFlush();
-      }
-    }
+  if (!mIsDestroying && !mResizeEventPending) {
+    mResizeEventPending = true;
+    GetPresContext()->RefreshDriver()->AddResizeEventFlushObserver(this);
   }
 
   return NS_OK; //XXX this needs to be real. MMP
 }
 
 void
 PresShell::FireResizeEvent()
 {
-  if (mAsyncResizeTimerIsActive) {
-    mAsyncResizeTimerIsActive = false;
-    mAsyncResizeEventTimer->Cancel();
-  }
-  mResizeEvent.Revoke();
-
-  if (mIsDocumentGone)
+  if (mIsDocumentGone) {
     return;
+  }
+
+  mResizeEventPending = false;
 
   //Send resize event from here.
   WidgetEvent event(true, NS_RESIZE_EVENT);
   nsEventStatus status = nsEventStatus_eIgnore;
 
-  nsPIDOMWindow *window = mDocument->GetWindow();
+  nsPIDOMWindow* window = mDocument->GetWindow();
   if (window) {
-    nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
-    mInResize = true;
     EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status);
-    mInResize = false;
   }
 }
 
 void
 PresShell::SetIgnoreFrameDestruction(bool aIgnore)
 {
   if (mDocument) {
     // We need to tell the ImageLoader to drop all its references to frames
@@ -4210,23 +4178,16 @@ PresShell::FlushPendingNotifications(moz
   nsRefPtr<nsViewManager> viewManagerDeathGrip = mViewManager;
   bool didStyleFlush = false;
   bool didLayoutFlush = false;
   if (isSafeToFlush && mViewManager) {
     // Processing pending notifications can kill us, and some callers only
     // hold weak refs when calling FlushPendingNotifications().  :(
     nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
 
-    if (mResizeEvent.IsPending()) {
-      FireResizeEvent();
-      if (mIsDestroying) {
-        return;
-      }
-    }
-
     // We need to make sure external resource documents are flushed too (for
     // example, svg filters that reference a filter in an external document
     // need the frames in the external document to be constructed for the
     // filter to work). We only need external resources to be flushed when the
     // main document is flushing >= Flush_Frames, so we flush external
     // resources here instead of nsDocument::FlushPendingNotifications.
     mDocument->FlushExternalResources(flushType);
 
--- a/layout/base/nsPresShell.h
+++ b/layout/base/nsPresShell.h
@@ -385,16 +385,18 @@ public:
   virtual void RecordShadowStyleChange(mozilla::dom::ShadowRoot* aShadowRoot) override;
 
   virtual void DispatchAfterKeyboardEvent(nsINode* aTarget,
                                           const mozilla::WidgetKeyboardEvent& aEvent,
                                           bool aEmbeddedCancelled) override;
 
   void SetNextPaintCompressed() { mNextPaintCompressed = true; }
 
+  virtual void FireResizeEvent() override;
+
 protected:
   virtual ~PresShell();
 
   void HandlePostedReflowCallbacks(bool aInterruptible);
   void CancelPostedReflowCallbacks();
 
   void UnsuppressAndInvalidate();
 
@@ -695,19 +697,16 @@ protected:
 
   // Get the selected item and coordinates in device pixels relative to root
   // document's root view for element, first ensuring the element is onscreen
   void GetCurrentItemAndPositionForElement(nsIDOMElement *aCurrentEl,
                                            nsIContent **aTargetToUse,
                                            mozilla::LayoutDeviceIntPoint& aTargetPt,
                                            nsIWidget *aRootWidget);
 
-  void FireResizeEvent();
-  static void AsyncResizeEventCallback(nsITimer* aTimer, void* aPresShell);
-
   virtual void SynthesizeMouseMove(bool aFromScroll) override;
 
   PresShell* GetRootPresShell();
 
   nscolor GetDefaultBackgroundColorToDraw();
 
   DOMHighResTimeStamp GetPerformanceNow();
 
@@ -787,18 +786,16 @@ protected:
   // Set of frames that we should mark with NS_FRAME_HAS_DIRTY_CHILDREN after
   // we finish reflowing mCurrentReflowRoot.
   nsTHashtable<nsPtrHashKey<nsIFrame> > mFramesToDirty;
 
   // Reflow roots that need to be reflowed.
   nsTArray<nsIFrame*>       mDirtyRoots;
 
   nsTArray<nsAutoPtr<DelayedEvent> > mDelayedEvents;
-  nsRevocableEventPtr<nsRunnableMethod<PresShell> > mResizeEvent;
-  nsCOMPtr<nsITimer>        mAsyncResizeEventTimer;
 private:
   nsIFrame*                 mCurrentEventFrame;
   nsCOMPtr<nsIContent>      mCurrentEventContent;
   nsTArray<nsIFrame*>       mCurrentEventFrameStack;
   nsCOMArray<nsIContent>    mCurrentEventContentStack;
 protected:
   nsRevocableEventPtr<nsSynthMouseMoveEvent> mSynthMouseMoveEvent;
   nsCOMPtr<nsIContent>      mLastAnchorScrolledTo;
@@ -856,18 +853,17 @@ protected:
   // We've been disconnected from the document.  We will refuse to paint the
   // document until either our timer fires or all frames are constructed.
   bool                      mIsDocumentGone : 1;
 
   // Indicates that it is safe to unlock painting once all pending reflows
   // have been processed.
   bool                      mShouldUnsuppressPainting : 1;
 
-  bool                      mAsyncResizeTimerIsActive : 1;
-  bool                      mInResize : 1;
+  bool                      mResizeEventPending : 1;
 
   bool                      mImageVisibilityVisited : 1;
 
   bool                      mNextPaintCompressed : 1;
 
   bool                      mHasCSSBackgroundColor : 1;
 
   // Whether content should be scaled by the resolution amount. If this is
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -1325,16 +1325,17 @@ nsRefreshDriver::ObserverCount() const
   for (uint32_t i = 0; i < ArrayLength(mObservers); ++i) {
     sum += mObservers[i].Length();
   }
 
   // Even while throttled, we need to process layout and style changes.  Style
   // changes can trigger transitions which fire events when they complete, and
   // layout changes can affect media queries on child documents, triggering
   // style changes, etc.
+  sum += mResizeEventFlushObservers.Length();
   sum += mStyleFlushObservers.Length();
   sum += mLayoutFlushObservers.Length();
   sum += mFrameRequestCallbackDocs.Length();
   sum += mThrottledFrameRequestCallbackDocs.Length();
   sum += mViewManagerFlushIsPending;
   return sum;
 }
 
@@ -1622,16 +1623,34 @@ nsRefreshDriver::Tick(int64_t aNowEpoch,
   }
 
   AutoRestore<bool> restoreInRefresh(mInRefresh);
   mInRefresh = true;
 
   AutoRestore<TimeStamp> restoreTickStart(mTickStart);
   mTickStart = TimeStamp::Now();
 
+  // Resize events should be fired before layout flushes or
+  // calling animation frame callbacks.
+  nsAutoTArray<nsIPresShell*, 16> observers;
+  observers.AppendElements(mResizeEventFlushObservers);
+  for (uint32_t i = observers.Length(); i; --i) {
+    if (!mPresContext || !mPresContext->GetPresShell()) {
+      break;
+    }
+    // Make sure to not process observers which might have been removed
+    // during previous iterations.
+    nsIPresShell* shell = observers[i - 1];
+    if (!mResizeEventFlushObservers.Contains(shell)) {
+      continue;
+    }
+    mResizeEventFlushObservers.RemoveElement(shell);
+    shell->FireResizeEvent();
+  }
+
   /*
    * 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
@@ -144,16 +144,32 @@ public:
    * imgIRequests to the nsRefreshDriver for notification of paint events.
    *
    * @returns whether the operation succeeded, or void in the case of removal.
    */
   bool AddImageRequest(imgIRequest* aRequest);
   void RemoveImageRequest(imgIRequest* aRequest);
 
   /**
+   * Add / remove presshells which have pending resize event.
+   */
+  void AddResizeEventFlushObserver(nsIPresShell* aShell)
+  {
+    NS_ASSERTION(!mResizeEventFlushObservers.Contains(aShell),
+      "Double-adding resize event flush observer");
+    mResizeEventFlushObservers.AppendElement(aShell);
+    EnsureTimerStarted();
+  }
+
+  void RemoveResizeEventFlushObserver(nsIPresShell* aShell)
+  {
+    mResizeEventFlushObservers.RemoveElement(aShell);
+  }
+
+  /**
    * Add / remove presshells that we should flush style and layout on
    */
   bool AddStyleFlushObserver(nsIPresShell* aShell) {
     NS_ASSERTION(!mStyleFlushObservers.Contains(aShell),
 		 "Double-adding style flush observer");
     // We only get the cause for the first observer each frame because capturing
     // a stack is expensive. This is still useful if (1) you're trying to remove
     // all flushes for a particial frame or (2) the costly flush is triggered
@@ -387,16 +403,17 @@ private:
   mozilla::TimeStamp mTickStart;
   mozilla::TimeStamp mNextThrottledFrameRequestTick;
 
   // separate arrays for each flush type we support
   ObserverArray mObservers[3];
   RequestTable mRequests;
   ImageStartTable mStartTable;
 
+  nsAutoTArray<nsIPresShell*, 16> mResizeEventFlushObservers;
   nsAutoTArray<nsIPresShell*, 16> mStyleFlushObservers;
   nsAutoTArray<nsIPresShell*, 16> mLayoutFlushObservers;
   nsAutoTArray<nsIPresShell*, 16> mPresShellsToInvalidateIfHidden;
   // nsTArray on purpose, because we want to be able to swap.
   nsTArray<nsIDocument*> mFrameRequestCallbackDocs;
   nsTArray<nsIDocument*> mThrottledFrameRequestCallbackDocs;
   nsTArray<nsAPostRefreshObserver*> mPostRefreshObservers;
 
--- a/layout/reftests/font-face/ex-unit-1-dynamic.html
+++ b/layout/reftests/font-face/ex-unit-1-dynamic.html
@@ -10,11 +10,11 @@ body { font-family: Ahhhem; font-size: 5
 <script type="application/ecmascript">
 function run() {
   document.getElementsByTagName("iframe")[0].contentWindow.arm();
   document.getElementsByTagName("style")[0].sheet.insertRule(
     '@font-face { font-family: "Ahhhem"; src: url(../fonts/Ahem.ttf); }',
     0);
 }
 </script>
-<body onload="setTimeout(run, 0)">
+<body onload="setTimeout(run, 20)">
 <iframe style="visibility:hidden;position:absolute;height:100%;width:100%" src="resize-detector-iframe.html"></iframe>
 </body>