Bug 1461299: Make ESM not point to unbound NAC in the hover / active chain. r=smaug
☠☠ backed out by 9d8086740c86 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 14 May 2018 14:49:40 +0200
changeset 418298 fe2b8fcd7a475b65fd5cf85921a58ff22868ac53
parent 418297 9a0ca81ddbcea5f74bcdf30a5ae424ada99013b4
child 418299 9d8086740c860e8f7beeb890401b7d850d68f539
push id33997
push userncsoregi@mozilla.com
push dateTue, 15 May 2018 09:53:53 +0000
treeherdermozilla-central@cf3ee1402348 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1461299
milestone62.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 1461299: Make ESM not point to unbound NAC in the hover / active chain. r=smaug MozReview-Commit-ID: 8mL7Yv3TwQM
dom/events/EventStateManager.cpp
dom/events/EventStateManager.h
layout/base/nsFrameManager.cpp
layout/base/nsFrameManager.h
layout/generic/nsFrame.cpp
--- a/dom/events/EventStateManager.cpp
+++ b/dom/events/EventStateManager.cpp
@@ -5364,16 +5364,61 @@ EventStateManager::ResetLastOverForConte
   if (aElemWrapper && aElemWrapper->mLastOverElement &&
       nsContentUtils::ContentIsDescendantOf(aElemWrapper->mLastOverElement,
                                             aContent)) {
     aElemWrapper->mLastOverElement = nullptr;
   }
 }
 
 void
+EventStateManager::RemoveNodeFromChainIfNeeded(EventStates aState,
+                                               nsIContent* aContentRemoved)
+{
+  MOZ_ASSERT(aState == NS_EVENT_STATE_HOVER || aState == NS_EVENT_STATE_ACTIVE);
+  if (!aContentRemoved->IsElement() ||
+      !aContentRemoved->AsElement()->State().HasState(aState)) {
+    return;
+  }
+
+  nsCOMPtr<nsIContent>& leaf =
+    aState == NS_EVENT_STATE_HOVER ? mHoverContent : mActiveContent;
+
+  MOZ_ASSERT(leaf);
+  // XBL Likes to unbind content without notifying, thus the
+  // NODE_IS_ANONYMOUS_ROOT check...
+  MOZ_ASSERT(nsContentUtils::ContentIsFlattenedTreeDescendantOf(
+               leaf, aContentRemoved) ||
+             leaf->SubtreeRoot()->HasFlag(NODE_IS_ANONYMOUS_ROOT));
+
+  nsIContent* newLeaf = aContentRemoved->GetFlattenedTreeParent();
+  MOZ_ASSERT_IF(newLeaf,
+                newLeaf->IsElement() &&
+                newLeaf->AsElement()->State().HasState(aState));
+  if (aContentRemoved->IsRootOfNativeAnonymousSubtree()) {
+    // We don't update the removed content's state here, since removing NAC
+    // happens from layout and we don't really want to notify at that point or
+    // what not.
+    //
+    // Also, NAC is not observable and NAC being removed will go away soon.
+    leaf = newLeaf;
+  } else {
+    SetContentState(newLeaf, aState);
+  }
+  MOZ_ASSERT(leaf == newLeaf);
+}
+
+void
+EventStateManager::NativeAnonymousContentRemoved(nsIContent* aContent)
+{
+  MOZ_ASSERT(aContent->IsRootOfNativeAnonymousSubtree());
+  RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_HOVER, aContent);
+  RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_ACTIVE, aContent);
+}
+
+void
 EventStateManager::ContentRemoved(nsIDocument* aDocument, nsIContent* aContent)
 {
   /*
    * Anchor and area elements when focused or hovered might make the UI to show
    * the current link. We want to make sure that the UI gets informed when they
    * are actually removed from the DOM.
    */
   if (aContent->IsAnyOfHTMLElements(nsGkAtoms::a, nsGkAtoms::area) &&
@@ -5387,39 +5432,18 @@ EventStateManager::ContentRemoved(nsIDoc
   IMEStateManager::OnRemoveContent(mPresContext, aContent);
 
   // inform the focus manager that the content is being removed. If this
   // content is focused, the focus will be removed without firing events.
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   if (fm)
     fm->ContentRemoved(aDocument, aContent);
 
-  if (aContent->IsElement() &&
-      aContent->AsElement()->State().HasState(NS_EVENT_STATE_HOVER)) {
-    MOZ_ASSERT(mHoverContent);
-    // XBL Likes to unbind content without notifying, thus the
-    // NODE_IS_ANONYMOUS_ROOT check...
-    MOZ_ASSERT(nsContentUtils::ContentIsFlattenedTreeDescendantOf(mHoverContent,
-                                                                  aContent) ||
-               mHoverContent->SubtreeRoot()->HasFlag(NODE_IS_ANONYMOUS_ROOT));
-    // Since hover is hierarchical, set the current hover to the
-    // content's parent node.
-    SetContentState(aContent->GetFlattenedTreeParent(), NS_EVENT_STATE_HOVER);
-  }
-
-  if (aContent->IsElement() &&
-      aContent->AsElement()->State().HasState(NS_EVENT_STATE_ACTIVE)) {
-    MOZ_ASSERT(mActiveContent);
-    MOZ_ASSERT(nsContentUtils::ContentIsFlattenedTreeDescendantOf(mActiveContent,
-                                                                  aContent) ||
-               mHoverContent->SubtreeRoot()->HasFlag(NODE_IS_ANONYMOUS_ROOT));
-    // Active is hierarchical, so set the current active to the
-    // content's parent node.
-    SetContentState(aContent->GetFlattenedTreeParent(), NS_EVENT_STATE_ACTIVE);
-  }
+  RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_HOVER, aContent);
+  RemoveNodeFromChainIfNeeded(NS_EVENT_STATE_ACTIVE, aContent);
 
   if (sDragOverContent &&
       sDragOverContent->OwnerDoc() == aContent->OwnerDoc() &&
       nsContentUtils::ContentIsFlattenedTreeDescendantOf(sDragOverContent, aContent)) {
     sDragOverContent = nullptr;
   }
 
   PointerEventHandler::ReleaseIfCaptureByDescendant(aContent);
--- a/dom/events/EventStateManager.h
+++ b/dom/events/EventStateManager.h
@@ -143,16 +143,18 @@ public:
    * @return  Whether the content was able to change all states. Returns false
    *                  if a resulting DOM event causes the content node passed in
    *                  to not change states. Note, the frame for the content may
    *                  change as a result of the content state change, because of
    *                  frame reconstructions that may occur, but this does not
    *                  affect the return value.
    */
   bool SetContentState(nsIContent* aContent, EventStates aState);
+
+  void NativeAnonymousContentRemoved(nsIContent* aAnonContent);
   void ContentRemoved(nsIDocument* aDocument, nsIContent* aContent);
 
   bool EventStatusOK(WidgetGUIEvent* aEvent);
 
   /**
    * EventStateManager stores IMEContentObserver while it's observing contents.
    * Following mehtods are called by IMEContentObserver when it starts to
    * observe or stops observing the content.
@@ -1082,16 +1084,23 @@ protected:
   bool HandleCrossProcessEvent(WidgetEvent* aEvent,
                                nsEventStatus* aStatus);
 
   void ReleaseCurrentIMEContentObserver();
 
   void HandleQueryContentEvent(WidgetQueryContentEvent* aEvent);
 
 private:
+  // Removes a node from the :hover / :active chain if needed, notifying if the
+  // node is not a NAC subtree.
+  //
+  // Only meant to be called from ContentRemoved and
+  // NativeAnonymousContentRemoved.
+  void RemoveNodeFromChainIfNeeded(EventStates aState,
+                                   nsIContent* aContentRemoved);
 
   bool IsEventOutsideDragThreshold(WidgetInputEvent* aEvent) const;
 
   static inline void DoStateChange(dom::Element* aElement,
                                    EventStates aState, bool aAddState);
   static inline void DoStateChange(nsIContent* aContent, EventStates aState,
                                    bool aAddState);
   static void UpdateAncestorState(nsIContent* aStartNode,
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -255,20 +255,12 @@ nsFrameManager::RestoreFrameState(nsIFra
     nsFrameList::Enumerator childFrames(lists.CurrentList());
     for (; !childFrames.AtEnd(); childFrames.Next()) {
       RestoreFrameState(childFrames.get(), aState);
     }
   }
 }
 
 void
-nsFrameManager::DestroyAnonymousContent(already_AddRefed<nsIContent> aContent)
-{
-  if (nsCOMPtr<nsIContent> content = aContent) {
-    content->UnbindFromTree();
-  }
-}
-
-void
 nsFrameManager::AddSizeOfIncludingThis(nsWindowSizes& aSizes) const
 {
   aSizes.mLayoutPresShellSize += aSizes.mState.mMallocSizeOf(this);
 }
--- a/layout/base/nsFrameManager.h
+++ b/layout/base/nsFrameManager.h
@@ -88,18 +88,16 @@ public:
 
   /*
    * Add/restore state for one frame
    */
   void CaptureFrameStateFor(nsIFrame* aFrame, nsILayoutHistoryState* aState);
 
   void RestoreFrameStateFor(nsIFrame* aFrame, nsILayoutHistoryState* aState);
 
-  void DestroyAnonymousContent(already_AddRefed<nsIContent> aContent);
-
   void AddSizeOfIncludingThis(nsWindowSizes& aSizes) const;
 
 protected:
   // weak link, because the pres shell owns us
   nsIPresShell* MOZ_NON_OWNING_REF mPresShell;
   nsIFrame* mRootFrame;
 };
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -241,18 +241,20 @@ nsReflowStatus::UpdateTruncated(const Re
     mTruncated = false;
   }
 }
 
 /* static */ void
 nsIFrame::DestroyAnonymousContent(nsPresContext* aPresContext,
                                   already_AddRefed<nsIContent>&& aContent)
 {
-  aPresContext->PresShell()->FrameConstructor()
-              ->DestroyAnonymousContent(Move(aContent));
+  if (nsCOMPtr<nsIContent> content = aContent) {
+    aPresContext->EventStateManager()->NativeAnonymousContentRemoved(content);
+    content->UnbindFromTree();
+  }
 }
 
 // Formerly the nsIFrameDebug interface
 
 #ifdef DEBUG
 std::ostream& operator<<(std::ostream& aStream,
                          const nsReflowStatus& aStatus)
 {