Bug 1426525 part 1. Remove XULDocument's mRefMap member, since there should be no XUL elements with 'ref' attributes anymore. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 05 Jan 2018 13:48:38 -0500
changeset 398072 c713ef0ecf0966e392acd6fe6d790c8844b70ec5
parent 398071 be780bf2c08a71780495c15d95b1cfc03c89b6a1
child 398073 07e377e19c7d462a4dc2e674e0163a400b3a572d
push id33201
push userapavel@mozilla.com
push dateSat, 06 Jan 2018 09:50:43 +0000
treeherdermozilla-central@ece8a96dfaa4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1426525, 1425356
milestone59.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 1426525 part 1. Remove XULDocument's mRefMap member, since there should be no XUL elements with 'ref' attributes anymore. r=smaug 'ref' used to be used for templates, but those are gone as of bug 1425356 being fixed. MozReview-Commit-ID: GerfZrckypp
dom/base/DocumentOrShadowRoot.cpp
dom/xul/XULDocument.cpp
dom/xul/XULDocument.h
--- a/dom/base/DocumentOrShadowRoot.cpp
+++ b/dom/base/DocumentOrShadowRoot.cpp
@@ -1,17 +1,16 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "DocumentOrShadowRoot.h"
 #include "mozilla/dom/StyleSheetList.h"
-#include "XULDocument.h"
 
 namespace mozilla {
 namespace dom {
 
 DocumentOrShadowRoot::DocumentOrShadowRoot(mozilla::dom::ShadowRoot& aShadowRoot)
   : mAsNode(aShadowRoot)
   , mKind(Kind::ShadowRoot)
 {}
@@ -39,21 +38,16 @@ DocumentOrShadowRoot::GetElementById(con
   }
 
   if (nsIdentifierMapEntry* entry = mIdentifierMap.GetEntry(aElementId)) {
     if (Element* el = entry->GetIdElement()) {
       return el;
     }
   }
 
-  if (MOZ_UNLIKELY(mKind == Kind::Document &&
-      static_cast<nsIDocument&>(AsNode()).IsXULDocument())) {
-    return static_cast<XULDocument&>(AsNode()).GetRefById(aElementId);
-  }
-
   return nullptr;
 }
 
 already_AddRefed<nsContentList>
 DocumentOrShadowRoot::GetElementsByTagNameNS(const nsAString& aNamespaceURI,
                                    const nsAString& aLocalName)
 {
   ErrorResult rv;
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -144,46 +144,16 @@ struct BroadcastListener {
 };
 
 struct BroadcasterMapEntry : public PLDHashEntryHdr
 {
     Element* mBroadcaster;  // [WEAK]
     nsTArray<BroadcastListener*> mListeners;  // [OWNING] of BroadcastListener objects
 };
 
-Element*
-nsRefMapEntry::GetFirstElement()
-{
-    return mRefContentList.SafeElementAt(0);
-}
-
-void
-nsRefMapEntry::AppendAll(nsCOMArray<Element>* aElements)
-{
-    for (size_t i = 0; i < mRefContentList.Length(); ++i) {
-        aElements->AppendObject(mRefContentList[i]);
-    }
-}
-
-bool
-nsRefMapEntry::AddElement(Element* aElement)
-{
-    if (mRefContentList.Contains(aElement)) {
-        return true;
-    }
-    return mRefContentList.AppendElement(aElement);
-}
-
-bool
-nsRefMapEntry::RemoveElement(Element* aElement)
-{
-    mRefContentList.RemoveElement(aElement);
-    return mRefContentList.IsEmpty();
-}
-
 //----------------------------------------------------------------------
 //
 // ctors & dtors
 //
 
 namespace mozilla {
 namespace dom {
 
@@ -886,34 +856,16 @@ XULDocument::ExecuteOnBroadcastHandlerFo
             EventDispatcher::Dispatch(child, aPresContext, &event, nullptr,
                                       &status);
         }
     }
 
     return NS_OK;
 }
 
-void
-XULDocument::AttributeWillChange(nsIDocument* aDocument,
-                                 Element* aElement, int32_t aNameSpaceID,
-                                 nsAtom* aAttribute, int32_t aModType,
-                                 const nsAttrValue* aNewValue)
-{
-    MOZ_ASSERT(aElement, "Null content!");
-    NS_PRECONDITION(aAttribute, "Must have an attribute that's changing!");
-
-    // XXXbz check aNameSpaceID, dammit!
-    // See if we need to update our ref map.
-    if (aAttribute == nsGkAtoms::ref) {
-        // Might not need this, but be safe for now.
-        nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
-        RemoveElementFromRefMap(aElement);
-    }
-}
-
 static bool
 ShouldPersistAttribute(Element* aElement, nsAtom* aAttribute)
 {
     if (aElement->IsXULElement(nsGkAtoms::window)) {
         // This is not an element of the top document, its owner is
         // not an nsXULWindow. Persist it.
         if (aElement->OwnerDoc()->GetParentDocument()) {
             return true;
@@ -937,22 +889,16 @@ XULDocument::AttributeChanged(nsIDocumen
                               nsAtom* aAttribute, int32_t aModType,
                               const nsAttrValue* aOldValue)
 {
     NS_ASSERTION(aDocument == this, "unexpected doc");
 
     // Might not need this, but be safe for now.
     nsCOMPtr<nsIMutationObserver> kungFuDeathGrip(this);
 
-    // XXXbz check aNameSpaceID, dammit!
-    // See if we need to update our ref map.
-    if (aAttribute == nsGkAtoms::ref) {
-        AddElementToRefMap(aElement);
-    }
-
     // Synchronize broadcast listeners
     if (mBroadcasterMap &&
         CanBroadcast(aNameSpaceID, aAttribute)) {
         auto entry = static_cast<BroadcasterMapEntry*>
                                 (mBroadcasterMap->Search(aElement));
 
         if (entry) {
             // We've got listeners: push the value.
@@ -1080,20 +1026,16 @@ XULDocument::GetElementsForID(const nsAS
                               nsCOMArray<Element>& aElements)
 {
     aElements.Clear();
 
     nsIdentifierMapEntry *entry = mIdentifierMap.GetEntry(aID);
     if (entry) {
         entry->AppendAllIdContent(&aElements);
     }
-    nsRefMapEntry *refEntry = mRefMap.GetEntry(aID);
-    if (refEntry) {
-        refEntry->AppendAll(&aElements);
-    }
 }
 
 nsresult
 XULDocument::AddForwardReference(nsForwardReference* aRef)
 {
     if (mResolutionPhase < aRef->GetPhase()) {
         if (!mForwardReferences.AppendElement(aRef)) {
             delete aRef;
@@ -1572,45 +1514,31 @@ XULDocument::SetTooltipNode(nsIDOMNode* 
 NS_IMETHODIMP
 XULDocument::GetCommandDispatcher(nsIDOMXULCommandDispatcher** aTracker)
 {
     *aTracker = mCommandDispatcher;
     NS_IF_ADDREF(*aTracker);
     return NS_OK;
 }
 
-Element*
-XULDocument::GetRefById(const nsAString& aID)
-{
-    if (nsRefMapEntry* refEntry = mRefMap.GetEntry(aID)) {
-        MOZ_ASSERT(refEntry->GetFirstElement());
-        return refEntry->GetFirstElement();
-    }
-
-    return nullptr;
-}
-
 nsresult
 XULDocument::AddElementToDocumentPre(Element* aElement)
 {
     // Do a bunch of work that's necessary when an element gets added
     // to the XUL Document.
     nsresult rv;
 
-    // 1. Add the element to the resource-to-element map. Also add it to
-    // the id map, since it seems this can be called when creating
-    // elements from prototypes.
+    // 1. Add the element to the id map, since it seems this can be
+    // called when creating elements from prototypes.
     nsAtom* id = aElement->GetID();
     if (id) {
         // FIXME: Shouldn't BindToTree take care of this?
         nsAutoScriptBlocker scriptBlocker;
         AddToIdTable(aElement, id);
     }
-    rv = AddElementToRefMap(aElement);
-    if (NS_FAILED(rv)) return rv;
 
     // 2. If the element is a 'command updater' (i.e., has a
     // "commandupdater='true'" attribute), then add the element to the
     // document's command dispatcher
     if (aElement->AttrValueIs(kNameSpaceID_None, nsGkAtoms::commandupdater,
                               nsGkAtoms::_true, eCaseMatters)) {
         rv = nsXULContentUtils::SetCommandUpdater(this, aElement);
         if (NS_FAILED(rv)) return rv;
@@ -1701,20 +1629,18 @@ XULDocument::RemoveSubtreeFromDocument(n
          child;
          child = child->GetPreviousSibling()) {
 
         rv = RemoveSubtreeFromDocument(child);
         if (NS_FAILED(rv))
             return rv;
     }
 
-    // 2. Remove the element from the resource-to-element map.
-    // Also remove it from the id map, since we added it in
+    // Remove the element from the id map, since we added it in
     // AddElementToDocumentPre().
-    RemoveElementFromRefMap(aElement);
     nsAtom* id = aElement->GetID();
     if (id) {
         // FIXME: Shouldn't UnbindFromTree take care of this?
         nsAutoScriptBlocker scriptBlocker;
         RemoveFromIdTable(aElement, id);
     }
 
     // 3. If the element is a 'command updater', then remove the
@@ -1738,56 +1664,16 @@ XULDocument::RemoveSubtreeFromDocument(n
                          broadcasterID, attribute, getter_AddRefs(broadcaster));
     if (rv == NS_FINDBROADCASTER_FOUND) {
         RemoveBroadcastListenerFor(*broadcaster, *listener, attribute);
     }
 
     return NS_OK;
 }
 
-static void
-GetRefMapAttribute(Element* aElement, nsAutoString* aValue)
-{
-    aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::ref, *aValue);
-}
-
-nsresult
-XULDocument::AddElementToRefMap(Element* aElement)
-{
-    // Look at the element's 'ref' attribute, and if set,
-    // add an entry in the resource-to-element map to the element.
-    nsAutoString value;
-    GetRefMapAttribute(aElement, &value);
-    if (!value.IsEmpty()) {
-        nsRefMapEntry *entry = mRefMap.PutEntry(value);
-        if (!entry)
-            return NS_ERROR_OUT_OF_MEMORY;
-        if (!entry->AddElement(aElement))
-            return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    return NS_OK;
-}
-
-void
-XULDocument::RemoveElementFromRefMap(Element* aElement)
-{
-    // Remove the element from the resource-to-element map.
-    nsAutoString value;
-    GetRefMapAttribute(aElement, &value);
-    if (!value.IsEmpty()) {
-        nsRefMapEntry *entry = mRefMap.GetEntry(value);
-        if (!entry)
-            return;
-        if (entry->RemoveElement(aElement)) {
-            mRefMap.RemoveEntry(entry);
-        }
-    }
-}
-
 //----------------------------------------------------------------------
 //
 // nsIDOMNode interface
 //
 
 nsresult
 XULDocument::Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                    bool aPreallocateChildren) const
@@ -2233,19 +2119,16 @@ XULDocument::PrepareToWalk()
     if (mState == eState_Master) {
         // Add the root element
         rv = CreateElementFromPrototype(proto, getter_AddRefs(root), true);
         if (NS_FAILED(rv)) return rv;
 
         rv = AppendChildTo(root, false);
         if (NS_FAILED(rv)) return rv;
 
-        rv = AddElementToRefMap(root);
-        if (NS_FAILED(rv)) return rv;
-
         // Block onload until we've finished building the complete
         // document content model.
         BlockOnload();
     }
 
     // There'd better not be anything on the context stack at this
     // point! This is the basis case for our "induction" in
     // ResumeWalk(), below, which'll assume that there's always a
--- a/dom/xul/XULDocument.h
+++ b/dom/xul/XULDocument.h
@@ -41,49 +41,16 @@ class nsIObjectOutputStream;
 #else
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsXULElement.h"
 #endif
 #include "nsURIHashKey.h"
 #include "nsInterfaceHashtable.h"
 
-class nsRefMapEntry : public nsStringHashKey
-{
-public:
-  explicit nsRefMapEntry(const nsAString& aKey) :
-    nsStringHashKey(&aKey)
-  {
-  }
-  explicit nsRefMapEntry(const nsAString* aKey) :
-    nsStringHashKey(aKey)
-  {
-  }
-  nsRefMapEntry(const nsRefMapEntry& aOther) :
-    nsStringHashKey(&aOther.GetKey())
-  {
-    NS_ERROR("Should never be called");
-  }
-
-  mozilla::dom::Element* GetFirstElement();
-  void AppendAll(nsCOMArray<mozilla::dom::Element>* aElements);
-  /**
-   * @return true if aElement was added, false if we failed due to OOM
-   */
-  bool AddElement(mozilla::dom::Element* aElement);
-  /**
-   * @return true if aElement was removed and it was the last content for
-   * this ref, so this entry should be removed from the map
-   */
-  bool RemoveElement(mozilla::dom::Element* aElement);
-
-private:
-  nsTArray<mozilla::dom::Element*> mRefContentList;
-};
-
 /**
  * The XUL document class
  */
 
 namespace mozilla {
 namespace dom {
 
 class XULDocument final : public XMLDocument,
@@ -117,17 +84,16 @@ public:
 
     virtual void EndLoad() override;
 
     // nsIMutationObserver interface
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
     NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
     NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
-    NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTEWILLCHANGE
 
     // nsIXULDocument interface
     virtual void GetElementsForID(const nsAString& aID,
                                   nsCOMArray<mozilla::dom::Element>& aElements) override;
 
     NS_IMETHOD AddSubtreeToDocument(nsIContent* aContent) override;
     NS_IMETHOD RemoveSubtreeFromDocument(nsIContent* aContent) override;
     NS_IMETHOD OnPrototypeLoadDone(bool aResumeWalk) override;
@@ -148,19 +114,16 @@ public:
     using mozilla::dom::DocumentOrShadowRoot::GetElementById;
     using nsDocument::GetImplementation;
     using nsDocument::GetTitle;
     using nsDocument::SetTitle;
     using nsDocument::GetLastStyleSheetSet;
     using nsDocument::MozSetImageElement;
     using nsIDocument::GetLocation;
 
-    // Helper for StyleScope::GetElementById.
-    Element* GetRefById(const nsAString & elementId);
-
     // nsIDOMXULDocument interface
     NS_DECL_NSIDOMXULDOCUMENT
 
     // nsICSSLoaderObserver
     NS_IMETHOD StyleSheetLoaded(mozilla::StyleSheet* aSheet,
                                 bool aWasAlternate,
                                 nsresult aStatus) override;
 
@@ -228,21 +191,16 @@ protected:
 
     // Implementation methods
     friend nsresult
     (::NS_NewXULDocument(nsIXULDocument** aResult));
 
     nsresult Init(void) override;
     nsresult StartLayout(void);
 
-    nsresult
-    AddElementToRefMap(Element* aElement);
-    void
-    RemoveElementFromRefMap(Element* aElement);
-
     nsresult GetViewportSize(int32_t* aWidth, int32_t* aHeight);
 
     nsresult PrepareToLoad(nsISupports* aContainer,
                            const char* aCommand,
                            nsIChannel* aChannel,
                            nsILoadGroup* aLoadGroup,
                            nsIParser** aResult);
 
@@ -313,19 +271,16 @@ protected:
     // for owning pointers and raw COM interface pointers for weak
     // (ie, non owning) references. If you add any members to this
     // class, please make the ownership explicit (pinkerton, scc).
     // NOTE, THIS IS STILL IN PROGRESS, TALK TO PINK OR SCC BEFORE
     // CHANGING
 
     XULDocument*             mNextSrcLoadWaiter;  // [OWNER] but not COMPtr
 
-    // Tracks elements with a 'ref' attribute, or an 'id' attribute where
-    // the element's namespace has no registered ID attribute name.
-    nsTHashtable<nsRefMapEntry> mRefMap;
     nsCOMPtr<nsIXULStore>       mLocalStore;
     bool                        mApplyingPersistedAttrs;
     bool                        mIsWritingFastLoad;
     bool                        mDocumentLoaded;
     /**
      * Since ResumeWalk is interruptible, it's possible that last
      * stylesheet finishes loading while the PD walk is still in
      * progress (waiting for an overlay to finish loading).