Bug 1430844: Make resize reflow sound re. viewport units. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 16 Jan 2018 19:42:28 +0100
changeset 721005 285fc05f28a00f4c8fad3d16c5b4af0f18707cf2
parent 720918 7978192f79bf10d81c0d971c32f5586759915c47
child 721147 ca5d7696eafd1b6ada603212d48f7e50083b74c3
push id95730
push userbmo:emilio@crisal.io
push dateTue, 16 Jan 2018 18:54:37 +0000
reviewersbz
bugs1430844
milestone59.0a1
Bug 1430844: Make resize reflow sound re. viewport units. r?bz In particular, we set the pres context visible area _before_ processing restyles. This causes inconsistencies when resolving viewport units. In particular, the resulting style tree will have some units resolved in terms of the old size, and some in terms of the new size, depending on whatever is dirty, because we don't flush the pending media query changes. Also, some sizes are resolved against the unconstrained size because of the shrink-to-fit stuff. Fix this by flushing _before_ in this case, instead of after, since we're going to set the size to an actual value later when reflowing the root frame. MozReview-Commit-ID: ExI5yTJCjGp
layout/base/PresShell.cpp
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -1949,51 +1949,61 @@ PresShell::ResizeReflowIgnoreOverride(ns
     mPresContext->SetVisibleArea(nsRect(0, 0, aWidth, aHeight));
     // There isn't anything useful we can do if the initial reflow hasn't
     // happened.
     return NS_OK;
   }
 
   WritingMode wm = rootFrame->GetWritingMode();
   const bool shrinkToFit = aOptions == ResizeReflowOptions::eBSizeLimit;
-  NS_PRECONDITION(shrinkToFit ||
-                  (wm.IsVertical() ? aWidth : aHeight) !=
-                    NS_UNCONSTRAINEDSIZE,
-                  "unconstrained bsize only usable with eBSizeLimit");
-  NS_PRECONDITION((wm.IsVertical() ? aHeight : aWidth) !=
-                    NS_UNCONSTRAINEDSIZE,
-                  "unconstrained isize not allowed");
+  MOZ_ASSERT(shrinkToFit ||
+             (wm.IsVertical() ? aWidth : aHeight) != NS_UNCONSTRAINEDSIZE,
+             "unconstrained bsize only usable with eBSizeLimit");
+  MOZ_ASSERT((wm.IsVertical() ? aHeight : aWidth) != NS_UNCONSTRAINEDSIZE,
+             "unconstrained isize not allowed");
   bool isBSizeChanging = wm.IsVertical() ? aOldWidth != aWidth
                                          : aOldHeight != aHeight;
   nscoord targetWidth = aWidth;
   nscoord targetHeight = aHeight;
 
   if (shrinkToFit) {
     if (wm.IsVertical()) {
       targetWidth = NS_UNCONSTRAINEDSIZE;
     } else {
       targetHeight = NS_UNCONSTRAINEDSIZE;
     }
     isBSizeChanging = true;
   }
 
-  mPresContext->SetVisibleArea(nsRect(0, 0, targetWidth, targetHeight));
+  const bool suppressingResizeReflow =
+    GetPresContext()->SuppressingResizeReflow();
 
   RefPtr<nsViewManager> viewManager = mViewManager;
   nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
 
-  if (!GetPresContext()->SuppressingResizeReflow()) {
-    // Have to make sure that the content notifications are flushed before we
-    // start messing with the frame model; otherwise we can get content doubling.
-    mDocument->FlushPendingNotifications(FlushType::ContentAndNotify);
-
-    // Make sure style is up to date
-    {
-      nsAutoScriptBlocker scriptBlocker;
-      mPresContext->RestyleManager()->ProcessPendingRestyles();
+  if (!suppressingResizeReflow && shrinkToFit) {
+    // Make sure that style is flushed before setting the pres context
+    // VisibleArea if we're shrinking to fit.
+    //
+    // Otherwise we may end up with bogus viewport units resolved against the
+    // unconstrained bsize, or restyling the whole document resolving viewport
+    // units against targetWidth, which may end up doing wasteful work.
+    mDocument->FlushPendingNotifications(FlushType::Frames);
+  }
+
+  mPresContext->SetVisibleArea(nsRect(0, 0, targetWidth, targetHeight));
+  if (!suppressingResizeReflow) {
+    if (!shrinkToFit) {
+      // Flush styles _now_ (with the correct visible area) if not shrinking.
+      //
+      // We've asserted above that sizes are not unconstrained, so this is going
+      // to be the final size, which means that we'll get the (correct) final
+      // styles now, and avoid a further potentially-wasteful full recascade on
+      // the next flush.
+      mDocument->FlushPendingNotifications(FlushType::Frames);
     }
 
     rootFrame = mFrameConstructor->GetRootFrame();
     if (!mIsDestroying && rootFrame) {
       // XXX Do a full invalidate at the beginning so that invalidates along
       // the way don't have region accumulation issues?
 
       if (isBSizeChanging) {