Bug 1486182: Part 1 - Include both category names and values in category enumerator. r=froydnj
authorKris Maglione <maglione.k@gmail.com>
Fri, 24 Aug 2018 22:22:07 -0700
changeset 488809 8340805509c5503ff44c5d47d7333aa206960a58
parent 488808 7edee7974317eae6ce129eadf99833a58b6f543a
child 488810 a6435d51a3463a46e4a86813bb9f2816d33973f3
push id9734
push usershindli@mozilla.com
push dateThu, 30 Aug 2018 12:18:07 +0000
treeherdermozilla-beta@71c71ab3afae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1486182
milestone63.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 1486182: Part 1 - Include both category names and values in category enumerator. r=froydnj Nearly all of the consumers of category enumerators require the entry value, either along with or instead of the name. Including both by default simplifies things considerably for most consumers, and allows us to remove the XPCOMUtils wrapper that JS callers typically use to enumerate category entries. Differential Revision: https://phabricator.services.mozilla.com/D4277
toolkit/components/commandlines/nsCommandLine.cpp
xpcom/components/nsCategoryCache.cpp
xpcom/components/nsCategoryManager.cpp
xpcom/components/nsICategoryManager.idl
--- a/toolkit/components/commandlines/nsCommandLine.cpp
+++ b/toolkit/components/commandlines/nsCommandLine.cpp
@@ -8,16 +8,17 @@
 #include "nsICommandLineHandler.h"
 #include "nsICommandLineValidator.h"
 #include "nsIConsoleService.h"
 #include "nsIClassInfoImpl.h"
 #include "nsIDOMWindow.h"
 #include "nsIFile.h"
 #include "nsISimpleEnumerator.h"
 #include "nsIStringEnumerator.h"
+#include "mozilla/SimpleEnumerator.h"
 
 #include "nsCOMPtr.h"
 #include "mozilla/ModuleUtils.h"
 #include "nsISupportsImpl.h"
 #include "nsNativeCharsetUtils.h"
 #include "nsNetUtil.h"
 #include "nsIFileProtocolHandler.h"
 #include "nsIURI.h"
@@ -40,16 +41,18 @@
 
 #ifdef DEBUG_bsmedberg
 #define DEBUG_COMMANDLINE
 #endif
 
 #define NS_COMMANDLINE_CID \
   { 0x23bcc750, 0xdc20, 0x460b, { 0xb2, 0xd4, 0x74, 0xd8, 0xf5, 0x8d, 0x36, 0x15 } }
 
+using mozilla::SimpleEnumerator;
+
 class nsCommandLine final : public nsICommandLineRunner
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSICOMMANDLINE
   NS_DECL_NSICOMMANDLINERUNNER
 
   nsCommandLine();
@@ -501,32 +504,25 @@ nsCommandLine::EnumerateHandlers(Enumera
     (do_GetService(NS_CATEGORYMANAGER_CONTRACTID));
   NS_ENSURE_TRUE(catman, NS_ERROR_UNEXPECTED);
 
   nsCOMPtr<nsISimpleEnumerator> entenum;
   rv = catman->EnumerateCategory("command-line-handler",
                                  getter_AddRefs(entenum));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsCOMPtr<nsIUTF8StringEnumerator> strenum (do_QueryInterface(entenum));
-  NS_ENSURE_TRUE(strenum, NS_ERROR_UNEXPECTED);
-
-  nsAutoCString entry;
-  bool hasMore;
-  while (NS_SUCCEEDED(strenum->HasMore(&hasMore)) && hasMore) {
-    strenum->GetNext(entry);
-
-    nsCString contractID;
-    rv = catman->GetCategoryEntry("command-line-handler", entry,
-				  contractID);
-    if (NS_FAILED(rv))
-      continue;
+  for (auto& categoryEntry : SimpleEnumerator<nsICategoryEntry>(entenum)) {
+    nsAutoCString contractID;
+    categoryEntry->GetValue(contractID);
 
     nsCOMPtr<nsICommandLineHandler> clh(do_GetService(contractID.get()));
     if (!clh) {
+      nsCString entry;
+      categoryEntry->GetEntry(entry);
+
       LogConsoleMessage(u"Contract ID '%s' was registered as a command line handler for entry '%s', but could not be created.",
                         contractID.get(), entry.get());
       continue;
     }
 
     rv = (aCallback)(clh, this, aClosure);
     if (rv == NS_ERROR_ABORT)
       break;
@@ -549,26 +545,19 @@ nsCommandLine::EnumerateValidators(Enume
   nsCOMPtr<nsISimpleEnumerator> entenum;
   rv = catman->EnumerateCategory("command-line-validator",
                                  getter_AddRefs(entenum));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIUTF8StringEnumerator> strenum (do_QueryInterface(entenum));
   NS_ENSURE_TRUE(strenum, NS_ERROR_UNEXPECTED);
 
-  nsAutoCString entry;
-  bool hasMore;
-  while (NS_SUCCEEDED(strenum->HasMore(&hasMore)) && hasMore) {
-    strenum->GetNext(entry);
-
-    nsCString contractID;
-    rv = catman->GetCategoryEntry("command-line-validator",
-				  entry, contractID);
-    if (contractID.IsVoid())
-      continue;
+  for (auto& categoryEntry : SimpleEnumerator<nsICategoryEntry>(entenum)) {
+    nsAutoCString contractID;
+    categoryEntry->GetValue(contractID);
 
     nsCOMPtr<nsICommandLineValidator> clv(do_GetService(contractID.get()));
     if (!clv)
       continue;
 
     rv = (aCallback)(clv, this, aClosure);
     if (rv == NS_ERROR_ABORT)
       break;
--- a/xpcom/components/nsCategoryCache.cpp
+++ b/xpcom/components/nsCategoryCache.cpp
@@ -1,23 +1,26 @@
 /* -*- 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 "nsIObserverService.h"
 #include "mozilla/Services.h"
+#include "mozilla/SimpleEnumerator.h"
 #include "nsISupportsPrimitives.h"
 #include "nsIStringEnumerator.h"
 
 #include "nsXPCOMCID.h"
 
 #include "nsCategoryCache.h"
 
+using mozilla::SimpleEnumerator;
+
 nsCategoryObserver::nsCategoryObserver(const nsACString& aCategory)
   : mCategory(aCategory)
   , mCallback(nullptr)
   , mClosure(nullptr)
   , mObserversRemoved(false)
 {
   MOZ_ASSERT(NS_IsMainThread());
   // First, enumerate the currently existing entries
@@ -29,31 +32,25 @@ nsCategoryObserver::nsCategoryObserver(c
 
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   nsresult rv = catMan->EnumerateCategory(aCategory,
                                           getter_AddRefs(enumerator));
   if (NS_FAILED(rv)) {
     return;
   }
 
-  nsCOMPtr<nsIUTF8StringEnumerator> strings = do_QueryInterface(enumerator);
-  MOZ_ASSERT(strings);
-
-  bool more;
-  while (NS_SUCCEEDED(strings->HasMore(&more)) && more) {
-    nsAutoCString entryName;
-    strings->GetNext(entryName);
+  for (auto& categoryEntry : SimpleEnumerator<nsICategoryEntry>(enumerator)) {
+    nsAutoCString entryValue;
+    categoryEntry->GetValue(entryValue);
 
-    nsCString entryValue;
-    rv = catMan->GetCategoryEntry(aCategory, entryName, entryValue);
-    if (NS_SUCCEEDED(rv)) {
-      nsCOMPtr<nsISupports> service = do_GetService(entryValue.get());
-      if (service) {
-        mHash.Put(entryName, service);
-      }
+    if (nsCOMPtr<nsISupports> service = do_GetService(entryValue.get())) {
+      nsAutoCString entryName;
+      categoryEntry->GetEntry(entryName);
+
+      mHash.Put(entryName, service);
     }
   }
 
   // Now, listen for changes
   nsCOMPtr<nsIObserverService> serv =
     mozilla::services::GetObserverService();
   if (serv) {
     serv->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
--- a/xpcom/components/nsCategoryManager.cpp
+++ b/xpcom/components/nsCategoryManager.cpp
@@ -4,16 +4,17 @@
  * 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 "nsICategoryManager.h"
 #include "nsCategoryManager.h"
 
 #include "prio.h"
 #include "prlock.h"
+#include "nsArrayEnumerator.h"
 #include "nsCOMPtr.h"
 #include "nsTHashtable.h"
 #include "nsClassHashtable.h"
 #include "nsIFactory.h"
 #include "nsIStringEnumerator.h"
 #include "nsSupportsPrimitives.h"
 #include "nsComponentManagerUtils.h"
 #include "nsServiceManagerUtils.h"
@@ -22,16 +23,17 @@
 #include "nsReadableUtils.h"
 #include "nsCRT.h"
 #include "nsQuickSort.h"
 #include "nsEnumeratorUtils.h"
 #include "nsThreadUtils.h"
 #include "mozilla/ArenaAllocatorExtensions.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Services.h"
+#include "mozilla/SimpleEnumerator.h"
 
 #include "ManifestParser.h"
 #include "nsSimpleEnumerator.h"
 
 using namespace mozilla;
 class nsIComponentLoaderManager;
 
 /*
@@ -42,161 +44,217 @@ class nsIComponentLoaderManager;
   In other words, the CategoryDatabase is a tree, whose root is a hashtable.
   Internal nodes (or Categories) are hashtables. Leaf nodes are strings.
 
   The leaf strings are allocated in an arena, because we assume they're not
   going to change much ;)
 */
 
 //
-// BaseStringEnumerator is subclassed by EntryEnumerator and
-// CategoryEnumerator
+// CategoryEnumerator class
 //
-class BaseStringEnumerator
+
+class CategoryEnumerator
   : public nsSimpleEnumerator
   , private nsIUTF8StringEnumerator
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSISIMPLEENUMERATOR
   NS_DECL_NSIUTF8STRINGENUMERATOR
 
   const nsID& DefaultInterface() override
   {
     return NS_GET_IID(nsISupportsCString);
   }
 
+  static CategoryEnumerator* Create(nsClassHashtable<nsDepCharHashKey,
+                                                     CategoryNode>& aTable);
+
 protected:
-  // Callback function for NS_QuickSort to sort mArray
-  static int SortCallback(const void*, const void*, void*);
-
-  BaseStringEnumerator()
+  CategoryEnumerator()
     : mArray(nullptr)
     , mCount(0)
     , mSimpleCurItem(0)
     , mStringCurItem(0)
   {
   }
 
-  // A virtual destructor is needed here because subclasses of
-  // BaseStringEnumerator do not implement their own Release() method.
-
-  virtual ~BaseStringEnumerator()
+  ~CategoryEnumerator() override
   {
     delete [] mArray;
   }
 
-  void Sort();
-
   const char** mArray;
   uint32_t mCount;
   uint32_t mSimpleCurItem;
   uint32_t mStringCurItem;
 };
 
-NS_IMPL_ISUPPORTS_INHERITED(BaseStringEnumerator, nsSimpleEnumerator,
+NS_IMPL_ISUPPORTS_INHERITED(CategoryEnumerator, nsSimpleEnumerator,
                             nsIUTF8StringEnumerator)
 
 NS_IMETHODIMP
-BaseStringEnumerator::HasMoreElements(bool* aResult)
+CategoryEnumerator::HasMoreElements(bool* aResult)
 {
   *aResult = (mSimpleCurItem < mCount);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-BaseStringEnumerator::GetNext(nsISupports** aResult)
+CategoryEnumerator::GetNext(nsISupports** aResult)
 {
   if (mSimpleCurItem >= mCount) {
     return NS_ERROR_FAILURE;
   }
 
   auto* str = new nsSupportsDependentCString(mArray[mSimpleCurItem++]);
   if (!str) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   *aResult = str;
   NS_ADDREF(*aResult);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-BaseStringEnumerator::HasMore(bool* aResult)
+CategoryEnumerator::HasMore(bool* aResult)
 {
   *aResult = (mStringCurItem < mCount);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-BaseStringEnumerator::GetNext(nsACString& aResult)
+CategoryEnumerator::GetNext(nsACString& aResult)
 {
   if (mStringCurItem >= mCount) {
     return NS_ERROR_FAILURE;
   }
 
   aResult = nsDependentCString(mArray[mStringCurItem++]);
   return NS_OK;
 }
 
-int
-BaseStringEnumerator::SortCallback(const void* aE1, const void* aE2,
-                                   void* /*unused*/)
-{
-  char const* const* s1 = reinterpret_cast<char const* const*>(aE1);
-  char const* const* s2 = reinterpret_cast<char const* const*>(aE2);
-
-  return strcmp(*s1, *s2);
-}
-
-void
-BaseStringEnumerator::Sort()
+CategoryEnumerator*
+CategoryEnumerator::Create(nsClassHashtable<nsDepCharHashKey, CategoryNode>&
+                           aTable)
 {
-  NS_QuickSort(mArray, mCount, sizeof(mArray[0]), SortCallback, nullptr);
-}
-
-//
-// EntryEnumerator is the wrapper that allows nsICategoryManager::EnumerateCategory
-//
-class EntryEnumerator
-  : public BaseStringEnumerator
-{
-public:
-  static EntryEnumerator* Create(nsTHashtable<CategoryLeaf>& aTable);
-};
-
-
-EntryEnumerator*
-EntryEnumerator::Create(nsTHashtable<CategoryLeaf>& aTable)
-{
-  auto* enumObj = new EntryEnumerator();
+  auto* enumObj = new CategoryEnumerator();
   if (!enumObj) {
     return nullptr;
   }
 
-  enumObj->mArray = new char const* [aTable.Count()];
+  enumObj->mArray = new const char* [aTable.Count()];
   if (!enumObj->mArray) {
     delete enumObj;
     return nullptr;
   }
 
   for (auto iter = aTable.Iter(); !iter.Done(); iter.Next()) {
-    CategoryLeaf* leaf = iter.Get();
-    if (leaf->value) {
-      enumObj->mArray[enumObj->mCount++] = leaf->GetKey();
+    // if a category has no entries, we pretend it doesn't exist
+    CategoryNode* aNode = iter.UserData();
+    if (aNode->Count()) {
+      enumObj->mArray[enumObj->mCount++] = iter.Key();
     }
   }
 
-  enumObj->Sort();
-
   return enumObj;
 }
 
+class CategoryEntry final : public nsICategoryEntry
+{
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSICATEGORYENTRY
+  NS_DECL_NSISUPPORTSCSTRING
+  NS_DECL_NSISUPPORTSPRIMITIVE
+
+  CategoryEntry(const char* aKey, const char* aValue)
+    : mKey(aKey)
+    , mValue(aValue)
+  {}
+
+  const char* Key() const { return mKey; }
+
+  static CategoryEntry* Cast(nsICategoryEntry* aEntry)
+  {
+    return static_cast<CategoryEntry*>(aEntry);
+  }
+
+private:
+  ~CategoryEntry() = default;
+
+  const char* mKey;
+  const char* mValue;
+};
+
+NS_IMPL_ISUPPORTS(CategoryEntry, nsICategoryEntry, nsISupportsCString)
+
+nsresult
+CategoryEntry::ToString(char** aResult)
+{
+  *aResult = moz_xstrdup(mKey);
+  return NS_OK;
+}
+
+nsresult
+CategoryEntry::GetType(uint16_t* aType)
+{
+  *aType = TYPE_CSTRING;
+  return NS_OK;
+}
+
+nsresult
+CategoryEntry::GetData(nsACString& aData)
+{
+  aData = mKey;
+  return NS_OK;
+}
+
+nsresult
+CategoryEntry::SetData(const nsACString& aData)
+{
+  return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+nsresult
+CategoryEntry::GetEntry(nsACString& aEntry)
+{
+  aEntry = mKey;
+  return NS_OK;
+}
+
+nsresult
+CategoryEntry::GetValue(nsACString& aValue)
+{
+  aValue = mValue;
+  return NS_OK;
+}
+
+static nsresult
+CreateEntryEnumerator(nsTHashtable<CategoryLeaf>& aTable,
+                      nsISimpleEnumerator** aResult)
+{
+  nsCOMArray<nsICategoryEntry> entries(aTable.Count());
+
+  for (auto iter = aTable.Iter(); !iter.Done(); iter.Next()) {
+    CategoryLeaf* leaf = iter.Get();
+    if (leaf->value) {
+      entries.AppendElement(new CategoryEntry(leaf->GetKey(), leaf->value));
+    }
+  }
+
+  entries.Sort([](nsICategoryEntry* aA, nsICategoryEntry* aB, void*) {
+    return strcmp(CategoryEntry::Cast(aA)->Key(), CategoryEntry::Cast(aB)->Key());
+  }, nullptr);
+
+  return NS_NewArrayEnumerator(aResult, entries, NS_GET_IID(nsICategoryEntry));
+}
 
 //
 // CategoryNode implementations
 //
 
 CategoryNode*
 CategoryNode::Create(CategoryAllocator* aArena)
 {
@@ -279,80 +337,29 @@ CategoryNode::DeleteLeaf(const nsACStrin
 
   // we can just remove the entire hash entry without introspection
   mTable.RemoveEntry(PromiseFlatCString(aEntryName).get());
 }
 
 nsresult
 CategoryNode::Enumerate(nsISimpleEnumerator** aResult)
 {
-  if (NS_WARN_IF(!aResult)) {
-    return NS_ERROR_INVALID_ARG;
-  }
-
   MutexAutoLock lock(mLock);
-  EntryEnumerator* enumObj = EntryEnumerator::Create(mTable);
-
-  if (!enumObj) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  *aResult = enumObj;
-  NS_ADDREF(*aResult);
-  return NS_OK;
+  return CreateEntryEnumerator(mTable, aResult);
 }
 
 size_t
 CategoryNode::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf)
 {
   // We don't measure the strings pointed to by the entries because the
   // pointers are non-owning.
   return mTable.ShallowSizeOfExcludingThis(aMallocSizeOf);
 }
 
 //
-// CategoryEnumerator class
-//
-
-class CategoryEnumerator
-  : public BaseStringEnumerator
-{
-public:
-  static CategoryEnumerator* Create(nsClassHashtable<nsDepCharHashKey,
-                                                     CategoryNode>& aTable);
-};
-
-CategoryEnumerator*
-CategoryEnumerator::Create(nsClassHashtable<nsDepCharHashKey, CategoryNode>&
-                           aTable)
-{
-  auto* enumObj = new CategoryEnumerator();
-  if (!enumObj) {
-    return nullptr;
-  }
-
-  enumObj->mArray = new const char* [aTable.Count()];
-  if (!enumObj->mArray) {
-    delete enumObj;
-    return nullptr;
-  }
-
-  for (auto iter = aTable.Iter(); !iter.Done(); iter.Next()) {
-    // if a category has no entries, we pretend it doesn't exist
-    CategoryNode* aNode = iter.UserData();
-    if (aNode->Count()) {
-      enumObj->mArray[enumObj->mCount++] = iter.Key();
-    }
-  }
-
-  return enumObj;
-}
-
-
-//
 // nsCategoryManager implementations
 //
 
 NS_IMPL_QUERY_INTERFACE(nsCategoryManager, nsICategoryManager, nsIMemoryReporter)
 
 NS_IMETHODIMP_(MozExternalRefCountType)
 nsCategoryManager::AddRef()
 {
@@ -740,36 +747,23 @@ NS_CreateServicesFromCategory(const char
 
   nsCOMPtr<nsISimpleEnumerator> enumerator;
   rv = categoryManager->EnumerateCategory(category,
                                           getter_AddRefs(enumerator));
   if (NS_FAILED(rv)) {
     return;
   }
 
-  nsCOMPtr<nsIUTF8StringEnumerator> senumerator =
-    do_QueryInterface(enumerator);
-  if (!senumerator) {
-    NS_WARNING("Category enumerator doesn't support nsIUTF8StringEnumerator.");
-    return;
-  }
-
-  bool hasMore;
-  while (NS_SUCCEEDED(senumerator->HasMore(&hasMore)) && hasMore) {
+  for (auto& categoryEntry : SimpleEnumerator<nsICategoryEntry>(enumerator)) {
     // From here on just skip any error we get.
     nsAutoCString entryString;
-    if (NS_FAILED(senumerator->GetNext(entryString))) {
-      continue;
-    }
+    categoryEntry->GetEntry(entryString);
 
-    nsCString contractID;
-    rv = categoryManager->GetCategoryEntry(category, entryString, contractID);
-    if (NS_FAILED(rv)) {
-      continue;
-    }
+    nsAutoCString contractID;
+    categoryEntry->GetValue(contractID);
 
     nsCOMPtr<nsISupports> instance = do_GetService(contractID.get());
     if (!instance) {
       LogMessage("While creating services from category '%s', could not create service for entry '%s', contract ID '%s'",
                  aCategory, entryString.get(), contractID.get());
       continue;
     }
 
--- a/xpcom/components/nsICategoryManager.idl
+++ b/xpcom/components/nsICategoryManager.idl
@@ -1,25 +1,34 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /* 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 "nsISupports.idl"
+#include "nsISupportsPrimitives.idl"
 
 interface nsISimpleEnumerator;
 
 %{C++
 #include "nsString.h"
 %}
 
 /*
  * nsICategoryManager
  */
 
+[scriptable, uuid(de021d54-57a3-4025-ae63-4c8eedbe74c0)]
+interface nsICategoryEntry : nsISupportsCString
+{
+  readonly attribute ACString entry;
+
+  readonly attribute ACString value;
+};
+
 [builtinclass, scriptable, uuid(3275b2cd-af6d-429a-80d7-f0c5120342ac)]
 interface nsICategoryManager : nsISupports
 {
     /**
      * Get the value for the given category's entry.
      * @param aCategory The name of the category ("protocol")
      * @param aEntry The entry you're looking for ("http")
      * @return The value.
@@ -53,20 +62,21 @@ interface nsICategoryManager : nsISuppor
      * @param aCategory The category to be deleted.
      */
     void deleteCategory(in ACString aCategory);
 
     /**
      * Enumerate the entries in a category.
      * @param aCategory The category to be enumerated.
      * @return a simple enumerator, each result QIs to
-     *         nsISupportsCString.
+     *         nsICategoryEntry.
      */
     nsISimpleEnumerator enumerateCategory(in ACString aCategory);
 
+
     /**
      * Enumerate all existing categories
      * @param aCategory The category to be enumerated.
      * @return a simple enumerator, each result QIs to
      *         nsISupportsCString.
      */
     nsISimpleEnumerator enumerateCategories();