Bug 1437790: No need to reframe synchronously on chrome-flush-skin-caches. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 13 Feb 2018 10:38:30 +0100
changeset 754806 67312c67d4dffacba9f6d424e4f359a753de83eb
parent 754789 31bd81f251b33b01fbcdae17e824f062b615970f
child 754807 1fc9ef06a1563e58ceb77342fe2ff99600add88d
push id99007
push userbmo:emilio@crisal.io
push dateWed, 14 Feb 2018 10:55:56 +0000
reviewersxidorn
bugs1437790
milestone60.0a1
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);