Bug 1397091: Allow calling RecreateFramesForContent with async insertion for non-elements. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 07 Sep 2017 16:24:29 +0200
changeset 660796 12cbbc56f51cbe14b9b87d3ec8a6495c5d71e389
parent 660782 18a890c863887d57c5b8ece352644c664d4b07ee
child 660797 bb7f6c994b9161c0b7454735e77b016d3c24e2e3
push id78540
push userbmo:emilio@crisal.io
push dateThu, 07 Sep 2017 15:28:10 +0000
bugs1397091
milestone57.0a1
Bug 1397091: Allow calling RecreateFramesForContent with async insertion for non-elements. Kind of bothers me to relax that assertion, but it's the easiest and less risky way to fix this. Let me know if you'd prefer to manually do ContentRemoved + ContentInserted instead though, I can also do that. 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,45 @@ 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.
         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