Bug 1393791 - Stop unbinding native-anonymous content off a script ru…

…nner. r=emilio

The failure mode in the attached crashtest is an inconsistency in the flattened
tree. Specifically, we null out mVideoControls in an nsVideoFrame, but defer
the UnbindFromTree call on that NAC element, which measn that its mParent still
points to the nsVideoFrame's mContent. Because all this stuff runs off of script
runners, and the anonymous content destroyer is not guaranteed to run before
other potential script runners, we end up running arbitrary script while the
tree mismatch exists. This script calls back into ProcessPendingRestyles, which
causes trouble.

We could build a separate deferral mechanism, but it's not clear that we actually
need to defer the unbind anymore. The deferred unbind was added in bug 489008,
which predated a lot of simplifications in layout/dom interaction.

MozReview-Commit-ID: 1JYAhiXKVJC

dom/base/nsContentUtils.cpp

@@ -5412,52 +5412,6 @@ nsContentUtils::IsInSameAnonymousTree(const nsINode* aNode,
   return nodeAsContent->GetBindingParent() == aContent->GetBindingParent();
 }
 
-class AnonymousContentDestroyer : public Runnable {
-public:
-  explicit AnonymousContentDestroyer(nsCOMPtr<nsIContent>* aContent)
-    : mozilla::Runnable("AnonymousContentDestroyer")
-  {
-    mContent.swap(*aContent);
-    mParent = mContent->GetParent();
-    mDoc = mContent->OwnerDoc();
-  }
-  explicit AnonymousContentDestroyer(nsCOMPtr<Element>* aElement)
-    : mozilla::Runnable("AnonymousContentDestroyer")
-  {
-    mContent = aElement->forget();
-    mParent = mContent->GetParent();
-    mDoc = mContent->OwnerDoc();
-  }
-  NS_IMETHOD Run() override {
-    mContent->UnbindFromTree();
-    return NS_OK;
-  }
-private:
-  nsCOMPtr<nsIContent> mContent;
-  // Hold strong refs to the parent content and document so that they
-  // don't die unexpectedly
-  nsCOMPtr<nsIDocument> mDoc;
-  nsCOMPtr<nsIContent> mParent;
-};
-
-/* static */
-void
-nsContentUtils::DestroyAnonymousContent(nsCOMPtr<nsIContent>* aContent)
-{
-  if (*aContent) {
-    AddScriptRunner(new AnonymousContentDestroyer(aContent));
-  }
-}
-
-/* static */
-void
-nsContentUtils::DestroyAnonymousContent(nsCOMPtr<Element>* aElement)
-{
-  if (*aElement) {
-    AddScriptRunner(new AnonymousContentDestroyer(aElement));
-  }
-}
-
 /* static */
 void
 nsContentUtils::NotifyInstalledMenuKeyboardListener(bool aInstalling)

dom/base/nsContentUtils.h

@@ -1672,12 +1672,6 @@ class nsContentUtils
    */
   static void DestroyMatchString(void* aData);
 
-  /**
-   * Unbinds the content from the tree and nulls it out if it's not null.
-   */
-  static void DestroyAnonymousContent(nsCOMPtr<nsIContent>* aContent);
-  static void DestroyAnonymousContent(nsCOMPtr<Element>* aElement);
-
   /*
    * Notify when the first XUL menu is opened and when the all XUL menus are
    * closed. At opening, aInstalling should be TRUE, otherwise, it should be

dom/html/nsTextEditorState.cpp

@@ -2264,9 +2264,9 @@ nsTextEditorState::UnbindFromFrame(nsTextControlFrame* aFrame)
   // Unbind the anonymous content from the tree.
   // We actually hold a reference to the content nodes so that
   // they're not actually destroyed.
-  nsContentUtils::DestroyAnonymousContent(&rootNode);
-  nsContentUtils::DestroyAnonymousContent(&mPlaceholderDiv);
-  nsContentUtils::DestroyAnonymousContent(&mPreviewDiv);
+  aFrame->DestroyAnonymousContent(rootNode.forget());
+  aFrame->DestroyAnonymousContent(mPlaceholderDiv.forget());
+  aFrame->DestroyAnonymousContent(mPreviewDiv.forget());
 }
 
 nsresult

layout/base/nsFrameManager.cpp

@@ -681,6 +681,21 @@ nsFrameManager::RestoreFrameState(nsIFrame* aFrame,
   }
 }
 
+void
+nsFrameManager::DestroyAnonymousContent(already_AddRefed<nsIContent> aContent)
+{
+  nsCOMPtr<nsIContent> content = aContent;
+  if (content) {
+    // Invoke ClearAllMapsFor before unbinding from the tree. When we unbind,
+    // we remove the mPrimaryFrame pointer, which is used by the frame
+    // teardown code to determine whether to invoke ClearAllMapsFor or not.
+    // These maps will go away when we drop support for the old style system.
+    ClearAllMapsFor(content);
+
+    content->UnbindFromTree();
+  }
+}
+
 //----------------------------------------------------------------------
 
 nsFrameManagerBase::UndisplayedMap::UndisplayedMap()

layout/base/nsFrameManager.h

@@ -199,6 +199,8 @@ class nsFrameManager : public nsFrameManagerBase
 
   void RestoreFrameStateFor(nsIFrame* aFrame, nsILayoutHistoryState* aState);
 
+  void DestroyAnonymousContent(already_AddRefed<nsIContent> aContent);
+
 protected:
   static nsIContent* ParentForUndisplayedMap(const nsIContent* aContent);
 

layout/forms/nsColorControlFrame.cpp

@@ -44,7 +44,7 @@ NS_QUERYFRAME_TAIL_INHERITING(nsHTMLButtonControlFrame)
 void nsColorControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
-  nsContentUtils::DestroyAnonymousContent(&mColorContent);
+  DestroyAnonymousContent(mColorContent.forget());
   nsHTMLButtonControlFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsComboboxControlFrame.cpp

@@ -1418,8 +1418,8 @@ nsComboboxControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 
   // Cleanup frames in popup child list
   mPopupFrames.DestroyFramesFrom(aDestructRoot);
-  nsContentUtils::DestroyAnonymousContent(&mDisplayContent);
-  nsContentUtils::DestroyAnonymousContent(&mButtonContent);
+  DestroyAnonymousContent(mDisplayContent.forget());
+  DestroyAnonymousContent(mButtonContent.forget());
   nsBlockFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsDateTimeControlFrame.cpp

@@ -50,7 +50,7 @@ nsDateTimeControlFrame::nsDateTimeControlFrame(nsStyleContext* aContext)
 void
 nsDateTimeControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mInputAreaContent);
+  DestroyAnonymousContent(mInputAreaContent.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsFileControlFrame.cpp

@@ -68,8 +68,8 @@ nsFileControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
                                         mMouseListener, false);
   }
 
-  nsContentUtils::DestroyAnonymousContent(&mTextContent);
-  nsContentUtils::DestroyAnonymousContent(&mBrowseFilesOrDirs);
+  DestroyAnonymousContent(mTextContent.forget());
+  DestroyAnonymousContent(mBrowseFilesOrDirs.forget());
 
   mMouseListener->ForgetFrame();
   nsBlockFrame::DestroyFrom(aDestructRoot);

layout/forms/nsGfxButtonControlFrame.cpp

@@ -33,7 +33,7 @@ NS_IMPL_FRAMEARENA_HELPERS(nsGfxButtonControlFrame)
 
 void nsGfxButtonControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mTextContent);
+  DestroyAnonymousContent(mTextContent.forget());
   nsHTMLButtonControlFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsMeterFrame.cpp

@@ -55,7 +55,7 @@ nsMeterFrame::DestroyFrom(nsIFrame* aDestructRoot)
                "nsMeterFrame should not have continuations; if it does we "
                "need to call RegUnregAccessKey only for the first.");
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
-  nsContentUtils::DestroyAnonymousContent(&mBarDiv);
+  DestroyAnonymousContent(mBarDiv.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsNumberControlFrame.cpp

@@ -62,7 +62,7 @@ nsNumberControlFrame::DestroyFrom(nsIFrame* aDestructRoot)
                "nsNumberControlFrame should not have continuations; if it does we "
                "need to call RegUnregAccessKey only for the first");
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
-  nsContentUtils::DestroyAnonymousContent(&mOuterWrapper);
+  DestroyAnonymousContent(mOuterWrapper.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsProgressFrame.cpp

@@ -54,7 +54,7 @@ nsProgressFrame::DestroyFrom(nsIFrame* aDestructRoot)
                "nsProgressFrame should not have continuations; if it does we "
                "need to call RegUnregAccessKey only for the first.");
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
-  nsContentUtils::DestroyAnonymousContent(&mBarDiv);
+  DestroyAnonymousContent(mBarDiv.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/forms/nsRangeFrame.cpp

@@ -104,9 +104,9 @@ nsRangeFrame::DestroyFrom(nsIFrame* aDestructRoot)
   mContent->RemoveEventListener(NS_LITERAL_STRING("touchstart"), mDummyTouchListener, false);
 
   nsFormControlFrame::RegUnRegAccessKey(static_cast<nsIFrame*>(this), false);
-  nsContentUtils::DestroyAnonymousContent(&mTrackDiv);
-  nsContentUtils::DestroyAnonymousContent(&mProgressDiv);
-  nsContentUtils::DestroyAnonymousContent(&mThumbDiv);
+  DestroyAnonymousContent(mTrackDiv.forget());
+  DestroyAnonymousContent(mProgressDiv.forget());
+  DestroyAnonymousContent(mThumbDiv.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/generic/DetailsFrame.cpp

@@ -83,7 +83,7 @@ DetailsFrame::CheckValidMainSummary(const nsFrameList& aFrameList) const
 void
 DetailsFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mDefaultSummary);
+  DestroyAnonymousContent(mDefaultSummary.forget());
   nsBlockFrame::DestroyFrom(aDestructRoot);
 }
 

layout/generic/nsCanvasFrame.cpp

@@ -153,7 +153,7 @@ nsCanvasFrame::DestroyFrom(nsIFrame* aDestructRoot)
       content->SetContentNode(clonedElement->AsElement());
     }
   }
-  nsContentUtils::DestroyAnonymousContent(&mCustomContentContainer);
+  DestroyAnonymousContent(mCustomContentContainer.forget());
 
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }

layout/generic/nsFrame.cpp

@@ -240,6 +240,13 @@ nsReflowStatus::UpdateTruncated(const ReflowInput& aReflowInput,
   }
 }
 
+void
+nsIFrame::DestroyAnonymousContent(already_AddRefed<nsIContent> aContent)
+{
+  PresContext()->PresShell()->FrameConstructor()
+               ->DestroyAnonymousContent(mozilla::Move(aContent));
+}
+
 // Formerly the nsIFrameDebug interface
 
 #ifdef DEBUG

layout/generic/nsGfxScrollFrame.cpp

@@ -4607,10 +4607,10 @@ ScrollFrameHelper::Destroy()
   }
 
   // Unbind any content created in CreateAnonymousContent from the tree
-  nsContentUtils::DestroyAnonymousContent(&mHScrollbarContent);
-  nsContentUtils::DestroyAnonymousContent(&mVScrollbarContent);
-  nsContentUtils::DestroyAnonymousContent(&mScrollCornerContent);
-  nsContentUtils::DestroyAnonymousContent(&mResizerContent);
+  mOuter->DestroyAnonymousContent(mHScrollbarContent.forget());
+  mOuter->DestroyAnonymousContent(mVScrollbarContent.forget());
+  mOuter->DestroyAnonymousContent(mScrollCornerContent.forget());
+  mOuter->DestroyAnonymousContent(mResizerContent.forget());
 
   if (mPostedReflowCallback) {
     mOuter->PresContext()->PresShell()->CancelReflowCallback(this);

layout/generic/nsIFrame.h

@@ -4026,6 +4026,8 @@ class nsIFrame : public nsQueryFrame
 
   DisplayItemArray& DisplayItemData() { return mDisplayItemData; }
 
+  void DestroyAnonymousContent(already_AddRefed<nsIContent> aContent);
+
 protected:
 
   /**

layout/generic/nsVideoFrame.cpp

@@ -177,9 +177,9 @@ nsVideoFrame::AppendAnonymousContentTo(nsTArray<nsIContent*>& aElements,
 void
 nsVideoFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mCaptionDiv);
-  nsContentUtils::DestroyAnonymousContent(&mVideoControls);
-  nsContentUtils::DestroyAnonymousContent(&mPosterImage);
+  DestroyAnonymousContent(mCaptionDiv.forget());
+  DestroyAnonymousContent(mVideoControls.forget());
+  DestroyAnonymousContent(mPosterImage.forget());
   nsContainerFrame::DestroyFrom(aDestructRoot);
 }
 

layout/svg/nsSVGUseFrame.cpp

@@ -102,7 +102,7 @@ nsSVGUseFrame::AttributeChanged(int32_t         aNameSpaceID,
 void
 nsSVGUseFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mContentClone);
+  DestroyAnonymousContent(mContentClone.forget());
   nsSVGGFrame::DestroyFrom(aDestructRoot);
 }
 

layout/xul/nsDocElementBoxFrame.cpp

@@ -75,8 +75,8 @@ NS_IMPL_FRAMEARENA_HELPERS(nsDocElementBoxFrame)
 void
 nsDocElementBoxFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
-  nsContentUtils::DestroyAnonymousContent(&mPopupgroupContent);
-  nsContentUtils::DestroyAnonymousContent(&mTooltipContent);
+  DestroyAnonymousContent(mPopupgroupContent.forget());
+  DestroyAnonymousContent(mTooltipContent.forget());
   nsBoxFrame::DestroyFrom(aDestructRoot);
 }