Bug 1397091: Allow calling RecreateFramesForContent with async insertion for non-elements. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 07 Sep 2017 16:24:29 +0200
changeset 429239 e51ad458391342c3b9e37fd827c1bc4f4b2264c3
parent 429238 dee58c528463ec9447a9c478c151ba24d982a3cb
child 429240 cb5a5cf16f1bd33cb02e238f80f6fe48d6945c53
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1397091
milestone57.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 1397091: Allow calling RecreateFramesForContent with async insertion for non-elements. r=bz Using the lazy frame construction path instead of the PostRestyleEvent path. It'd be nice to unify those though... MozReview-Commit-ID: CwDHZQUBm8e
layout/base/nsCSSFrameConstructor.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -9003,30 +9003,25 @@ nsCSSFrameConstructor::CharacterDataChan
       (aContent->HasFlag(NS_REFRAME_IF_WHITESPACE) &&
        aContent->TextIsOnlyWhitespace())) {
 #ifdef DEBUG
     nsIFrame* frame = aContent->GetPrimaryFrame();
     NS_ASSERTION(!frame || !frame->IsGeneratedContentFrame(),
                  "Bit should never be set on generated content");
 #endif
     LAYOUT_PHASE_TEMP_EXIT();
-    RecreateFramesForContent(aContent, InsertionKind::Sync);
+    RecreateFramesForContent(aContent, InsertionKind::Async);
     LAYOUT_PHASE_TEMP_REENTER();
     return;
   }
 
-  // Find the child frame
-  nsIFrame* frame = aContent->GetPrimaryFrame();
-
-  // Notify the first frame that maps the content. It will generate a reflow
-  // command
 
   // It's possible the frame whose content changed isn't inserted into the
   // frame hierarchy yet, or that there is no frame that maps the content
-  if (nullptr != frame) {
+  if (nsIFrame* frame = aContent->GetPrimaryFrame()) {
 #if 0
     NS_FRAME_LOG(NS_FRAME_TRACE_CALLS,
        ("nsCSSFrameConstructor::CharacterDataChanged: content=%p[%s] subcontent=%p frame=%p",
         aContent, ContentTag(aContent, 0),
         aSubContent, frame));
 #endif
 
     // Special check for text content that is a child of a letter frame.  If
@@ -9047,16 +9042,18 @@ nsCSSFrameConstructor::CharacterDataChan
         RemoveLetterFrames(mPresShell, block);
         // Reget |frame|, since we might have killed it.
         // Do we really need to call CharacterDataChanged in this case, though?
         frame = aContent->GetPrimaryFrame();
         NS_ASSERTION(frame, "Should have frame here!");
       }
     }
 
+    // Notify the first frame that maps the content. It will generate a reflow
+    // command
     frame->CharacterDataChanged(aInfo);
 
     if (haveFirstLetterStyle) {
       RecoverLetterFrames(block);
     }
   }
 }
 
@@ -9963,17 +9960,16 @@ nsCSSFrameConstructor::UpdateTableCellSp
     cellFrame->GetTableFrame()->RowOrColSpanChanged(cellFrame);
   }
 }
 
 void
 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent,
                                                 InsertionKind aInsertionKind)
 {
-  MOZ_ASSERT(aInsertionKind != InsertionKind::Async || aContent->IsElement());
   MOZ_ASSERT(aContent);
 
   // If there is no document, we don't want to recreate frames for it.  (You
   // shouldn't generally be giving this method content without a document
   // anyway).
   // Rebuilding the frame tree can have bad effects, especially if it's the
   // frame tree for chrome (see bug 157322).
   if (NS_WARN_IF(!aContent->GetComposedDoc())) {
@@ -10048,42 +10044,49 @@ nsCSSFrameConstructor::RecreateFramesFor
   // XXXbz how can containerNode be null here?
   if (containerNode) {
     // Before removing the frames associated with the content object,
     // ask them to save their state onto a temporary state object.
     CaptureStateForFramesOf(aContent, mTempFrameTreeState);
 
     // Need the nsIContent parent, which might be null here, since we need to
     // pass it to ContentInserted and ContentRemoved.
-    //
-    // FIXME(emilio): Do we need a strong ref here?
-    nsCOMPtr<nsIContent> container = aContent->GetParent();
+    nsIContent* container = aContent->GetParent();
 
     // Remove the frames associated with the content object.
     nsIContent* nextSibling = aContent->IsRootOfAnonymousSubtree() ?
       nullptr : aContent->GetNextSibling();
     bool didReconstruct =
       ContentRemoved(container, aContent, nextSibling, aInsertionKind,
                      REMOVE_FOR_RECONSTRUCTION);
 
     if (!didReconstruct) {
-      if (aInsertionKind == InsertionKind::Async) {
+      if (aInsertionKind == InsertionKind::Async && aContent->IsElement()) {
         // FIXME(emilio, bug 1397239): There's nothing removing the frame state
         // for elements that go away before we come back to the frame
         // constructor.
+        //
+        // Also, it'd be nice to just use the `ContentRangeInserted` path for
+        // both elements and non-elements, but we need to make lazy frame
+        // construction to apply to all elements first.
         RestyleManager()->PostRestyleEvent(aContent->AsElement(),
                                            nsRestyleHint(0),
                                            nsChangeHint_ReconstructFrame);
       } else {
         // Now, recreate the frames associated with this content object. If
         // ContentRemoved triggered reconstruction, then we don't need to do this
         // because the frames will already have been built.
+        auto lazyFrameConstructionAllowed =
+          aInsertionKind == InsertionKind::Async
+            ? LazyConstructionAllowed::Yes
+            : LazyConstructionAllowed::No;
+
         ContentRangeInserted(container, aContent, aContent->GetNextSibling(),
                              mTempFrameTreeState,
-                             LazyConstructionAllowed::No,
+                             lazyFrameConstructionAllowed,
                              true,  // aForReconstruction
                              nullptr);
       }
     }
   }
 }
 
 bool