Bug 1437790: No need to reframe synchronously on chrome-flush-skin-caches. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 13 Feb 2018 10:38:30 +0100
changeset 458777 36480ec73c274b6e671fcf4498d051410e318434
parent 458776 7080e442652d1a3725b68f7ce36348360276aa73
child 458778 36e6f059546183aa8e93d4de4112745cf2ea4730
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1437790
milestone60.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 1437790: No need to reframe synchronously on chrome-flush-skin-caches. r=xidorn Instead, just post the reframe, and process them normally on the next style flush. The assertion happens because the observer is triggered due to a doc ltr -> rtl change, which triggers style invalidation. There's nothing processing the restyles resulting from that invalidation before we hackily go into frame construction from there, and thus we assert that the styles aren't up-to-date. MozReview-Commit-ID: GnOKFqmtTnq
layout/base/PresShell.cpp
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -9206,68 +9206,70 @@ PresShell::WindowSizeMoveDone()
 #ifdef MOZ_XUL
 /*
  * It's better to add stuff to the |DidSetStyleContext| method of the
  * relevant frames than adding it here.  These methods should (ideally,
  * anyway) go away.
  */
 
 // Return value says whether to walk children.
-typedef bool (* frameWalkerFn)(nsIFrame *aFrame, void *aClosure);
+typedef bool (*frameWalkerFn)(nsIFrame* aFrame);
 
 static bool
-ReResolveMenusAndTrees(nsIFrame *aFrame, void *aClosure)
+ReResolveMenusAndTrees(nsIFrame* aFrame)
 {
   // Trees have a special style cache that needs to be flushed when
   // the theme changes.
-  nsTreeBodyFrame *treeBody = do_QueryFrame(aFrame);
+  nsTreeBodyFrame* treeBody = do_QueryFrame(aFrame);
   if (treeBody)
     treeBody->ClearStyleAndImageCaches();
 
   // We deliberately don't re-resolve style on a menu's popup
   // sub-content, since doing so slows menus to a crawl.  That means we
   // have to special-case them on a skin switch, and ensure that the
   // popup frames just get destroyed completely.
   nsMenuFrame* menu = do_QueryFrame(aFrame);
   if (menu)
     menu->CloseMenu(true);
   return true;
 }
 
 static bool
-ReframeImageBoxes(nsIFrame *aFrame, void *aClosure)
-{
-  nsStyleChangeList *list = static_cast<nsStyleChangeList*>(aClosure);
+ReframeImageBoxes(nsIFrame* aFrame)
+{
   if (aFrame->IsImageBoxFrame()) {
-    list->AppendChange(aFrame, aFrame->GetContent(),
-                       nsChangeHint_ReconstructFrame);
+    aFrame->PresContext()->RestyleManager()->PostRestyleEvent(
+        aFrame->GetContent()->AsElement(),
+        nsRestyleHint(0),
+        nsChangeHint_ReconstructFrame);
     return false; // don't walk descendants
   }
   return true; // walk descendants
 }
 
 static void
-WalkFramesThroughPlaceholders(nsPresContext *aPresContext, nsIFrame *aFrame,
-                              frameWalkerFn aFunc, void *aClosure)
-{
-  bool walkChildren = (*aFunc)(aFrame, aClosure);
+WalkFramesThroughPlaceholders(nsPresContext* aPresContext,
+                              nsIFrame* aFrame,
+                              frameWalkerFn aFunc)
+{
+  bool walkChildren = (*aFunc)(aFrame);
   if (!walkChildren)
     return;
 
   nsIFrame::ChildListIterator lists(aFrame);
   for (; !lists.IsDone(); lists.Next()) {
     nsFrameList::Enumerator childFrames(lists.CurrentList());
     for (; !childFrames.AtEnd(); childFrames.Next()) {
       nsIFrame* child = childFrames.get();
       if (!(child->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) {
         // only do frames that are in flow, and recur through the
         // out-of-flows of placeholders.
         WalkFramesThroughPlaceholders(aPresContext,
                                       nsPlaceholderFrame::GetRealFrameFor(child),
-                                      aFunc, aClosure);
+                                      aFunc);
       }
     }
   }
 }
 #endif
 
 NS_IMETHODIMP
 PresShell::Observe(nsISupports* aSubject,
@@ -9276,47 +9278,28 @@ PresShell::Observe(nsISupports* aSubject
 {
   if (mIsDestroying) {
     NS_WARNING("our observers should have been unregistered by now");
     return NS_OK;
   }
 
 #ifdef MOZ_XUL
   if (!nsCRT::strcmp(aTopic, "chrome-flush-skin-caches")) {
-    nsIFrame *rootFrame = mFrameConstructor->GetRootFrame();
     // Need to null-check because "chrome-flush-skin-caches" can happen
     // at interesting times during startup.
-    if (rootFrame) {
+    if (nsIFrame* rootFrame = mFrameConstructor->GetRootFrame()) {
       NS_ASSERTION(mViewManager, "View manager must exist");
 
-      AutoWeakFrame weakRoot(rootFrame);
-      // 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);
-
-      if (weakRoot.IsAlive()) {
-        WalkFramesThroughPlaceholders(mPresContext, rootFrame,
-                                      &ReResolveMenusAndTrees, nullptr);
-
-        // Because "chrome:" URL equality is messy, reframe image box
-        // frames (hack!).
-        nsStyleChangeList changeList(mPresContext->StyleSet()->BackendType());
-        WalkFramesThroughPlaceholders(mPresContext, rootFrame,
-                                      ReframeImageBoxes, &changeList);
-        // Mark ourselves as not safe to flush while we're doing frame
-        // construction.
-        {
-          nsAutoScriptBlocker scriptBlocker;
-          ++mChangeNestCount;
-          RestyleManager* restyleManager = mPresContext->RestyleManager();
-          restyleManager->ProcessRestyledFrames(changeList);
-          restyleManager->FlushOverflowChangedTracker();
-          --mChangeNestCount;
-        }
-      }
+      WalkFramesThroughPlaceholders(
+          mPresContext, rootFrame, ReResolveMenusAndTrees);
+
+      // Because "chrome:" URL equality is messy, reframe image box
+      // frames (hack!).
+      WalkFramesThroughPlaceholders(
+          mPresContext, rootFrame, ReframeImageBoxes);
     }
     return NS_OK;
   }
 #endif
 
   if (!nsCRT::strcmp(aTopic, "memory-pressure")) {
     if (!AssumeAllFramesVisible() && mPresContext->IsRootContentDocument()) {
       DoUpdateApproximateFrameVisibility(/* aRemoveOnly = */ true);