Bug 1505887 - Fix FindChromeAccessOnlySubtreeOwner so that we handle UA widget being ChromeOnlyAccess root. r=smaug
☠☠ backed out by dc298299ebad ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Nov 2018 22:28:56 +0100
changeset 504715 490a99122a7fc04ad0ec6bf9e32036c36b92d631
parent 504714 7b9afff4ff11f683d9a5e46ae92d80d6e9e7add3
child 504716 dc6c022e9fe127bd0d32e25e3206f79e3d0d954c
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1505887
milestone65.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 1505887 - Fix FindChromeAccessOnlySubtreeOwner so that we handle UA widget being ChromeOnlyAccess root. r=smaug A single video controls test crashed without this, while dereferencing a null anonOwnerRelated in: https://searchfox.org/mozilla-central/rev/c0b26c40769a1e5607a1ae8be37fe64df64fc55e/dom/base/FragmentOrElement.cpp#964 I think this is the right fix for it, but the code that uses this is kind of complex, so worth double-checking... :)
dom/base/FragmentOrElement.cpp
dom/base/nsIContent.h
dom/base/nsINode.h
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -878,42 +878,39 @@ FragmentOrElement::GetChildren(uint32_t 
   AllChildrenIterator iter(this, aFilter);
   while (nsIContent* kid = iter.GetNextChild()) {
     list->AppendElement(kid);
   }
 
   return list.forget();
 }
 
-static nsIContent*
-FindChromeAccessOnlySubtreeOwner(nsIContent* aContent)
+static nsINode*
+FindChromeAccessOnlySubtreeOwner(nsINode* aNode)
 {
-  if (aContent->ChromeOnlyAccess()) {
-    bool chromeAccessOnly = false;
-    while (aContent && !chromeAccessOnly) {
-      chromeAccessOnly = aContent->IsRootOfChromeAccessOnlySubtree();
-      aContent = aContent->GetParent();
-    }
+  if (!aNode->ChromeOnlyAccess()) {
+    return aNode;
   }
-  return aContent;
+
+  while (aNode && !aNode->IsRootOfChromeAccessOnlySubtree()) {
+    aNode = aNode->GetParentNode();
+  }
+
+  return aNode ? aNode->GetParentOrHostNode() : nullptr;
 }
 
 already_AddRefed<nsINode>
 FindChromeAccessOnlySubtreeOwner(EventTarget* aTarget)
 {
   nsCOMPtr<nsINode> node = do_QueryInterface(aTarget);
   if (!node || !node->ChromeOnlyAccess()) {
     return node.forget();
   }
 
-  if (!node->IsContent()) {
-    return nullptr;
-  }
-
-  node = FindChromeAccessOnlySubtreeOwner(node->AsContent());
+  node = FindChromeAccessOnlySubtreeOwner(node);
   return node.forget();
 }
 
 void
 nsIContent::GetEventTargetParent(EventChainPreVisitor& aVisitor)
 {
   //FIXME! Document how this event retargeting works, Bug 329124.
   aVisitor.mCanHandle = true;
@@ -946,19 +943,19 @@ nsIContent::GetEventTargetParent(EventCh
       // target is descendant of an element which is anonymous for events,
       // we may want to stop event propagation.
       // If this is the original target, aVisitor.mRelatedTargetIsInAnon
       // must be updated.
       if (isAnonForEvents || aVisitor.mRelatedTargetIsInAnon ||
           (aVisitor.mEvent->mOriginalTarget == this &&
            (aVisitor.mRelatedTargetIsInAnon =
             relatedTarget->ChromeOnlyAccess()))) {
-        nsIContent* anonOwner = FindChromeAccessOnlySubtreeOwner(this);
+        nsINode* anonOwner = FindChromeAccessOnlySubtreeOwner(this);
         if (anonOwner) {
-          nsIContent* anonOwnerRelated =
+          nsINode* anonOwnerRelated =
             FindChromeAccessOnlySubtreeOwner(relatedTarget);
           if (anonOwnerRelated) {
             // Note, anonOwnerRelated may still be inside some other
             // native anonymous subtree. The case where anonOwner is still
             // inside native anonymous subtree will be handled when event
             // propagates up in the DOM tree.
             while (anonOwner != anonOwnerRelated &&
                    anonOwnerRelated->ChromeOnlyAccess()) {
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@@ -174,36 +174,16 @@ public:
    * @note calling this method with eAllButXBL will return children that are
    *  also in the eAllButXBL and eAllChildren child lists of other descendants
    *  of this node in the tree, but those other nodes cannot be reached from the
    *  eAllButXBL child list.
    */
   virtual already_AddRefed<nsINodeList> GetChildren(uint32_t aFilter) = 0;
 
   /**
-   * Get whether this content is C++-generated anonymous content
-   * @see nsIAnonymousContentCreator
-   * @return whether this content is anonymous
-   */
-  bool IsRootOfNativeAnonymousSubtree() const
-  {
-    NS_ASSERTION(!HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT) ||
-                 (HasFlag(NODE_IS_ANONYMOUS_ROOT) &&
-                  HasFlag(NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE)),
-                 "Some flags seem to be missing!");
-    return HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT);
-  }
-
-  bool IsRootOfChromeAccessOnlySubtree() const
-  {
-    return HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT |
-                   NODE_IS_ROOT_OF_CHROME_ONLY_ACCESS);
-  }
-
-  /**
    * Makes this content anonymous
    * @see nsIAnonymousContentCreator
    */
   void SetIsNativeAnonymousRoot()
   {
     SetFlags(NODE_IS_ANONYMOUS_ROOT | NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE |
              NODE_IS_NATIVE_ANONYMOUS_ROOT);
   }
--- a/dom/base/nsINode.h
+++ b/dom/base/nsINode.h
@@ -1228,16 +1228,36 @@ public:
   }
 
   bool IsInShadowTree() const
   {
     return HasFlag(NODE_IS_IN_SHADOW_TREE);
   }
 
   /**
+   * Get whether this node is C++-generated anonymous content
+   * @see nsIAnonymousContentCreator
+   * @return whether this content is anonymous
+   */
+  bool IsRootOfNativeAnonymousSubtree() const
+  {
+    NS_ASSERTION(!HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT) ||
+                 (HasFlag(NODE_IS_ANONYMOUS_ROOT) &&
+                  HasFlag(NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE)),
+                 "Some flags seem to be missing!");
+    return HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT);
+  }
+
+  bool IsRootOfChromeAccessOnlySubtree() const
+  {
+    return HasFlag(NODE_IS_NATIVE_ANONYMOUS_ROOT |
+                   NODE_IS_ROOT_OF_CHROME_ONLY_ACCESS);
+  }
+
+  /**
    * Returns true if |this| node is the common ancestor of the start/end
    * nodes of a Range in a Selection or a descendant of such a common ancestor.
    * This node is definitely not selected when |false| is returned, but it may
    * or may not be selected when |true| is returned.
    */
   bool IsSelectionDescendant() const
   {
     return IsDescendantOfCommonAncestorForRangeInSelection() ||