Bug 1489139 - Ensure unbound generated content doesn't remain in the active chain. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 11 Sep 2018 21:18:04 +0200
changeset 436028 b28cb41dd456532982ca7a2d579908b95f4ac17c
parent 436027 e5c0d34d8fdf1152de53a09421679f71a2fb7a0d
child 436029 5e62e70cd869ba3746b52498ca4d3b1aa3fee223
push id34625
push userdvarga@mozilla.com
push dateThu, 13 Sep 2018 02:31:40 +0000
treeherdermozilla-central@51e9e9660b3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1489139, 1461299
milestone64.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 1489139 - Ensure unbound generated content doesn't remain in the active chain. r=smaug This reuses the same code path that was added in bug 1461299 for NAC, but for generated content as well. DestroyAnonymousContent notifies to the ESM. Also, remove the NativeAnonymousContentRemoved bit about <svg:use> since it's no longer NAC. Differential Revision: https://phabricator.services.mozilla.com/D5575
dom/events/EventStateManager.cpp
dom/events/test/mochitest.ini
dom/events/test/test_unbound_before_in_active_chain.html
layout/generic/nsFrame.cpp
layout/generic/nsFrameList.h
layout/generic/nsIFrame.h
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -5373,19 +5373,17 @@ EventStateManager::RemoveNodeFromChainIf
     leaf = newLeaf;
   }
   MOZ_ASSERT(leaf == newLeaf);
 }
 
 void
 EventStateManager::NativeAnonymousContentRemoved(nsIContent* aContent)
 {
-  // FIXME(bug 1450250): <svg:use> is nasty.
-  MOZ_ASSERT(aContent->IsRootOfNativeAnonymousSubtree() ||
-             aContent->GetParentNode()->IsSVGElement(nsGkAtoms::use));
+  MOZ_ASSERT(aContent->IsRootOfNativeAnonymousSubtree());
   RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_HOVER, aContent, false);
   RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_ACTIVE, aContent, false);
 }
 
 void
 EventStateManager::ContentRemoved(nsIDocument* aDocument, nsIContent* aContent)
 {
   /*
--- a/dom/events/test/mochitest.ini
+++ b/dom/events/test/mochitest.ini
@@ -206,8 +206,9 @@ support-files = file_bug1446834.html
 support-files = window_bug1447993.html
 skip-if = toolkit == 'android'
 [test_bug1484371.html]
 support-files = file_bug1484371.html
 [test_dnd_with_modifiers.html]
 [test_hover_mouseleave.html]
 [test_slotted_mouse_event.html]
 [test_slotted_text_click.html]
+[test_unbound_before_in_active_chain.html]
new file mode 100644
--- /dev/null
+++ b/dom/events/test/test_unbound_before_in_active_chain.html
@@ -0,0 +1,38 @@
+<!doctype html>
+<title>Test for bug 1489139: Unbound generated content in the active chain</title>
+<script src="/tests/SimpleTest/EventUtils.js"></script>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+#target, #target::before {
+  width: 200px;
+  height: 200px;
+}
+
+#target::before {
+  content: " ";
+  display: block;
+  background: green;
+}
+
+#target:active::before {
+  content: "";
+  background: red;
+}
+</style>
+Should see a green square after clicking.
+<div id="target"></div>
+<script>
+SimpleTest.waitForExplicitFinish();
+onload = function() {
+  let target = document.getElementById("target");
+  requestAnimationFrame(() => {
+    synthesizeMouseAtPoint(100, 100, { type: "mousedown" })
+    ok(target.matches(":active"), "Should have been clicked");
+    requestAnimationFrame(() => {
+      synthesizeMouseAtPoint(100, 100, { type: "mouseup" })
+      ok(!target.matches(':active'), "Should stop matching :active afterwards");
+      SimpleTest.finish();
+    });
+  });
+}
+</script>
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -810,17 +810,17 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
   // 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());
+      aPostDestroyData.AddAnonymousContent(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
--- a/layout/generic/nsFrameList.h
+++ b/layout/generic/nsFrameList.h
@@ -52,24 +52,21 @@ namespace layout {
   };
 
   // A helper class for nsIFrame::Destroy[From].  It's defined here because
   // nsFrameList needs it and we can't use nsIFrame here.
   struct PostFrameDestroyData {
     PostFrameDestroyData(const PostFrameDestroyData&) = delete;
     PostFrameDestroyData() = default;
 
-    AutoTArray<RefPtr<nsIContent>, 50> mAnonymousContent;
-    AutoTArray<RefPtr<nsIContent>, 50> mGeneratedContent;
-    void AddAnonymousContent(already_AddRefed<nsIContent>&& aContent) {
+    AutoTArray<RefPtr<nsIContent>, 100> mAnonymousContent;
+    void AddAnonymousContent(already_AddRefed<nsIContent>&& aContent)
+    {
       mAnonymousContent.AppendElement(aContent);
     }
-    void AddGeneratedContent(already_AddRefed<nsIContent>&& aContent) {
-      mGeneratedContent.AppendElement(aContent);
-    }
   };
 } // namespace layout
 } // namespace mozilla
 
 // Uncomment this to enable expensive frame-list integrity checking
 // #define DEBUG_FRAME_LIST
 
 /**
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -654,19 +654,16 @@ public:
   struct MOZ_RAII AutoPostDestroyData
   {
     explicit AutoPostDestroyData(nsPresContext* aPresContext)
       : mPresContext(aPresContext) {}
     ~AutoPostDestroyData() {
       for (auto& content : mozilla::Reversed(mData.mAnonymousContent)) {
         nsIFrame::DestroyAnonymousContent(mPresContext, content.forget());
       }
-      for (auto& content : mozilla::Reversed(mData.mGeneratedContent)) {
-        content->UnbindFromTree();
-      }
     }
     nsPresContext* mPresContext;
     PostDestroyData mData;
   };
   /**
    * Destroys this frame and each of its child frames (recursively calls
    * Destroy() for each child). If this frame is a first-continuation, this
    * also removes the frame from the primary frame map and clears undisplayed