Bug 1505887 - Fix FindChromeAccessOnlySubtreeOwner so that we handle UA widget being ChromeOnlyAccess root. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Nov 2018 22:28:56 +0100
changeset 507507 07789ba4c3d73b0318cbd9fa10c8e7f110565801
parent 507506 ad4ea72ce4d4f7c9150575f6173fa0fbb05e967c
child 507508 34b22b4e2b029b67e030d58f20b8b4bdc99331c4
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [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() ||