Bug 1358712 - Force the frameloader to go through layout when content causes a TabParent to be created. r=Nika
authorMike Conley <mconley@mozilla.com>
Thu, 19 Apr 2018 14:26:56 -0700
changeset 468333 52471746531a4e4f3fd4957579ef19fcab9b19e9
parent 468332 4895ec59cf2b5240ad79fa35d3a9d32cc1f206d9
child 468334 a7fc392e48ab87dc3d02c155188c9e433425c7d7
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNika
bugs1358712
milestone61.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 1358712 - Force the frameloader to go through layout when content causes a TabParent to be created. r=Nika This is necessary to avoid web platform test failures for tests that rely on layout calculations occurring inside a recently opened tab or window. Originally, the layout flush was happening "accidentally" within the StatusPanel that displays loading status for the browser. That flush is being removed in another patch in this series. MozReview-Commit-ID: IUxiBS9CDRY
dom/base/nsFrameLoader.cpp
dom/base/nsFrameLoader.h
dom/ipc/ContentParent.cpp
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -1000,16 +1000,38 @@ nsFrameLoader::Hide()
 
   nsCOMPtr<nsIBaseWindow> baseWin = do_QueryInterface(mDocShell);
   NS_ASSERTION(baseWin,
                "Found an nsIDocShell which doesn't implement nsIBaseWindow.");
   baseWin->SetVisibility(false);
   baseWin->SetParentWidget(nullptr);
 }
 
+void
+nsFrameLoader::ForceLayoutIfNecessary()
+{
+  nsIFrame* frame = GetPrimaryFrameOfOwningContent();
+  if (!frame) {
+    return;
+  }
+
+  nsPresContext* presContext = frame->PresContext();
+  if (!presContext) {
+    return;
+  }
+
+  // Only force the layout flush if the frameloader hasn't ever been
+  // run through layout.
+  if (frame->GetStateBits() & NS_FRAME_FIRST_REFLOW) {
+    if (nsCOMPtr<nsIPresShell> shell = presContext->GetPresShell()) {
+      shell->FlushPendingNotifications(FlushType::Layout);
+    }
+  }
+}
+
 nsresult
 nsFrameLoader::SwapWithOtherRemoteLoader(nsFrameLoader* aOther,
                                          nsIFrameLoaderOwner* aThisOwner,
                                          nsIFrameLoaderOwner* aOtherOwner)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
 #ifdef DEBUG
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -228,16 +228,21 @@ public:
 
   /**
    * Called from the layout frame associated with this frame loader, when
    * the frame is being torn down; this notifies us that out widget and view
    * are going away and we should unhook from them.
    */
   void Hide();
 
+  // Used when content is causing a FrameLoader to be created, and
+  // needs to try forcing layout to flush in order to get accurate
+  // dimensions for the content area.
+  void ForceLayoutIfNecessary();
+
   // The guts of an nsIFrameLoaderOwner::SwapFrameLoader implementation.  A
   // frame loader owner needs to call this, and pass in the two references to
   // nsRefPtrs for frame loaders that need to be swapped.
   nsresult SwapWithOtherLoader(nsFrameLoader* aOther,
                                nsIFrameLoaderOwner* aThisOwner,
                                nsIFrameLoaderOwner* aOtherOwner);
 
   nsresult SwapWithOtherRemoteLoader(nsFrameLoader* aOther,
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -4732,16 +4732,22 @@ ContentParent::CommonCreateWindow(PBrows
                                               nsIBrowserDOMWindow::OPEN_NEW,
                                               aNextTabParentId, aName,
                                               getter_AddRefs(frameLoaderOwner));
     }
     if (NS_SUCCEEDED(aResult) && frameLoaderOwner) {
       RefPtr<nsFrameLoader> frameLoader = frameLoaderOwner->GetFrameLoader();
       if (frameLoader) {
         aNewTabParent = frameLoader->GetTabParent();
+        // At this point, it's possible the inserted frameloader hasn't gone through
+        // layout yet. To ensure that the dimensions that we send down when telling the
+        // frameloader to display will be correct (instead of falling back to a 10x10
+        // default), we force layout if necessary to get the most up-to-date dimensions.
+        // See bug 1358712 for details.
+        frameLoader->ForceLayoutIfNecessary();
       }
     } else if (NS_SUCCEEDED(aResult) && !frameLoaderOwner) {
       // Fall through to the normal window opening code path when there is no
       // window which we can open a new tab in.
       openLocation = nsIBrowserDOMWindow::OPEN_NEWWINDOW;
     } else {
       *aWindowIsNew = false;
     }
@@ -4763,16 +4769,36 @@ ContentParent::CommonCreateWindow(PBrows
                                              aNextTabParentId,
                                              !aSetOpener,
                                              getter_AddRefs(aNewTabParent));
   if (NS_WARN_IF(NS_FAILED(aResult))) {
     return IPC_OK();
   }
 
   MOZ_ASSERT(aNewTabParent);
+
+  // At this point, it's possible the inserted frameloader hasn't gone through
+  // layout yet. To ensure that the dimensions that we send down when telling the
+  // frameloader to display will be correct (instead of falling back to a 10x10
+  // default), we force layout if necessary to get the most up-to-date dimensions.
+  // See bug 1358712 for details.
+  //
+  // This involves doing a bit of gymnastics in order to get at the FrameLoader,
+  // so we scope this to avoid polluting the main function scope.
+  {
+    nsCOMPtr<Element> frameElement =
+      TabParent::GetFrom(aNewTabParent)->GetOwnerElement();
+    MOZ_ASSERT(frameElement);
+    nsCOMPtr<nsIFrameLoaderOwner> frameLoaderOwner = do_QueryInterface(frameElement);
+    MOZ_ASSERT(frameLoaderOwner);
+    RefPtr<nsFrameLoader> frameLoader = frameLoaderOwner->GetFrameLoader();
+    MOZ_ASSERT(frameLoader);
+    frameLoader->ForceLayoutIfNecessary();
+  }
+
   // If we were passed a name for the window which would override the default,
   // we should send it down to the new tab.
   if (nsContentUtils::IsOverridingWindowName(aName)) {
     Unused << TabParent::GetFrom(aNewTabParent)->SendSetWindowName(aName);
   }
 
   // Don't send down the OriginAttributes if the content process is handling
   // setting up the window for us. We only want to send them in the async case.