Bug 1400618 part 2 - Remove the nsIFrame::GenConProperty property and make aPostDestroyData deal with unbinding it. r=bz
authorMats Palmgren <mats@mozilla.com>
Tue, 07 Nov 2017 01:20:34 +0100
changeset 443693 9a72b179c10dc970bd708ebfa551c8010e26d921
parent 443692 e4815e8465a2cc7d09d479dd76e7143a09927fd3
child 443694 50c2aba68909c45c8136fd5ba20865e9c0309b87
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1400618
milestone58.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 1400618 part 2 - Remove the nsIFrame::GenConProperty property and make aPostDestroyData deal with unbinding it. r=bz MozReview-Commit-ID: HJ5OTJ4v4Y1
dom/base/test/test_mutationobserver_anonymous.html
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
--- a/dom/base/test/test_mutationobserver_anonymous.html
+++ b/dom/base/test/test_mutationobserver_anonymous.html
@@ -151,21 +151,21 @@ function testRemovedDueToDisplay() {
     is(records.length, 2, "Correct number of records");
     is(records[0].type, "nativeAnonymousChildList", "Correct record type (1)");
     is(records[1].type, "nativeAnonymousChildList", "Correct record type (2)");
     is(records[0].target, parent, "Correct target (1)");
     is(records[1].target, parent, "Correct target (2)");
 
     is(records[0].addedNodes.length, 0, "Shouldn't have got addedNodes");
     is(records[0].removedNodes.length, 1, "Should have got removedNodes");
-    assertSamePseudoElement("before", records[0].removedNodes[0], originalBeforeElement);
+    assertSamePseudoElement("after", records[0].removedNodes[0], originalAfterElement);
 
     is(records[1].addedNodes.length, 0, "Shouldn't have got addedNodes");
     is(records[1].removedNodes.length, 1, "Should have got removedNodes");
-    assertSamePseudoElement("after", records[1].removedNodes[0], originalAfterElement);
+    assertSamePseudoElement("before", records[1].removedNodes[0], originalBeforeElement);
 
     observer.disconnect();
     testAddedDueToDisplay();
   });
 
   SpecialPowers.observeMutationEvents(m, parent, true);
   parent.style.display = "none";
 }
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6188,16 +6188,17 @@ nsCSSFrameConstructor::AddFrameConstruct
                              pendingBinding, styleContext.forget(),
                              aSuppressWhiteSpaceOptimizations, aAnonChildren);
   }
   item->mIsText = isText;
   item->mIsGeneratedContent = isGeneratedContent;
   item->mIsAnonymousContentCreatorContent =
     aFlags & ITEM_IS_ANONYMOUSCONTENTCREATOR_CONTENT;
   if (isGeneratedContent) {
+    // We need to keep this alive until the frame takes ownership.
     NS_ADDREF(item->mContent);
   }
   item->mIsRootPopupgroup =
     aNameSpaceID == kNameSpaceID_XUL && aTag == nsGkAtoms::popupgroup &&
     aContent->IsRootOfNativeAnonymousSubtree();
   if (item->mIsRootPopupgroup) {
     aState.mHavePendingPopupgroup = true;
   }
@@ -6277,32 +6278,16 @@ nsCSSFrameConstructor::AddFrameConstruct
   if ((bits & FCDATA_IS_LINE_PARTICIPANT) &&
       ((bits & FCDATA_DISALLOW_OUT_OF_FLOW) ||
        !aState.GetGeometricParent(display, nullptr))) {
     item->mIsLineParticipant = true;
     aItems.LineParticipantItemAdded();
   }
 }
 
-static void
-AddGenConPseudoToFrame(nsIFrame* aOwnerFrame, nsIContent* aContent)
-{
-  // FIXME(emilio): Remove this property, and use the frame of the generated
-  // content itself to tear the content down? It should be quite simpler.
-
-  aOwnerFrame = nsLayoutUtils::FirstContinuationOrIBSplitSibling(aOwnerFrame);
-  nsIFrame::ContentArray* value =
-    aOwnerFrame->GetProperty(nsIFrame::GenConProperty());
-  if (!value) {
-    value = new nsIFrame::ContentArray;
-    aOwnerFrame->AddProperty(nsIFrame::GenConProperty(), value);
-  }
-  value->AppendElement(aContent);
-}
-
 /**
  * Return true if the frame construction item pointed to by aIter will
  * create a frame adjacent to a line boundary in the frame tree, and that
  * line boundary is induced by a content node adjacent to the frame's
  * content node in the content tree. The latter condition is necessary so
  * that ContentAppended/ContentInserted/ContentRemoved can easily find any
  * text nodes that were suppressed here.
  */
@@ -6388,32 +6373,31 @@ nsCSSFrameConstructor::ConstructFramesFr
   // guaranteed that they will be started before onload fires.
   styleContext->StartBackgroundImageLoads();
 
   nsFrameState savedStateBits = aState.mAdditionalStateBits;
   if (item.mIsGeneratedContent) {
     // Ensure that frames created here are all tagged with
     // NS_FRAME_GENERATED_CONTENT.
     aState.mAdditionalStateBits |= NS_FRAME_GENERATED_CONTENT;
-
-    // Note that we're not necessarily setting this property on the primary
-    // frame for the content for which this is generated content.  We might be
-    // setting it on a table pseudo-frame inserted under that instead.  That's
-    // OK, though; we just need to do the property set so that the content will
-    // get cleaned up when the frame is destroyed.
-    ::AddGenConPseudoToFrame(aParentFrame, item.mContent);
+  }
+
+  // XXXbz maybe just inline ConstructFrameFromItemInternal here or something?
+  ConstructFrameFromItemInternal(item, aState, adjParentFrame, aFrameItems);
+
+  if (item.mIsGeneratedContent) {
+    // This corresponds to the AddRef in AddFrameConstructionItemsInternal.
+    // The frame owns the generated content now.
+    item.mContent->Release();
 
     // Now that we've passed ownership of item.mContent to the frame, unset
     // our generated content flag so we don't release or unbind it ourselves.
     item.mIsGeneratedContent = false;
   }
 
-  // XXXbz maybe just inline ConstructFrameFromItemInternal here or something?
-  ConstructFrameFromItemInternal(item, aState, adjParentFrame, aFrameItems);
-
   aState.mAdditionalStateBits = savedStateBits;
 }
 
 
 inline bool
 IsRootBoxFrame(nsIFrame *aFrame)
 {
   return (aFrame->IsRootFrame());
@@ -6538,28 +6522,27 @@ nsCSSFrameConstructor::GetFloatContainin
  */
 static nsContainerFrame*
 AdjustAppendParentForAfterContent(nsFrameManager* aFrameManager,
                                   nsIContent* aContainer,
                                   nsContainerFrame* aParentFrame,
                                   nsIContent* aChild,
                                   nsIFrame** aAfterFrame)
 {
-  // If the parent frame has any pseudo-elements or aContainer is a
-  // display:contents node then we need to walk through the child
-  // frames to find the first one that is either a ::after frame for an
-  // ancestor of aChild or a frame that is for a node later in the
-  // document than aChild and return that in aAfterFrame.
-  nsIFrame* afterBeforeOwnerFrame =
-    nsLayoutUtils::FirstContinuationOrIBSplitSibling(aParentFrame);
-  if (afterBeforeOwnerFrame->GetProperty(nsIFrame::GenConProperty()) ||
-      nsLayoutUtils::HasPseudoStyle(aContainer, aParentFrame->StyleContext(),
+  // If the parent frame may have an ::after pseudo or aContainer is a
+  // display:contents node or aContainer have display:contents children
+  // then we need to walk through the child frames to find the first one
+  // that is either a ::after frame for an ancestor of aChild or a frame
+  // that is for a node later in the document than aChild and return that
+  // in aAfterFrame.
+  if (nsLayoutUtils::HasPseudoStyle(aContainer, aParentFrame->StyleContext(),
                                     CSSPseudoElementType::after,
                                     aParentFrame->PresContext()) ||
-      aFrameManager->GetDisplayContentsStyleFor(aContainer)) {
+      aFrameManager->GetDisplayContentsStyleFor(aContainer) ||
+      aFrameManager->GetAllRegisteredDisplayContentsStylesIn(aContainer)) {
     nsIFrame* afterFrame = nullptr;
     nsContainerFrame* parent =
       static_cast<nsContainerFrame*>(aParentFrame->LastContinuation());
     bool done = false;
     while (!done && parent) {
       // Ensure that all normal flow children are on the principal child list.
       parent->DrainSelfOverflowList();
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -822,16 +822,23 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
   if (view) {
     view->SetFrame(nullptr);
     view->Destroy();
   }
 
   // Make sure that our deleted frame can't be returned from GetPrimaryFrame()
   if (isPrimaryFrame) {
     mContent->SetPrimaryFrame(nullptr);
+
+    // Pass the root of a generated content subtree (e.g. ::after/::before) to
+    // aPostDestroyData to unbind it after frame destruction is done.
+    if (HasAnyStateBits(NS_FRAME_GENERATED_CONTENT) &&
+        mContent->IsRootOfNativeAnonymousSubtree()) {
+      aPostDestroyData.AddGeneratedContent(mContent.forget());
+    }
   }
 
   // Delete all properties attached to the frame, to ensure any property
   // destructors that need the frame pointer are handled properly.
   DeleteAllProperties();
 
   // Must retrieve the object ID before calling destructors, so the
   // vtable is still valid.
@@ -10842,26 +10849,16 @@ nsIFrame::CreateOwnLayerIfNeeded(nsDispl
 bool
 nsIFrame::IsSelected() const
 {
   return (GetContent() && GetContent()->IsSelectionDescendant()) ?
     IsFrameSelected() : false;
 }
 
 /*static*/ void
-nsIFrame::DestroyContentArray(ContentArray* aArray)
-{
-  for (nsIContent* content : *aArray) {
-    content->UnbindFromTree();
-    NS_RELEASE(content);
-  }
-  delete aArray;
-}
-
-/*static*/ void
 nsIFrame::DestroyWebRenderUserDataTable(WebRenderUserDataTable* aTable)
 {
   for (auto iter = aTable->Iter(); !iter.Done(); iter.Next()) {
     iter.UserData()->RemoveFromTable();
   }
   delete aTable;
 }
 
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1156,19 +1156,16 @@ public:
                                  aContainerSize - mRect.Size());
   }
 
   virtual nsPoint GetPositionOfChildIgnoringScrolling(nsIFrame* aChild)
   { return aChild->GetPosition(); }
 
   nsPoint GetPositionIgnoringScrolling();
 
-  typedef AutoTArray<nsIContent*, 2> ContentArray;
-  static void DestroyContentArray(ContentArray* aArray);
-
   typedef AutoTArray<nsDisplayItem*, 4> DisplayItemArray;
 
   typedef mozilla::layers::WebRenderUserData WebRenderUserData;
   typedef nsRefPtrHashtable<nsUint32HashKey, WebRenderUserData> WebRenderUserDataTable;
 
 #define NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(prop, type, dtor)             \
   static const mozilla::FramePropertyDescriptor<type>* prop() {           \
     /* Use of constexpr caused startup crashes with MSVC2015u1 PGO. */    \
@@ -1255,19 +1252,16 @@ public:
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(FragStretchBSizeProperty, nscoord)
 
   // The block-axis margin-box size associated with eBClampMarginBoxMinSize.
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BClampMarginBoxMinSizeProperty, nscoord)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(IBaselinePadProperty, nscoord)
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BBaselinePadProperty, nscoord)
 
-  NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(GenConProperty, ContentArray,
-                                      DestroyContentArray)
-
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(ModifiedFrameList, std::vector<WeakFrame>)
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(DisplayItems, DisplayItemArray)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BidiDataProperty, mozilla::FrameBidiData)
 
   NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(PlaceholderFrameProperty, nsPlaceholderFrame)
   NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(WebRenderUserDataProperty, WebRenderUserDataTable, DestroyWebRenderUserDataTable)