Bug 1472020 - Make AccessibleCaret a bit saner. r=bz,TYLin
author"Emilio Cobos Álvarez" <emilio@crisal.io>
Mon, 13 Aug 2018 11:56:48 +0200
changeset 486304 5c9a4d66f5f3f8ff76b64824b8fbb66c310a7443
parent 486303 b5e9fdddf12e598962d6282a4b053aa66b7aabda
child 486305 6f714c1fb6e128977b9d4f86198c2dd1c43e422e
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, TYLin
bugs1472020
milestone63.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 1472020 - Make AccessibleCaret a bit saner. r=bz,TYLin Avoid processing anon content in nsCanvasFrame, then getting more anon content via AccessibleCaretEventHub::Init. Instead call Init before creating the custom content container. We could also throw a script runner at it I guess, but this prevents the reentrancy issue. Avoid cloning nodes during layout, just use the same node (already cloned in InsertAnonymousContent) instead. The RemoveChild in GetAnonymousContent to handle the reframes instead of cloning around is a bit hacky, but I don't think it's really worth extending PostDestroyData for this special case. Differential Revision: https://phabricator.services.mozilla.com/D1889
dom/base/AnonymousContent.cpp
dom/base/AnonymousContent.h
dom/base/nsDocument.cpp
dom/base/test/test_anonymousContent_xul_window.xul
layout/base/AccessibleCaret.cpp
layout/base/AccessibleCaret.h
layout/base/AccessibleCaretEventHub.cpp
layout/base/crashtests/1472020.html
layout/base/crashtests/crashtests.list
layout/base/nsLayoutUtils.cpp
layout/generic/nsCanvasFrame.cpp
--- a/dom/base/AnonymousContent.cpp
+++ b/dom/base/AnonymousContent.cpp
@@ -17,35 +17,23 @@
 namespace mozilla {
 namespace dom {
 
 // Ref counting and cycle collection
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(AnonymousContent, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(AnonymousContent, Release)
 NS_IMPL_CYCLE_COLLECTION(AnonymousContent, mContentNode)
 
-AnonymousContent::AnonymousContent(Element* aContentNode) :
-  mContentNode(aContentNode)
-{}
-
-AnonymousContent::~AnonymousContent()
+AnonymousContent::AnonymousContent(already_AddRefed<Element> aContentNode)
+  : mContentNode(aContentNode)
 {
+  MOZ_ASSERT(mContentNode);
 }
 
-Element*
-AnonymousContent::GetContentNode()
-{
-  return mContentNode;
-}
-
-void
-AnonymousContent::SetContentNode(Element* aContentNode)
-{
-  mContentNode = aContentNode;
-}
+AnonymousContent::~AnonymousContent() = default;
 
 void
 AnonymousContent::SetTextContentForElement(const nsAString& aElementId,
                                            const nsAString& aText,
                                            ErrorResult& aRv)
 {
   Element* element = GetElementById(aElementId);
   if (!element) {
--- a/dom/base/AnonymousContent.h
+++ b/dom/base/AnonymousContent.h
@@ -20,21 +20,26 @@ class UnrestrictedDoubleOrAnonymousKeyfr
 
 class AnonymousContent final
 {
 public:
   // Ref counting and cycle collection
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(AnonymousContent)
   NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(AnonymousContent)
 
-  explicit AnonymousContent(Element* aContentNode);
-  Element* GetContentNode();
+  explicit AnonymousContent(already_AddRefed<Element> aContentNode);
+  Element& ContentNode()
+  {
+    return *mContentNode;
+  }
+
   Element* GetElementById(const nsAString& aElementId);
-  void SetContentNode(Element* aContentNode);
-  bool WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto, JS::MutableHandle<JSObject*> aReflector);
+  bool WrapObject(JSContext* aCx,
+                  JS::Handle<JSObject*> aGivenProto,
+                  JS::MutableHandle<JSObject*> aReflector);
 
   // WebIDL methods
   void SetTextContentForElement(const nsAString& aElementId,
                                 const nsAString& aText,
                                 ErrorResult& aRv);
 
   void GetTextContentForElement(const nsAString& aElementId,
                                 DOMString& aText,
@@ -71,15 +76,15 @@ public:
 
   void GetComputedStylePropertyValue(const nsAString& aElementId,
                                      const nsAString& aPropertyName,
                                      DOMString& aResult,
                                      ErrorResult& aRv);
 
 private:
   ~AnonymousContent();
-  nsCOMPtr<Element> mContentNode;
+  RefPtr<Element> mContentNode;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_AnonymousContent_h
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -5313,93 +5313,90 @@ nsIDocument::StyleRuleRemoved(StyleSheet
   DO_STYLESHEET_NOTIFICATION(StyleRuleChangeEvent,
                              "StyleRuleRemoved",
                              mRule,
                              aStyleRule);
 }
 
 #undef DO_STYLESHEET_NOTIFICATION
 
+static Element*
+GetCustomContentContainer(nsIPresShell* aShell)
+{
+  if (!aShell || !aShell->GetCanvasFrame()) {
+    return nullptr;
+  }
+
+  return aShell->GetCanvasFrame()->GetCustomContentContainer();
+}
+
+static void
+InsertAnonContentIntoCanvas(AnonymousContent& aAnonContent,
+                            nsIPresShell* aShell)
+{
+  Element* container = GetCustomContentContainer(aShell);
+  if (!container) {
+    return;
+  }
+
+  nsresult rv = container->AppendChildTo(&aAnonContent.ContentNode(), true);
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  aShell->GetCanvasFrame()->ShowCustomContentContainer();
+}
+
 already_AddRefed<AnonymousContent>
 nsIDocument::InsertAnonymousContent(Element& aElement, ErrorResult& aRv)
 {
-  nsIPresShell* shell = GetShell();
-  if (!shell || !shell->GetCanvasFrame()) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return nullptr;
-  }
-
   nsAutoScriptBlocker scriptBlocker;
-  nsCOMPtr<Element> container = shell->GetCanvasFrame()
-                                     ->GetCustomContentContainer();
-  if (!container) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return nullptr;
-  }
-
-  // Clone the node to avoid returning a direct reference
-  nsCOMPtr<nsINode> clonedElement = aElement.CloneNode(true, aRv);
+
+  // Clone the node to avoid returning a direct reference.
+  nsCOMPtr<nsINode> clone = aElement.CloneNode(true, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
-  // Insert the element into the container
-  nsresult rv;
-  rv = container->AppendChildTo(clonedElement->AsContent(), true);
-  if (NS_FAILED(rv)) {
-    return nullptr;
-  }
-
-  RefPtr<AnonymousContent> anonymousContent =
-    new AnonymousContent(clonedElement->AsElement());
-  mAnonymousContents.AppendElement(anonymousContent);
-
-  shell->GetCanvasFrame()->ShowCustomContentContainer();
-
-  return anonymousContent.forget();
+  auto anonContent =
+    MakeRefPtr<AnonymousContent>(clone.forget().downcast<Element>());
+  mAnonymousContents.AppendElement(anonContent);
+
+  InsertAnonContentIntoCanvas(*anonContent, GetShell());
+
+  return anonContent.forget();
+}
+
+static void
+RemoveAnonContentFromCanvas(AnonymousContent& aAnonContent,
+                            nsIPresShell* aShell)
+{
+  RefPtr<Element> container = GetCustomContentContainer(aShell);
+  if (!container) {
+    return;
+  }
+  container->RemoveChild(aAnonContent.ContentNode(), IgnoreErrors());
 }
 
 void
 nsIDocument::RemoveAnonymousContent(AnonymousContent& aContent,
                                     ErrorResult& aRv)
 {
-  nsIPresShell* shell = GetShell();
-  if (!shell || !shell->GetCanvasFrame()) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return;
-  }
-
   nsAutoScriptBlocker scriptBlocker;
-  nsCOMPtr<Element> container = shell->GetCanvasFrame()
-                                     ->GetCustomContentContainer();
-  if (!container) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return;
-  }
-
-  // Iterate over mAnonymousContents to find and remove the given node.
-  for (size_t i = 0, len = mAnonymousContents.Length(); i < len; ++i) {
-    if (mAnonymousContents[i] == &aContent) {
-      // Get the node from the customContent
-      nsCOMPtr<Element> node = aContent.GetContentNode();
-
-      // Remove the entry in mAnonymousContents
-      mAnonymousContents.RemoveElementAt(i);
-
-      // Remove the node from its container
-      container->RemoveChild(*node, aRv);
-      if (aRv.Failed()) {
-        return;
-      }
-
-      break;
-    }
-  }
-  if (mAnonymousContents.IsEmpty()) {
-    shell->GetCanvasFrame()->HideCustomContentContainer();
+
+  auto index = mAnonymousContents.IndexOf(&aContent);
+  if (index == mAnonymousContents.NoIndex) {
+    return;
+  }
+
+  mAnonymousContents.RemoveElementAt(index);
+  RemoveAnonContentFromCanvas(aContent, GetShell());
+
+  if (mAnonymousContents.IsEmpty() && GetCustomContentContainer(GetShell())) {
+    GetShell()->GetCanvasFrame()->HideCustomContentContainer();
   }
 }
 
 Element*
 nsIDocument::GetAnonRootIfInAnonymousContentContainer(nsINode* aNode) const
 {
   if (!aNode->IsInNativeAnonymousSubtree()) {
     return nullptr;
--- a/dom/base/test/test_anonymousContent_xul_window.xul
+++ b/dom/base/test/test_anonymousContent_xul_window.xul
@@ -1,30 +1,23 @@
 <?xml version="1.0"?>
 <?xml-stylesheet type="text/css" href="chrome://global/skin"?>
 <?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=1020244
-Check that XUL windows aren't supported, that the API throws, but doesn't crash.
+Check that XUL windows don't make insertAnonymousContent crash.
 -->
 <window title="Anonymous content in a XUL window"
 xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
 
   <!-- test results are displayed in the html:body -->
   <body xmlns="http://www.w3.org/1999/xhtml"></body>
   <box>This is a test box</box>
 
   <script type="application/javascript">
     // Insert content
     let testElement = document.querySelector("box");
-
-    let didThrow = false;
-    try {
-      document.insertAnonymousContent(testElement);
-    } catch (e) {
-      didThrow = true;
-    }
-
-    ok(didThrow,
-      "Inserting anonymous content in a XUL document did throw an exception")
+    document.insertAnonymousContent(testElement);
+    ok(true,
+      "Inserting anonymous content in a XUL document did not crash")
   </script>
 </window>
--- a/layout/base/AccessibleCaret.cpp
+++ b/layout/base/AccessibleCaret.cpp
@@ -73,19 +73,16 @@ operator<<(std::ostream& aStream,
 
 AccessibleCaret::AccessibleCaret(nsIPresShell* aPresShell)
   : mPresShell(aPresShell)
 {
   // Check all resources required.
   if (mPresShell) {
     MOZ_ASSERT(RootFrame());
     MOZ_ASSERT(mPresShell->GetDocument());
-    MOZ_ASSERT(mPresShell->GetCanvasFrame());
-    MOZ_ASSERT(mPresShell->GetCanvasFrame()->GetCustomContentContainer());
-
     InjectCaretElement(mPresShell->GetDocument());
   }
 
   static bool prefsAdded = false;
   if (!prefsAdded) {
     Preferences::AddFloatVarCache(&sWidth, "layout.accessiblecaret.width");
     Preferences::AddFloatVarCache(&sHeight, "layout.accessiblecaret.height");
     Preferences::AddFloatVarCache(&sMarginLeft, "layout.accessiblecaret.margin-left");
@@ -104,20 +101,20 @@ AccessibleCaret::~AccessibleCaret()
 void
 AccessibleCaret::SetAppearance(Appearance aAppearance)
 {
   if (mAppearance == aAppearance) {
     return;
   }
 
   ErrorResult rv;
-  CaretElement()->ClassList()->Remove(AppearanceString(mAppearance), rv);
+  CaretElement().ClassList()->Remove(AppearanceString(mAppearance), rv);
   MOZ_ASSERT(!rv.Failed(), "Remove old appearance failed!");
 
-  CaretElement()->ClassList()->Add(AppearanceString(aAppearance), rv);
+  CaretElement().ClassList()->Add(AppearanceString(aAppearance), rv);
   MOZ_ASSERT(!rv.Failed(), "Add new appearance failed!");
 
   AC_LOG("%s: %s -> %s", __FUNCTION__, ToString(mAppearance).c_str(),
          ToString(aAppearance).c_str());
 
   mAppearance = aAppearance;
 
   // Need to reset rect since the cached rect will be compared in SetPosition.
@@ -132,18 +129,18 @@ AccessibleCaret::SetSelectionBarEnabled(
 {
   if (mSelectionBarEnabled == aEnabled) {
     return;
   }
 
   AC_LOG("Set selection bar %s", aEnabled ? "Enabled" : "Disabled");
 
   ErrorResult rv;
-  CaretElement()->ClassList()->Toggle(NS_LITERAL_STRING("no-bar"),
-                                      Optional<bool>(!aEnabled), rv);
+  CaretElement().ClassList()->Toggle(NS_LITERAL_STRING("no-bar"),
+                                     Optional<bool>(!aEnabled), rv);
   MOZ_ASSERT(!rv.Failed());
 
   mSelectionBarEnabled = aEnabled;
 }
 
 /* static */ nsAutoString
 AccessibleCaret::AppearanceString(Appearance aAppearance)
 {
@@ -170,18 +167,18 @@ bool
 AccessibleCaret::Intersects(const AccessibleCaret& aCaret) const
 {
   MOZ_ASSERT(mPresShell == aCaret.mPresShell);
 
   if (!IsVisuallyVisible() || !aCaret.IsVisuallyVisible()) {
     return false;
   }
 
-  nsRect rect = nsLayoutUtils::GetRectRelativeToFrame(CaretElement(), RootFrame());
-  nsRect rhsRect = nsLayoutUtils::GetRectRelativeToFrame(aCaret.CaretElement(), RootFrame());
+  nsRect rect = nsLayoutUtils::GetRectRelativeToFrame(&CaretElement(), RootFrame());
+  nsRect rhsRect = nsLayoutUtils::GetRectRelativeToFrame(&aCaret.CaretElement(), RootFrame());
   return rect.Intersects(rhsRect);
 }
 
 bool
 AccessibleCaret::Contains(const nsPoint& aPoint, TouchArea aTouchArea) const
 {
   if (!IsVisuallyVisible()) {
     return false;
@@ -201,31 +198,32 @@ AccessibleCaret::Contains(const nsPoint&
 }
 
 void
 AccessibleCaret::EnsureApzAware()
 {
   // If the caret element was cloned, the listener might have been lost. So
   // if that's the case we register a dummy listener if there isn't one on
   // the element already.
-  if (!CaretElement()->IsApzAware()) {
-    CaretElement()->AddEventListener(NS_LITERAL_STRING("touchstart"),
-                                     mDummyTouchListener, false);
+  if (!CaretElement().IsApzAware()) {
+    // FIXME(emilio): Is this needed anymore?
+    CaretElement().AddEventListener(NS_LITERAL_STRING("touchstart"),
+                                    mDummyTouchListener, false);
   }
 }
 
 void
 AccessibleCaret::InjectCaretElement(nsIDocument* aDocument)
 {
   ErrorResult rv;
-  nsCOMPtr<Element> element = CreateCaretElement(aDocument);
+  RefPtr<Element> element = CreateCaretElement(aDocument);
   mCaretElementHolder = aDocument->InsertAnonymousContent(*element, rv);
 
   MOZ_ASSERT(!rv.Failed(), "Insert anonymous content should not fail!");
-  MOZ_ASSERT(mCaretElementHolder.get(), "We must have anonymous content!");
+  MOZ_ASSERT(mCaretElementHolder, "We must have anonymous content!");
 
   // InsertAnonymousContent will clone the element to make an AnonymousContent.
   // Since event listeners are not being cloned when cloning a node, we need to
   // add the listener here.
   EnsureApzAware();
 }
 
 already_AddRefed<Element>
@@ -233,54 +231,56 @@ AccessibleCaret::CreateCaretElement(nsID
 {
   // Content structure of AccessibleCaret
   // <div class="moz-accessiblecaret">  <- CaretElement()
   //   <div id="text-overlay"           <- TextOverlayElement()
   //   <div id="image">                 <- CaretImageElement()
   //   <div id="bar">                   <- SelectionBarElement()
 
   ErrorResult rv;
-  nsCOMPtr<Element> parent = aDocument->CreateHTMLElement(nsGkAtoms::div);
+  RefPtr<Element> parent = aDocument->CreateHTMLElement(nsGkAtoms::div);
   parent->ClassList()->Add(NS_LITERAL_STRING("moz-accessiblecaret"), rv);
   parent->ClassList()->Add(NS_LITERAL_STRING("none"), rv);
   parent->ClassList()->Add(NS_LITERAL_STRING("no-bar"), rv);
 
   auto CreateAndAppendChildElement = [aDocument, &parent](
     const nsLiteralString& aElementId)
   {
-    nsCOMPtr<Element> child = aDocument->CreateHTMLElement(nsGkAtoms::div);
+    RefPtr<Element> child = aDocument->CreateHTMLElement(nsGkAtoms::div);
     child->SetAttr(kNameSpaceID_None, nsGkAtoms::id, aElementId, true);
     parent->AppendChildTo(child, false);
   };
 
   CreateAndAppendChildElement(sTextOverlayElementId);
   CreateAndAppendChildElement(sCaretImageElementId);
   CreateAndAppendChildElement(sSelectionBarElementId);
 
   return parent.forget();
 }
 
 void
 AccessibleCaret::RemoveCaretElement(nsIDocument* aDocument)
 {
-  CaretElement()->RemoveEventListener(NS_LITERAL_STRING("touchstart"),
-                                      mDummyTouchListener, false);
+  CaretElement().RemoveEventListener(NS_LITERAL_STRING("touchstart"),
+                                     mDummyTouchListener, false);
 
-  if (nsIFrame* frame = CaretElement()->GetPrimaryFrame()) {
+  // FIXME(emilio): This shouldn't be needed and should be done by
+  // ContentRemoved via RemoveAnonymousContent, but the current setup tears down
+  // the accessible caret manager after the shell has stopped observing the
+  // document, but before the frame tree has gone away. This could clearly be
+  // better...
+  if (nsIFrame* frame = CaretElement().GetPrimaryFrame()) {
     if (frame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)) {
       frame = frame->GetPlaceholderFrame();
     }
     nsAutoScriptBlocker scriptBlocker;
     frame->GetParent()->RemoveFrame(nsIFrame::kPrincipalList, frame);
   }
 
-  ErrorResult rv;
-  aDocument->RemoveAnonymousContent(*mCaretElementHolder, rv);
-  // It's OK rv is failed since nsCanvasFrame might not exists now.
-  rv.SuppressException();
+  aDocument->RemoveAnonymousContent(*mCaretElementHolder, IgnoreErrors());
 }
 
 AccessibleCaret::PositionChangedResult
 AccessibleCaret::SetPosition(nsIFrame* aFrame, int32_t aOffset)
 {
   if (!CustomContentContainerFrame()) {
     return PositionChangedResult::NotChanged;
   }
@@ -341,17 +341,17 @@ AccessibleCaret::SetCaretElementStyle(co
   // formatting of floating-point values.
   styleStr.AppendFloat(sWidth/aZoomLevel);
   styleStr.AppendLiteral("px; height: ");
   styleStr.AppendFloat(sHeight/aZoomLevel);
   styleStr.AppendLiteral("px; margin-left: ");
   styleStr.AppendFloat(sMarginLeft/aZoomLevel);
   styleStr.AppendLiteral("px");
 
-  CaretElement()->SetAttr(kNameSpaceID_None, nsGkAtoms::style, styleStr, true);
+  CaretElement().SetAttr(kNameSpaceID_None, nsGkAtoms::style, styleStr, true);
   AC_LOG("%s: %s", __FUNCTION__, NS_ConvertUTF16toUTF8(styleStr).get());
 
   // Set style string for children.
   SetTextOverlayElementStyle(aRect, aZoomLevel);
   SetCaretImageElementStyle(aRect, aZoomLevel);
   SetSelectionBarElementStyle(aRect, aZoomLevel);
 }
 
--- a/layout/base/AccessibleCaret.h
+++ b/layout/base/AccessibleCaret.h
@@ -136,19 +136,19 @@ public:
   // The geometry center of the imaginary caret (nsCaret) to which this
   // AccessibleCaret is attached. It is needed when dragging the caret.
   nsPoint LogicalPosition() const
   {
     return mImaginaryCaretRect.Center();
   }
 
   // Element for 'Intersects' test. Container of image and bar elements.
-  dom::Element* CaretElement() const
+  dom::Element& CaretElement() const
   {
-    return mCaretElementHolder->GetContentNode();
+    return mCaretElementHolder->ContentNode();
   }
 
   // Ensures that the caret element is made "APZ aware" so that the APZ code
   // doesn't scroll the page when the user is trying to drag the caret.
   void EnsureApzAware();
 
 protected:
   // Argument aRect should be relative to CustomContentContainerFrame().
--- a/layout/base/AccessibleCaretEventHub.cpp
+++ b/layout/base/AccessibleCaretEventHub.cpp
@@ -378,18 +378,17 @@ AccessibleCaretEventHub::AccessibleCaret
 
 void
 AccessibleCaretEventHub::Init()
 {
   if (mInitialized && mManager) {
     mManager->OnFrameReconstruction();
   }
 
-  if (mInitialized || !mPresShell || !mPresShell->GetCanvasFrame() ||
-      !mPresShell->GetCanvasFrame()->GetCustomContentContainer()) {
+  if (mInitialized || !mPresShell || !mPresShell->GetCanvasFrame()) {
     return;
   }
 
   // Without nsAutoScriptBlocker, the script might be run after constructing
   // mFirstCaret in AccessibleCaretManager's constructor, which might destructs
   // the whole frame tree. Therefore we'll fail to construct mSecondCaret
   // because we cannot get root frame or canvas frame from mPresShell to inject
   // anonymous content. To avoid that, we protect Init() by nsAutoScriptBlocker.
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1472020.html
@@ -0,0 +1,11 @@
+<style>
+body { display:contents }
+</style>
+<object id="a"></object>
+<script>
+  document.body.offsetHeight;
+  document.getElementById("a").style.cssText="display:table-column-group"
+  document.body.offsetHeight;
+  document.body.style.cssText="scroll-snap-destination:6%"
+  document.body.offsetHeight;
+</script>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -536,8 +536,9 @@ load 1462412.html
 load 1463940.html
 pref(dom.webcomponents.shadowdom.enabled,true) HTTP load 1464641.html
 load 1464737.html
 load 1466638.html
 load 1467688.html
 load 1467964.html
 load 1469354.html
 load 1472027.html
+pref(layout.accessiblecaret.enabled,true) load 1472020.html
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -963,23 +963,22 @@ GetDisplayPortFromMarginsData(nsIContent
 
   return result;
 }
 
 static bool
 HasVisibleAnonymousContents(nsIDocument* aDoc)
 {
   for (RefPtr<AnonymousContent>& ac : aDoc->GetAnonymousContents()) {
-    Element* elem = ac->GetContentNode();
     // We check to see if the anonymous content node has a frame. If it doesn't,
     // that means that's not visible to the user because e.g. it's display:none.
     // For now we assume that if it has a frame, it is visible. We might be able
     // to refine this further by adding complexity if it turns out this condition
     // results in a lot of false positives.
-    if (elem && elem->GetPrimaryFrame()) {
+    if (ac->ContentNode().GetPrimaryFrame()) {
       return true;
     }
   }
   return false;
 }
 
 bool
 nsLayoutUtils::ShouldDisableApzForElement(nsIContent* aContent)
--- a/layout/generic/nsCanvasFrame.cpp
+++ b/layout/generic/nsCanvasFrame.cpp
@@ -73,80 +73,105 @@ nsCanvasFrame::HideCustomContentContaine
                                      NS_LITERAL_STRING("true"),
                                      true);
   }
 }
 
 nsresult
 nsCanvasFrame::CreateAnonymousContent(nsTArray<ContentInfo>& aElements)
 {
+  MOZ_ASSERT(!mCustomContentContainer);
+
   if (!mContent) {
     return NS_OK;
   }
 
   nsCOMPtr<nsIDocument> doc = mContent->OwnerDoc();
-  nsresult rv = NS_OK;
+
+  RefPtr<AccessibleCaretEventHub> eventHub =
+    PresShell()->GetAccessibleCaretEventHub();
+
+  // This will go through InsertAnonymousContent and such, and we don't really
+  // want it to end up inserting into our content container.
+  //
+  // FIXME(emilio): The fact that this enters into InsertAnonymousContent is a
+  // bit nasty, can we avoid it, maybe doing this off a scriptrunner?
+  if (eventHub) {
+    eventHub->Init();
+  }
 
   // Create the custom content container.
   mCustomContentContainer = doc->CreateHTMLElement(nsGkAtoms::div);
 #ifdef DEBUG
   // We restyle our mCustomContentContainer, even though it's root anonymous
   // content.  Normally that's not OK because the frame constructor doesn't know
   // how to order the frame tree in such cases, but we make this work for this
   // particular case, so it's OK.
   mCustomContentContainer->SetProperty(nsGkAtoms::restylableAnonymousNode,
                                        reinterpret_cast<void*>(true));
 #endif // DEBUG
 
   mCustomContentContainer->SetProperty(nsGkAtoms::docLevelNativeAnonymousContent,
                                        reinterpret_cast<void*>(true));
 
+  // This will usually be done by the caller, but in this case we do it here,
+  // since we reuse the document's AnoymousContent list, and those survive
+  // across reframes and thus may already be flagged as being in an anonymous
+  // subtree. We don't really want to have this semi-broken state where
+  // anonymous nodes have a non-anonymous.
+  mCustomContentContainer->SetIsNativeAnonymousRoot();
+
   aElements.AppendElement(mCustomContentContainer);
 
   // Do not create an accessible object for the container.
   mCustomContentContainer->SetAttr(kNameSpaceID_None, nsGkAtoms::role,
                                    NS_LITERAL_STRING("presentation"), false);
 
-  // XXX add :moz-native-anonymous or will that be automatically set?
-  rv = mCustomContentContainer->SetAttr(kNameSpaceID_None, nsGkAtoms::_class,
-                                        NS_LITERAL_STRING("moz-custom-content-container"),
-                                        true);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // Append all existing AnonymousContent nodes stored at document level if any.
-  size_t len = doc->GetAnonymousContents().Length();
-  for (size_t i = 0; i < len; ++i) {
-    nsCOMPtr<Element> node = doc->GetAnonymousContents()[i]->GetContentNode();
-    mCustomContentContainer->AppendChildTo(node->AsContent(), true);
-  }
+  mCustomContentContainer->SetAttr(kNameSpaceID_None, nsGkAtoms::_class,
+                                   NS_LITERAL_STRING("moz-custom-content-container"),
+                                   false);
 
   // Only create a frame for mCustomContentContainer if it has some children.
-  if (len == 0) {
+  if (doc->GetAnonymousContents().IsEmpty()) {
     HideCustomContentContainer();
   }
 
-  RefPtr<AccessibleCaretEventHub> eventHub =
-    PresContext()->GetPresShell()->GetAccessibleCaretEventHub();
-  if (eventHub) {
-    // AccessibleCaret will insert anonymous caret elements.
-    eventHub->Init();
+  for (RefPtr<AnonymousContent>& anonContent : doc->GetAnonymousContents()) {
+    if (nsCOMPtr<nsINode> parent = anonContent->ContentNode().GetParentNode()) {
+      // Parent had better be an old custom content container already removed
+      // from a reframe. Forget about it since we're about to get inserted in a
+      // new one.
+      //
+      // TODO(emilio): Maybe we should extend PostDestroyData and do this stuff
+      // there instead, or something...
+      MOZ_ASSERT(parent != mCustomContentContainer);
+      MOZ_ASSERT(parent->IsElement());
+      MOZ_ASSERT(parent->AsElement()->IsRootOfNativeAnonymousSubtree());
+      MOZ_ASSERT(!parent->IsInComposedDoc());
+      MOZ_ASSERT(!parent->GetParentNode());
+
+      parent->RemoveChildNode(&anonContent->ContentNode(), false);
+    }
+
+    mCustomContentContainer->AppendChildTo(&anonContent->ContentNode(), false);
   }
 
   // Create a popupgroup element for chrome privileged top level non-XUL
   // documents to support context menus and tooltips.
   if (PresContext()->IsChrome() && PresContext()->IsRoot() &&
       doc->AllowXULXBL() && !doc->IsXULDocument()) {
     nsNodeInfoManager* nodeInfoManager = doc->NodeInfoManager();
     RefPtr<NodeInfo> nodeInfo =
       nodeInfoManager->GetNodeInfo(nsGkAtoms::popupgroup,
                                    nullptr, kNameSpaceID_XUL,
                                    nsINode::ELEMENT_NODE);
 
-    rv = NS_NewXULElement(getter_AddRefs(mPopupgroupContent),
-                          nodeInfo.forget(), dom::NOT_FROM_PARSER);
+    nsresult rv = NS_NewXULElement(getter_AddRefs(mPopupgroupContent),
+                                   nodeInfo.forget(),
+                                   dom::NOT_FROM_PARSER);
     NS_ENSURE_SUCCESS(rv, rv);
 
     aElements.AppendElement(mPopupgroupContent);
 
     nodeInfo = nodeInfoManager->GetNodeInfo(nsGkAtoms::tooltip, nullptr,
                                           kNameSpaceID_XUL,
                                           nsINode::ELEMENT_NODE);
 
@@ -185,31 +210,16 @@ void
 nsCanvasFrame::DestroyFrom(nsIFrame* aDestructRoot, PostDestroyData& aPostDestroyData)
 {
   nsIScrollableFrame* sf =
     PresContext()->GetPresShell()->GetRootScrollFrameAsScrollable();
   if (sf) {
     sf->RemoveScrollPositionListener(this);
   }
 
-  // Elements inserted in the custom content container have the same lifetime as
-  // the document, so before destroying the container, make sure to keep a clone
-  // of each of them at document level so they can be re-appended on reframe.
-  if (mCustomContentContainer) {
-    nsCOMPtr<nsIDocument> doc = mContent->OwnerDoc();
-    ErrorResult rv;
-
-    nsTArray<RefPtr<mozilla::dom::AnonymousContent>>& docAnonContents =
-      doc->GetAnonymousContents();
-    for (size_t i = 0, len = docAnonContents.Length(); i < len; ++i) {
-      AnonymousContent* content = docAnonContents[i];
-      nsCOMPtr<nsINode> clonedElement = content->GetContentNode()->CloneNode(true, rv);
-      content->SetContentNode(clonedElement->AsElement());
-    }
-  }
   aPostDestroyData.AddAnonymousContent(mCustomContentContainer.forget());
   if (mPopupgroupContent) {
     aPostDestroyData.AddAnonymousContent(mPopupgroupContent.forget());
   }
   if (mTooltipContent) {
     aPostDestroyData.AddAnonymousContent(mTooltipContent.forget());
   }
 
@@ -874,18 +884,17 @@ nsCanvasFrame::Reflow(nsPresContext*    
   NS_FRAME_TRACE_REFLOW_OUT("nsCanvasFrame::Reflow", aStatus);
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize);
 }
 
 nsresult
 nsCanvasFrame::GetContentForEvent(WidgetEvent* aEvent, nsIContent** aContent)
 {
   NS_ENSURE_ARG_POINTER(aContent);
-  nsresult rv = nsFrame::GetContentForEvent(aEvent,
-                                            aContent);
+  nsresult rv = nsFrame::GetContentForEvent(aEvent, aContent);
   if (NS_FAILED(rv) || !*aContent) {
     nsIFrame* kid = mFrames.FirstChild();
     if (kid) {
       rv = kid->GetContentForEvent(aEvent,
                                    aContent);
     }
   }