Bug 1449502: Cleanup a bit more the selector cache and CSSOM methods. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 29 Mar 2018 15:34:05 +0200
changeset 410615 e8407ca5e3923ae675ad07fee03f1bd889e08f1b
parent 410614 d3feb62465568e23b28f53c7665541149b8f0346
child 410616 7b24c2041026864c2ffbc1e0a7c982f1397b1b51
push id101539
push userecoal95@gmail.com
push dateThu, 29 Mar 2018 15:13:09 +0000
treeherdermozilla-inbound@e8407ca5e392 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1449502
milestone61.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 1449502: Cleanup a bit more the selector cache and CSSOM methods. r=xidorn MozReview-Commit-ID: 32FgbGFUdCM
dom/base/Element.cpp
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
dom/base/nsINode.cpp
dom/base/nsINode.h
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3468,47 +3468,32 @@ Element::GetTokenList(nsAtom* aAtom,
     SetProperty(aAtom, list, nsDOMTokenListPropertyDestructor);
   }
   return list;
 }
 
 Element*
 Element::Closest(const nsAString& aSelector, ErrorResult& aResult)
 {
-  return WithSelectorList<Element*>(
-    aSelector,
-    aResult,
-    [&](const RawServoSelectorList* aList) -> Element* {
-      if (!aList) {
-        return nullptr;
-      }
-      return const_cast<Element*>(Servo_SelectorList_Closest(this, aList));
-    },
-    [&](nsCSSSelectorList* aList) -> Element* {
-      MOZ_CRASH("old style system disabled");
-    }
-  );
+  const RawServoSelectorList* list = ParseSelectorList(aSelector, aResult);
+  if (!list) {
+    return nullptr;
+  }
+
+  return const_cast<Element*>(Servo_SelectorList_Closest(this, list));
 }
 
 bool
-Element::Matches(const nsAString& aSelector, ErrorResult& aError)
+Element::Matches(const nsAString& aSelector, ErrorResult& aResult)
 {
-  return WithSelectorList<bool>(
-    aSelector,
-    aError,
-    [&](const RawServoSelectorList* aList) {
-      if (!aList) {
-        return false;
-      }
-      return Servo_SelectorList_Matches(this, aList);
-    },
-    [&](nsCSSSelectorList* aList) {
-      MOZ_CRASH("old style system disabled");
-    }
-  );
+  const RawServoSelectorList* list = ParseSelectorList(aSelector, aResult);
+  if (!list) {
+    return false;
+  }
+  return Servo_SelectorList_Matches(this, list);
 }
 
 static const nsAttrValue::EnumTable kCORSAttributeTable[] = {
   // Order matters here
   // See ParseCORSValue
   { "anonymous",       CORS_ANONYMOUS       },
   { "use-credentials", CORS_USE_CREDENTIALS },
   { nullptr,           0 }
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -1305,36 +1305,16 @@ nsIDocument::SelectorCache::SelectorCach
       1000, "nsIDocument::SelectorCache", aEventTarget)
 { }
 
 nsIDocument::SelectorCache::~SelectorCache()
 {
   AgeAllGenerations();
 }
 
-void
-nsIDocument::SelectorCache::SelectorList::Reset()
-{
-  if (mServo) {
-    Servo_SelectorList_Drop(mServo);
-    mServo = nullptr;
-  }
-}
-
-
-void nsIDocument::SelectorCache::CacheList(
-  const nsAString& aSelector,
-  UniquePtr<RawServoSelectorList>&& aSelectorList)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  SelectorCacheKey* key = new SelectorCacheKey(aSelector);
-  mTable.Put(key->mKey, SelectorList(Move(aSelectorList)));
-  AddObject(key);
-}
-
 void nsIDocument::SelectorCache::NotifyExpired(SelectorCacheKey* aSelector)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aSelector);
 
   // There is no guarantee that this method won't be re-entered when selector
   // matching is ongoing because "memory-pressure" could be notified immediately
   // when OOM happens according to the design of nsExpirationTracker.
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -115,17 +115,16 @@ class nsRange;
 class nsSMILAnimationController;
 class nsSVGElement;
 class nsTextNode;
 class nsUnblockOnloadEvent;
 class nsWindowSizes;
 class nsDOMCaretPosition;
 class nsViewportInfo;
 class nsIGlobalObject;
-struct nsCSSSelectorList;
 
 namespace mozilla {
 class AbstractThread;
 class CSSStyleSheet;
 class Encoding;
 class ErrorResult;
 class EventStates;
 class EventListenerManager;
@@ -1494,104 +1493,54 @@ private:
   };
 
   class SelectorCacheKeyDeleter;
 
 public:
   class SelectorCache final
     : public nsExpirationTracker<SelectorCacheKey, 4>
   {
-    public:
-      class SelectorList
-      {
-      public:
-        SelectorList()
-          : mGecko(nullptr)
-        {}
-
-        SelectorList(SelectorList&& aOther)
-        {
-          *this = mozilla::Move(aOther);
-        }
-
-        SelectorList& operator=(SelectorList&& aOther)
-        {
-          Reset();
-          mServo = aOther.mServo;
-          aOther.mServo = nullptr;
-          return *this;
-        }
-
-        SelectorList(const SelectorList& aOther) = delete;
-
-        explicit SelectorList(mozilla::UniquePtr<RawServoSelectorList>&& aList)
-          : mServo(aList.release())
-        {}
-
-
-        ~SelectorList() {
-          Reset();
-        }
-
-        explicit operator bool() const
-        {
-          return !!AsServo();
-        }
-
-        nsCSSSelectorList* AsGecko() const
-        {
-          return mGecko;
-        }
-
-        RawServoSelectorList* AsServo() const
-        {
-          return mServo;
-        }
-
-      private:
-        void Reset();
-
-        union {
-          nsCSSSelectorList* mGecko;
-          RawServoSelectorList* mServo;
-        };
-      };
-
-      explicit SelectorCache(nsIEventTarget* aEventTarget);
-
-      // CacheList takes ownership of aSelectorList.
-      void CacheList(const nsAString& aSelector,
-                     mozilla::UniquePtr<nsCSSSelectorList>&& aSelectorList);
-      void CacheList(const nsAString& aSelector,
-                     mozilla::UniquePtr<RawServoSelectorList>&& aSelectorList);
-
-      virtual void NotifyExpired(SelectorCacheKey* aSelector) override;
-
-      // We do not call MarkUsed because it would just slow down lookups and
-      // because we're OK expiring things after a few seconds even if they're
-      // being used.  Returns whether we actually had an entry for aSelector.
-      //
-      // If we have an entry and the selector list returned has a null
-      // nsCSSSelectorList*/RawServoSelectorList*, that indicates that aSelector
-      // has already been parsed and is not a syntactically valid selector.
-      SelectorList* GetList(const nsAString& aSelector)
-      {
-        return mTable.GetValue(aSelector);
-      }
-
-      ~SelectorCache();
-
-    private:
-      nsDataHashtable<nsStringHashKey, SelectorList> mTable;
+  public:
+    using SelectorList = mozilla::UniquePtr<RawServoSelectorList>;
+
+    explicit SelectorCache(nsIEventTarget* aEventTarget);
+
+    void CacheList(const nsAString& aSelector, SelectorList aSelectorList)
+    {
+      MOZ_ASSERT(NS_IsMainThread());
+      SelectorCacheKey* key = new SelectorCacheKey(aSelector);
+      mTable.Put(key->mKey, Move(aSelectorList));
+      AddObject(key);
+    }
+
+    void NotifyExpired(SelectorCacheKey* aSelector) final;
+
+    // We do not call MarkUsed because it would just slow down lookups and
+    // because we're OK expiring things after a few seconds even if they're
+    // being used.  Returns whether we actually had an entry for aSelector.
+    //
+    // If we have an entry and the selector list returned has a null
+    // RawServoSelectorList*, that indicates that aSelector has already been
+    // parsed and is not a syntactically valid selector.
+    SelectorList* GetList(const nsAString& aSelector)
+    {
+      return mTable.GetValue(aSelector);
+    }
+
+    ~SelectorCache();
+
+  private:
+    nsDataHashtable<nsStringHashKey, SelectorList> mTable;
   };
 
   SelectorCache& GetSelectorCache() {
     if (!mSelectorCache) {
-      mSelectorCache.reset(new SelectorCache(
-        EventTargetFor(mozilla::TaskCategory::Other)));
+      mSelectorCache =
+        mozilla::MakeUnique<SelectorCache>(
+          EventTargetFor(mozilla::TaskCategory::Other));
     }
     return *mSelectorCache;
   }
   // Get the root <html> element, or return null if there isn't one (e.g.
   // if the root isn't <html>)
   Element* GetHtmlElement() const;
   // Returns the first child of GetHtmlContent which has the given tag,
   // or nullptr if that doesn't exist.
--- a/dom/base/nsINode.cpp
+++ b/dom/base/nsINode.cpp
@@ -2484,53 +2484,53 @@ nsINode::Length() const
     return AsContent()->TextLength();
 
   default:
     return GetChildCount();
   }
 }
 
 const RawServoSelectorList*
-nsINode::ParseServoSelectorList(
-  const nsAString& aSelectorString,
-  ErrorResult& aRv)
+nsINode::ParseSelectorList(const nsAString& aSelectorString,
+                           ErrorResult& aRv)
 {
   nsIDocument* doc = OwnerDoc();
 
   nsIDocument::SelectorCache& cache = doc->GetSelectorCache();
   nsIDocument::SelectorCache::SelectorList* list =
     cache.GetList(aSelectorString);
   if (list) {
     if (!*list) {
       // Invalid selector.
       aRv.ThrowDOMException(NS_ERROR_DOM_SYNTAX_ERR,
         NS_LITERAL_CSTRING("'") + NS_ConvertUTF16toUTF8(aSelectorString) +
         NS_LITERAL_CSTRING("' is not a valid selector")
       );
       return nullptr;
     }
 
-    return list->AsServo();
+    return list->get();
   }
 
   NS_ConvertUTF16toUTF8 selectorString(aSelectorString);
 
-  auto* selectorList = Servo_SelectorList_Parse(&selectorString);
+  auto selectorList =
+    UniquePtr<RawServoSelectorList>(Servo_SelectorList_Parse(&selectorString));
   if (!selectorList) {
     aRv.ThrowDOMException(NS_ERROR_DOM_SYNTAX_ERR,
       NS_LITERAL_CSTRING("'") + selectorString +
       NS_LITERAL_CSTRING("' is not a valid selector")
     );
   }
 
-  cache.CacheList(aSelectorString, UniquePtr<RawServoSelectorList>(selectorList));
-  return selectorList;
+  auto* ret = selectorList.get();
+  cache.CacheList(aSelectorString, Move(selectorList));
+  return ret;
 }
 
-
 namespace {
 struct SelectorMatchInfo {
 };
 } // namespace
 
 // Given an id, find elements with that id under aRoot that match aMatchInfo if
 // any is provided.  If no SelectorMatchInfo is provided, just find the ones
 // with the given id.  aRoot must be in the document.
@@ -2586,54 +2586,36 @@ struct ElementHolder {
   Element* ElementAt(uint32_t aIndex) { return nullptr; }
 
   Element* mElement;
 };
 
 Element*
 nsINode::QuerySelector(const nsAString& aSelector, ErrorResult& aResult)
 {
-  return WithSelectorList<Element*>(
-    aSelector,
-    aResult,
-    [&](const RawServoSelectorList* aList) -> Element* {
-      if (!aList) {
-        return nullptr;
-      }
-      const bool useInvalidation = false;
-      return const_cast<Element*>(
-          Servo_SelectorList_QueryFirst(this, aList, useInvalidation));
-    },
-    [&](nsCSSSelectorList* aList) -> Element* {
-      MOZ_CRASH("old style system disabled");
-    }
-  );
+  const RawServoSelectorList* list = ParseSelectorList(aSelector, aResult);
+  if (!list) {
+    return nullptr;
+  }
+  const bool useInvalidation = false;
+  return const_cast<Element*>(
+    Servo_SelectorList_QueryFirst(this, list, useInvalidation));
 }
 
 already_AddRefed<nsINodeList>
 nsINode::QuerySelectorAll(const nsAString& aSelector, ErrorResult& aResult)
 {
   RefPtr<nsSimpleContentList> contentList = new nsSimpleContentList(this);
-
-  WithSelectorList<void>(
-    aSelector,
-    aResult,
-    [&](const RawServoSelectorList* aList) {
-      if (!aList) {
-        return;
-      }
-      const bool useInvalidation = false;
-      Servo_SelectorList_QueryAll(
-        this, aList, contentList.get(), useInvalidation);
-    },
-    [&](nsCSSSelectorList* aList) {
-      MOZ_CRASH("old style system disabled");
-    }
-  );
-
+  const RawServoSelectorList* list = ParseSelectorList(aSelector, aResult);
+  if (!list) {
+    return contentList.forget();
+  }
+
+  const bool useInvalidation = false;
+  Servo_SelectorList_QueryAll(this, list, contentList.get(), useInvalidation);
   return contentList.forget();
 }
 
 Element*
 nsINode::GetElementById(const nsAString& aId)
 {
   MOZ_ASSERT(IsElement() || IsNodeOfType(eDOCUMENT_FRAGMENT),
              "Bogus this object for GetElementById call");
--- a/dom/base/nsINode.h
+++ b/dom/base/nsINode.h
@@ -32,17 +32,16 @@
 #ifdef XP_WIN
 #ifdef GetClassInfo
 #undef GetClassInfo
 #endif
 #endif
 
 class nsAttrAndChildArray;
 class nsAttrChildContentList;
-struct nsCSSSelectorList;
 class nsDOMAttributeMap;
 class nsIAnimationObserver;
 class nsIContent;
 class nsIDocument;
 class nsIDOMElement;
 class nsIDOMNodeList;
 class nsIFrame;
 class nsIMutationObserver;
@@ -2007,58 +2006,25 @@ protected:
    * @param aIndex The index to insert at.
    * @param aNotify Whether to notify.
    * @param aChildArray The child array to work with
    */
   nsresult doInsertChildAt(nsIContent* aKid, uint32_t aIndex,
                            bool aNotify, nsAttrAndChildArray& aChildArray);
 
   /**
-   * Parse the given selector string into an nsCSSSelectorList.
-   *
-   * A null return value with a non-failing aRv means the string only
-   * contained pseudo-element selectors.
-   *
-   * A failing aRv means the string was not a valid selector.
-   *
-   * Note that the selector list returned here is owned by the owner doc's
-   * selector cache.
-   */
-  nsCSSSelectorList* ParseSelectorList(const nsAString& aSelectorString,
-                                       mozilla::ErrorResult& aRv);
-
-  /**
    * Parse the given selector string into a servo SelectorList.
    *
    * Never returns null if aRv is not failing.
    *
    * Note that the selector list returned here is owned by the owner doc's
    * selector cache.
    */
-  const RawServoSelectorList* ParseServoSelectorList(
-    const nsAString& aSelectorString,
-    mozilla::ErrorResult& aRv);
-
-  /**
-   * Parse the given selector string a SelectorList, depending on whether we're
-   * in a Servo or Gecko-backed document, and execute either aServoFunctor or
-   * aGeckoFunctor on it.
-   *
-   * Note that the selector list is owned by the owner doc's selector cache
-   * which can get expired, so you shouldn't keep it around for long.
-   */
-  template<typename Ret, typename ServoFunctor, typename GeckoFunctor>
-  Ret WithSelectorList(
-    const nsAString& aSelectorString,
-    mozilla::ErrorResult& aRv,
-    const ServoFunctor& aServoFunctor,
-    const GeckoFunctor& aGeckoFunctor)
-  {
-    return aServoFunctor(ParseServoSelectorList(aSelectorString, aRv));
-  }
+  const RawServoSelectorList* ParseSelectorList(const nsAString& aSelectorString,
+                                                mozilla::ErrorResult&);
 
 public:
   /* Event stuff that documents and elements share.  This needs to be
      NS_IMETHOD because some subclasses implement DOM methods with
      this exact name and signature and then the calling convention
      needs to match.
 
      Note that we include DOCUMENT_ONLY_EVENT events here so that we