Issue #1873 - Part 2: Ensure normalized parent is used for UndisplayedMap handling code

We have four entry points that deal with the parents of display:none/ display:contents content. These are the functions for setting, changing, getting and removing a style context. Or more specifically:

GetStyleContextInMap called by GetDisplay[None|Contents]StyleFor

SetStyleContextInMap called by RegisterDisplay[None|Contents]StyleFor

ChangeStyleContextInMap called by ChangeRegisteredDisplay[None|Contents]StyleFor

UnregisterDisplay[None|Contents]StyleFor okay, this is actually two functions, but they act as a pair

This change makes all these functions call GetApplicableParent up front and act on and pass around the parent that it returns. This is so that throughout the code we are always handling the parent that will be used as the key in the UndisplayedMap entry. This is necessary so that all the code that sets/gets the 'MayHaveChildrenWithLayoutBoxesDisabled' bit on/from an nsIContent object is using the same object, otherwise everything breaks down.

Partially based on part 2 of https://bugzilla.mozilla.org/show_bug.cgi?id=1367214

1 files changed, 28 insertions, 8 deletions

diff --git a/layout/base/nsFrameManager.cpp b/layout/base/nsFrameManager.cpp
index df91e65fae..ef6a81ecdb 100644
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -90,16 +90,22 @@ public:
   // Removes all entries from the hash table
   void  Clear();
 
-protected:
-  LinkedList<UndisplayedNode>* GetListFor(nsIContent* aParentContent);
-  LinkedList<UndisplayedNode>* GetOrCreateListFor(nsIContent* aParentContent);
-  void AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent);
   /**
    * Get the applicable parent for the map lookup. This is almost always the
    * provided argument, except if it's a <xbl:children> element, in which case
    * it's the parent of the children element.
+   *
+   * All functions that are entry points into code that handles "parent"
+   * objects (used as the hash table keys) must ensure that the parent objects
+   * that they act on (and pass to other code) have been normalized by calling
+   * this method.
    */
-  nsIContent* GetApplicableParent(nsIContent* aParent);
+  static nsIContent* GetApplicableParent(nsIContent* aParent);
+
+protected:
+  LinkedList<UndisplayedNode>* GetListFor(nsIContent* aParentContent);
+  LinkedList<UndisplayedNode>* GetOrCreateListFor(nsIContent* aParentContent);
+  void AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent);
 };
 
 //----------------------------------------------------------------------
@@ -139,6 +145,9 @@ nsFrameManager::ParentForUndisplayedMap(const nsIContent* aContent)
   nsIContent* parent = aContent->GetParentElementCrossingShadowRoot();
   MOZ_ASSERT(parent || !aContent->GetParent(), "no non-elements");
 
+  // Normalize the parent
+  parent = UndisplayedMap::GetApplicableParent(parent);
+
   return parent;
 }
 
@@ -258,7 +267,11 @@ nsFrameManager::ClearUndisplayedContentIn(nsIContent* aContent,
   if (!mUndisplayedMap) {
     return;
   }
-  
+
+  // This function is an entry point into UndisplayedMap handling code, so we
+  // must call GetApplicableParent so the parent we pass around is correct.
+  aParentContent = UndisplayedMap::GetApplicableParent(aParentContent);
+
   if (aParentContent && 
       !aParentContent->MayHaveChildrenWithLayoutBoxesDisabled()) {
     MOZ_ASSERT(!mUndisplayedMap->GetFirstNode(aParentContent),
@@ -391,6 +404,10 @@ nsFrameManager::ClearDisplayContentsIn(nsIContent* aContent,
     return;
   }
 
+  // This function is an entry point into UndisplayedMap handling code, so we
+  // must call GetApplicableParent so the parent we pass around is correct.
+  aParentContent = UndisplayedMap::GetApplicableParent(aParentContent);
+
   if (aParentContent &&
       !aParentContent->MayHaveChildrenWithLayoutBoxesDisabled()) {
     MOZ_ASSERT(!mDisplayContentsMap->GetFirstNode(aParentContent),
@@ -693,7 +710,8 @@ nsFrameManagerBase::UndisplayedMap::GetApplicableParent(nsIContent* aParent)
 LinkedList<UndisplayedNode>*
 nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent* aParent)
 {
-  aParent = GetApplicableParent(aParent);
+  MOZ_ASSERT(aParent == GetApplicableParent(aParent),
+             "The parent that we use as the hash key must have been normalized");
 
   LinkedList<UndisplayedNode>* list;
   if (Get(aParent, &list)) {
@@ -706,7 +724,9 @@ nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent* aParent)
 LinkedList<UndisplayedNode>*
 nsFrameManagerBase::UndisplayedMap::GetOrCreateListFor(nsIContent* aParent)
 {
-  aParent = GetApplicableParent(aParent);
+  MOZ_ASSERT(aParent == GetApplicableParent(aParent),
+             "The parent that we use as the hash key must have been normalized");
+
   return LookupOrAdd(aParent);
 }