Bug 1381157 - Cache 'mContent->GetPrimaryFrame == this' with a flag on nsIFrame and replace these calls to GetPrimaryFrame. r?mstange draft
authorNeerja Pancholi <npancholi@mozilla.com>
Fri, 18 Aug 2017 15:22:30 -0700
changeset 653269 2d6e28d12cd43973891f5f6fd07c36bba55ab1e8
parent 652136 892c8916ba32b7733e06bfbfdd4083ffae3ca028
child 728303 7b14702b36cec4708d5c9bf6f074c83defa5770a
push id76289
push userbmo:npancholi@mozilla.com
push dateFri, 25 Aug 2017 20:07:05 +0000
reviewersmstange
bugs1381157
milestone57.0a1
Bug 1381157 - Cache 'mContent->GetPrimaryFrame == this' with a flag on nsIFrame and replace these calls to GetPrimaryFrame. r?mstange MozReview-Commit-ID: BGrY9L5DgnN
layout/base/nsCSSFrameConstructor.cpp
layout/forms/nsComboboxControlFrame.cpp
layout/generic/nsFrame.cpp
layout/generic/nsFrameSetFrame.cpp
layout/generic/nsIFrame.h
layout/generic/nsImageMap.cpp
layout/generic/nsSubDocumentFrame.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2794,17 +2794,17 @@ nsCSSFrameConstructor::ConstructDocEleme
     ProcessChildren(state, aDocElement, styleContext, contentFrame, true,
                     childItems, false, nullptr);
 
     // Set the initial child lists
     contentFrame->SetInitialChildList(kPrincipalList, childItems);
   }
 
   // set the primary frame
-  aDocElement->SetPrimaryFrame(contentFrame);
+  nsIFrame::SetPrimaryFrame(contentFrame, aDocElement);
 
   SetInitialSingleChild(mDocElementContainingBlock, newFrame);
 
   // Create frames for anonymous contents if there is a canvas frame.
   if (mDocElementContainingBlock->IsCanvasFrame()) {
     ConstructAnonymousContentForCanvas(state, mDocElementContainingBlock,
                                        aDocElement);
   }
@@ -3600,18 +3600,19 @@ nsCSSFrameConstructor::ConstructTextFram
       }
       initializer->mNode.forget();
     }
   }
 
   // Add the newly constructed frame to the flow
   aFrameItems.AddChild(newFrame);
 
-  if (!aState.mCreatingExtraFrames)
-    aContent->SetPrimaryFrame(newFrame);
+  if (!aState.mCreatingExtraFrames) {
+    nsIFrame::SetPrimaryFrame(newFrame, aContent);
+  }
 }
 
 /* static */
 const nsCSSFrameConstructor::FrameConstructionData*
 nsCSSFrameConstructor::FindDataByInt(int32_t aInt,
                                      Element* aElement,
                                      nsStyleContext* aStyleContext,
                                      const FrameConstructionDataByInt* aDataPtr,
@@ -4267,17 +4268,17 @@ nsCSSFrameConstructor::ConstructFrameFro
   // generated content that doesn't have one yet.  Note that we have to examine
   // the frame bit, because by this point mIsGeneratedContent has been cleared
   // on aItem.
   if ((!aState.mCreatingExtraFrames ||
        (primaryFrame->HasAnyStateBits(NS_FRAME_ANONYMOUSCONTENTCREATOR_CONTENT |
                                       NS_FRAME_GENERATED_CONTENT) &&
         !aItem.mContent->GetPrimaryFrame())) &&
        !(bits & FCDATA_SKIP_FRAMESET)) {
-    aItem.mContent->SetPrimaryFrame(primaryFrame);
+    nsIFrame::SetPrimaryFrame(primaryFrame, aItem.mContent);
     ActiveLayerTracker::TransferActivityToFrame(aItem.mContent, primaryFrame);
   }
 }
 
 static void
 SetFlagsOnSubtree(nsIContent *aNode, uintptr_t aFlagsToSet)
 {
 #ifdef DEBUG
@@ -11916,17 +11917,17 @@ nsCSSFrameConstructor::CreateLetterFrame
 
     // Create a new text frame (the original one will be discarded)
     // pass a temporary stylecontext, the correct one will be set
     // later.  Start off by unsetting the primary frame for
     // aTextContent, so it's no longer pointing to the to-be-destroyed
     // frame.
     // XXXbz it would be really nice to destroy the old frame _first_,
     // then create the new one, so we could avoid this hack.
-    aTextContent->SetPrimaryFrame(nullptr);
+    nsIFrame::SetPrimaryFrame(nullptr, aTextContent);
     nsIFrame* textFrame = NS_NewTextFrame(mPresShell, textSC);
 
     NS_ASSERTION(aBlockContinuation == GetFloatContainingBlock(aParentFrame),
                  "Containing block is confused");
     TreeMatchContextHolder matchContext(mDocument);
     nsFrameConstructorState state(mPresShell,
                                   matchContext,
                                   GetAbsoluteContainingBlock(aParentFrame, FIXED_POS),
@@ -11963,17 +11964,17 @@ nsCSSFrameConstructor::CreateLetterFrame
       aBlockFrame->AddStateBits(NS_BLOCK_HAS_FIRST_LETTER_CHILD);
     }
     MOZ_ASSERT(!aBlockFrame->GetPrevContinuation(),
                "Setting up a first-letter frame on a non-first block continuation?");
     auto parent = static_cast<nsContainerFrame*>(aParentFrame->FirstContinuation());
     parent->SetHasFirstLetterChild();
     aBlockFrame->SetProperty(nsContainerFrame::FirstLetterProperty(),
                              letterFrame);
-    aTextContent->SetPrimaryFrame(textFrame);
+    nsIFrame::SetPrimaryFrame(textFrame, aTextContent);
   }
 }
 
 void
 nsCSSFrameConstructor::WrapFramesInFirstLetterFrame(
   nsContainerFrame*        aBlockFrame,
   nsFrameItems&            aBlockFrames)
 {
@@ -12144,17 +12145,17 @@ nsCSSFrameConstructor::RemoveFloatingFir
          textContent.get(), textFrame, newTextFrame);
 #endif
 
   // Remove placeholder frame and the float
   RemoveFrame(kPrincipalList, placeholderFrame);
 
   // Now that the old frames are gone, we can start pointing to our
   // new primary frame.
-  textContent->SetPrimaryFrame(newTextFrame);
+  nsIFrame::SetPrimaryFrame(newTextFrame, textContent);
 
   // Wallpaper bug 822910.
   bool offsetsNeedFixing = prevSibling && prevSibling->IsTextFrame();
   if (offsetsNeedFixing) {
     prevSibling->AddStateBits(TEXT_OFFSETS_NEED_FIXING);
   }
 
   // Insert text frame in its place
@@ -12199,17 +12200,17 @@ nsCSSFrameConstructor::RemoveFirstLetter
       textFrame = NS_NewTextFrame(aPresShell, newSC);
       textFrame->Init(textContent, aFrame, nullptr);
 
       // Next rip out the kid and replace it with the text frame
       RemoveFrame(kPrincipalList, kid);
 
       // Now that the old frames are gone, we can start pointing to our
       // new primary frame.
-      textContent->SetPrimaryFrame(textFrame);
+      nsIFrame::SetPrimaryFrame(textFrame, textContent);
 
       // Wallpaper bug 822910.
       bool offsetsNeedFixing = prevSibling && prevSibling->IsTextFrame();
       if (offsetsNeedFixing) {
         prevSibling->AddStateBits(TEXT_OFFSETS_NEED_FIXING);
       }
 
       // Insert text frame in its place
--- a/layout/forms/nsComboboxControlFrame.cpp
+++ b/layout/forms/nsComboboxControlFrame.cpp
@@ -1382,17 +1382,17 @@ nsComboboxControlFrame::CreateFrameForDi
   mDisplayFrame = new (shell) nsComboboxDisplayFrame(styleContext, this);
   mDisplayFrame->Init(mContent, this, nullptr);
 
   // Create a text frame and put it inside the block frame
   nsIFrame* textFrame = NS_NewTextFrame(shell, textStyleContext);
 
   // initialize the text frame
   textFrame->Init(mDisplayContent, mDisplayFrame, nullptr);
-  mDisplayContent->SetPrimaryFrame(textFrame);
+  nsIFrame::SetPrimaryFrame(textFrame, mDisplayContent);
 
   nsFrameList textList(textFrame, textFrame);
   mDisplayFrame->SetInitialChildList(kPrincipalList, textList);
   return mDisplayFrame;
 }
 
 void
 nsComboboxControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -735,16 +735,20 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
                  nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, placeholder),
                  "Placeholder relationship should have been torn down already; "
                  "this might mean we have a stray placeholder in the tree.");
     if (placeholder) {
       placeholder->SetOutOfFlowFrame(nullptr);
     }
   }
 
+  // XXXneerja All instances of 'mContent->GetPrimaryFrame() == this'
+  // have been replaced except for this one. The reason is that we must recheck
+  // (IsInUncomposedDoc() || IsInShadowTree()) here else it
+  // causes an assertion failure. See Bug 1381157.
   bool isPrimaryFrame = (mContent && mContent->GetPrimaryFrame() == this);
   if (isPrimaryFrame) {
     // This needs to happen before we clear our Properties() table.
     ActiveLayerTracker::TransferActivityToContent(this, mContent);
 
     // Unfortunately, we need to do this for all frames being reframed
     // and not only those whose current style involves CSS transitions,
     // because what matters is whether the new style (not the old)
@@ -792,17 +796,17 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
   nsView* view = GetView();
   if (view) {
     view->SetFrame(nullptr);
     view->Destroy();
   }
 
   // Make sure that our deleted frame can't be returned from GetPrimaryFrame()
   if (isPrimaryFrame) {
-    mContent->SetPrimaryFrame(nullptr);
+    nsIFrame::SetPrimaryFrame(nullptr, mContent);
   }
 
   // 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.
@@ -1332,32 +1336,32 @@ bool
 nsIFrame::HasAnimationOfTransform(EffectSet* aEffectSet) const
 {
   EffectSet* effects =
     aEffectSet ? aEffectSet : EffectSet::GetEffectSet(this);
 
   return mContent &&
     nsLayoutUtils::HasAnimationOfProperty(effects, eCSSProperty_transform) &&
     IsFrameOfType(eSupportsCSSTransforms) &&
-    mContent->GetPrimaryFrame() == this;
+    mIsPrimaryFrame;
 }
 
 bool
 nsIFrame::HasOpacityInternal(float aThreshold,
                              EffectSet* aEffectSet) const
 {
   MOZ_ASSERT(0.0 <= aThreshold && aThreshold <= 1.0, "Invalid argument");
   if (StyleEffects()->mOpacity < aThreshold ||
       (StyleDisplay()->mWillChangeBitField & NS_STYLE_WILL_CHANGE_OPACITY)) {
     return true;
   }
 
   EffectSet* effects =
     aEffectSet ? aEffectSet : EffectSet::GetEffectSet(this);
-  return (mContent && mContent->GetPrimaryFrame() == this &&
+  return ( mIsPrimaryFrame &&
           nsLayoutUtils::HasAnimationOfProperty(effects, eCSSProperty_opacity));
 }
 
 bool
 nsIFrame::IsSVGTransformed(gfx::Matrix *aOwnTransforms,
                            gfx::Matrix *aFromParentTransforms) const
 {
   return false;
@@ -5982,16 +5986,33 @@ nsFrame::Reflow(nsPresContext*          
 {
   MarkInReflow();
   DO_GLOBAL_REFLOW_COUNT("nsFrame");
   aDesiredSize.ClearSize();
   aStatus.Reset();
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize);
 }
 
+void
+nsIFrame::SetPrimaryFrame(nsIFrame* aFrame, nsIContent* aContent)
+{
+  MOZ_ASSERT(aContent, "Content here cannot be null!");
+
+  if (aFrame) {
+    aFrame->mIsPrimaryFrame = true;
+  } else {
+    nsIFrame* currentPrimaryFrame = aContent->GetPrimaryFrame();
+    if (currentPrimaryFrame) {
+      currentPrimaryFrame->mIsPrimaryFrame = false;
+    }
+  }
+
+  aContent->SetPrimaryFrame(aFrame);
+}
+
 nsresult
 nsFrame::CharacterDataChanged(CharacterDataChangeInfo* aInfo)
 {
   NS_NOTREACHED("should only be called for text frames");
   return NS_OK;
 }
 
 nsresult
@@ -9411,17 +9432,17 @@ nsFrame::DoGetParentStyleContext(nsIFram
     nsIContent* parentContent = mContent->GetFlattenedTreeParent();
     if (MOZ_LIKELY(parentContent)) {
       nsIAtom* pseudo = StyleContext()->GetPseudo();
       if (!pseudo || !mContent->IsElement() ||
           (!nsCSSAnonBoxes::IsAnonBox(pseudo) &&
            // Ensure that we don't return the display:contents style
            // of the parent content for pseudos that have the same content
            // as their primary frame (like -moz-list-bullets do):
-           mContent->GetPrimaryFrame() == this) ||
+           mIsPrimaryFrame) ||
           /* if next is true then it's really a request for the table frame's
              parent context, see nsTable[Outer]Frame::GetParentStyleContext. */
           pseudo == nsCSSAnonBoxes::tableWrapper) {
         nsFrameManager* fm = PresContext()->FrameManager();
         nsStyleContext* sc = fm->GetDisplayContentsStyleFor(parentContent);
         if (MOZ_UNLIKELY(sc)) {
           return sc;
         }
--- a/layout/generic/nsFrameSetFrame.cpp
+++ b/layout/generic/nsFrameSetFrame.cpp
@@ -316,17 +316,17 @@ nsHTMLFramesetFrame::Init(nsIContent*   
       } else { // frame
         frame = NS_NewSubDocumentFrame(shell, kidSC);
 
         frame->Init(child, this, nullptr);
 
         mChildFrameborder[mChildCount] = GetFrameBorder(child);
         mChildBorderColors[mChildCount].Set(GetBorderColor(child));
       }
-      child->SetPrimaryFrame(frame);
+      nsIFrame::SetPrimaryFrame(frame, child);
 
       mFrames.AppendFrame(nullptr, frame);
 
       mChildCount++;
     }
   }
 
   mNonBlankChildCount = mChildCount;
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -617,16 +617,17 @@ public:
     , mPrevSibling(nullptr)
     , mState(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY)
     , mClass(aID)
     , mMayHaveRoundedCorners(false)
     , mHasImageRequest(false)
     , mHasFirstLetterChild(false)
     , mParentIsWrapperAnonBox(false)
     , mIsWrapperBoxNeedingRestyle(false)
+    , mIsPrimaryFrame(false)
   {
     mozilla::PodZero(&mOverflow);
   }
 
   nsPresContext* PresContext() const {
     return StyleContext()->PresContext();
   }
 
@@ -2010,16 +2011,29 @@ public:
    * Checks if the current frame-state includes any of the listed bits
    */
   bool HasAnyStateBits(nsFrameState aBits) const
   {
     return mState & aBits;
   }
 
   /**
+   * Returns if the current frame is the primary frame.
+   */
+  bool IsPrimaryFrame() const
+  {
+    return mIsPrimaryFrame;
+  }
+
+  /**
+   * Mark this frame as the primary frame.
+   */
+   static void SetPrimaryFrame(nsIFrame* aFrame, nsIContent* aContent);
+
+  /**
    * This call is invoked on the primary frame for a character data content
    * node, when it is changed in the content tree.
    */
   virtual nsresult  CharacterDataChanged(CharacterDataChangeInfo* aInfo) = 0;
 
   /**
    * This call is invoked when the value of a content objects's attribute
    * is changed.
@@ -4168,17 +4182,22 @@ protected:
 
   /**
    * True if this is a wrapper anonymous box needing a restyle.  This is used to
    * track, during stylo post-traversal, whether we've already recomputed the
    * style of this anonymous box, if we end up seeing it twice.
    */
   bool mIsWrapperBoxNeedingRestyle : 1;
 
-  // There is an 11-bit gap left here.
+  /**
+   * True if this is the primary frame for the content associated with it
+   */
+  bool mIsPrimaryFrame : 1;
+
+  // There is an 10-bit gap left here.
 
   // Helpers
   /**
    * Can we stop inside this frame when we're skipping non-rendered whitespace?
    * @param  aForward [in] Are we moving forward (or backward) in content order.
    * @param  aOffset [in/out] At what offset into the frame to start looking.
    *         on output - what offset was reached (whether or not we found a place to stop).
    * @return STOP: An appropriate offset was found within this frame,
--- a/layout/generic/nsImageMap.cpp
+++ b/layout/generic/nsImageMap.cpp
@@ -713,17 +713,17 @@ nsImageMap::GetBoundsForAreaContent(nsIC
 void
 nsImageMap::FreeAreas()
 {
   for (auto* area : mAreas) {
     if (area->mArea->IsInUncomposedDoc()) {
       NS_ASSERTION(area->mArea->GetPrimaryFrame() == mImageFrame,
                    "Unexpected primary frame");
 
-      area->mArea->SetPrimaryFrame(nullptr);
+      nsIFrame::SetPrimaryFrame(nullptr, area->mArea);
     }
 
     area->mArea->RemoveSystemEventListener(NS_LITERAL_STRING("focus"), this,
                                            false);
     area->mArea->RemoveSystemEventListener(NS_LITERAL_STRING("blur"), this,
                                            false);
     delete area;
   }
@@ -840,17 +840,17 @@ nsImageMap::AddArea(nsIContent* aArea)
   aArea->AddSystemEventListener(NS_LITERAL_STRING("focus"), this, false, false);
   aArea->AddSystemEventListener(NS_LITERAL_STRING("blur"), this, false, false);
 
   // This is a nasty hack.  It needs to go away: see bug 135040.  Once this is
   // removed, the code added to RestyleManager::RestyleElement,
   // nsCSSFrameConstructor::ContentRemoved (both hacks there), and
   // RestyleManager::ProcessRestyledFrames to work around this issue can
   // be removed.
-  aArea->SetPrimaryFrame(mImageFrame);
+  nsIFrame::SetPrimaryFrame(mImageFrame, aArea);
 
   nsAutoString coords;
   aArea->GetAttr(kNameSpaceID_None, nsGkAtoms::coords, coords);
   area->ParseCoords(coords);
   mAreas.AppendElement(area);
 }
 
 nsIContent*
--- a/layout/generic/nsSubDocumentFrame.cpp
+++ b/layout/generic/nsSubDocumentFrame.cpp
@@ -129,17 +129,17 @@ nsSubDocumentFrame::Init(nsIContent*    
   // CreateView() creates this frame's view, stored in mOuterView.  It needs to
   // be created first since it's the parent of the inner view, stored in
   // mInnerView.
   CreateView();
   EnsureInnerView();
 
   // Set the primary frame now so that nsDocumentViewer::FindContainerView
   // called from within EndSwapDocShellsForViews below can find it if needed.
-  aContent->SetPrimaryFrame(this);
+  nsIFrame::SetPrimaryFrame(this, aContent);
 
   // If we have a detached subdoc's root view on our frame loader, re-insert
   // it into the view tree. This happens when we've been reframed, and
   // ensures the presentation persists across reframes. If the frame element
   // has changed documents however, we blow away the presentation.
   RefPtr<nsFrameLoader> frameloader = FrameLoader();
   if (frameloader) {
     nsCOMPtr<nsIDocument> oldContainerDoc;