Bug 1494515 - Clean up static atom registration. r=froydnj
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 27 Sep 2018 16:13:41 +1000
changeset 438626 1a9268216cd7c52323f5bc95166f5f154acc36be
parent 438625 aab722576793adebb6b0e0b8b126ab7859ea709b
child 438627 cb1bd4097d6bf9ad61c6f4ff146ba193bf370193
push id34728
push userccoroiu@mozilla.com
push dateFri, 28 Sep 2018 04:33:26 +0000
treeherdermozilla-central@95301d805205 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1494515
milestone64.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 1494515 - Clean up static atom registration. r=froydnj Static atom registration is a bit of a mess. NS_InitAtomTable() calls nsGkAtoms::RegisterStaticAtoms(), which calls NS_RegisterStaticAtoms(); i.e. we go from nsAtomTable.cpp to nsGkAtoms.cpp and back. (And NS_RegisterStaticAtoms() is declared in a .cpp file, not a .h file!) This commit makes NS_InitAtomTable() a friend of nsGkAtoms, so NS_InitAtomTable can see nsGkAtoms's atoms array directly, thus removing the need for NS_RegisterAtomTable() and nsGkAtoms::RegisterStaticAtoms(). This commit also removes an out-of-date part of a comment from XPCOMInit.cpp.
xpcom/build/XPCOMInit.cpp
xpcom/ds/nsAtomTable.cpp
xpcom/ds/nsGkAtoms.cpp
xpcom/ds/nsGkAtoms.h
--- a/xpcom/build/XPCOMInit.cpp
+++ b/xpcom/build/XPCOMInit.cpp
@@ -695,19 +695,17 @@ NS_InitXPCOM2(nsIServiceManager** aResul
   // add any services listed in the "xpcom-directory-providers" category
   // to the directory service.
   nsDirectoryService::gService->RegisterCategoryProviders();
 
   // Init SharedThreadPool (which needs the service manager).
   SharedThreadPool::InitStatics();
 
   // Force layout to spin up so that nsContentUtils is available for cx stack
-  // munging.  Note that layout registers a number of static atoms, and also
-  // seals the static atom table, so NS_RegisterStaticAtom may not be called
-  // beyond this point.
+  // munging.
   nsCOMPtr<nsISupports> componentLoader =
     do_GetService("@mozilla.org/moz/jsloader;1");
 
   mozilla::ScriptPreloader::GetSingleton();
   mozilla::scache::StartupCache::GetSingleton();
   mozilla::AvailableMemoryTracker::Init();
 
   // Notify observers of xpcom autoregistration start
--- a/xpcom/ds/nsAtomTable.cpp
+++ b/xpcom/ds/nsAtomTable.cpp
@@ -590,26 +590,24 @@ nsAtom::Release()
 //----------------------------------------------------------------------
 
 // Have the static atoms been inserted into the table?
 static bool gStaticAtomsDone = false;
 
 void
 NS_InitAtomTable()
 {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!gAtomTable);
-  gAtomTable = new nsAtomTable();
 
-  // Bug 1340710 has caused us to use an empty atom at arbitrary times after
-  // startup. If we end up creating one before nsGkAtoms::_empty is registered,
-  // we get an assertion about transmuting a dynamic atom into a static atom.
-  // In order to avoid that, we register nsGkAtoms immediately after creating
-  // the atom table to guarantee that the empty string atom will always be
-  // static.
-  nsGkAtoms::RegisterStaticAtoms();
+  // We register static atoms immediately so they're available for use as early
+  // as possible.
+  gAtomTable = new nsAtomTable();
+  gAtomTable->RegisterStaticAtoms(nsGkAtoms::sAtoms, nsGkAtoms::sAtomsLen);
+  gStaticAtomsDone = true;
 }
 
 void
 NS_ShutdownAtomTable()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(gAtomTable);
 
@@ -674,25 +672,16 @@ nsAtomTable::RegisterStaticAtoms(const n
       nsAutoCString name;
       he->mAtom->ToUTF8String(name);
       MOZ_CRASH_UNSAFE_PRINTF("Atom for '%s' already exists", name.get());
     }
     he->mAtom = const_cast<nsStaticAtom*>(atom);
   }
 }
 
-void
-NS_RegisterStaticAtoms(const nsStaticAtom* aAtoms, size_t aAtomsLen)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(gAtomTable);
-  gAtomTable->RegisterStaticAtoms(aAtoms, aAtomsLen);
-  gStaticAtomsDone = true;
-}
-
 already_AddRefed<nsAtom>
 NS_Atomize(const char* aUTF8String)
 {
   MOZ_ASSERT(gAtomTable);
   return gAtomTable->Atomize(nsDependentCString(aUTF8String));
 }
 
 already_AddRefed<nsAtom>
--- a/xpcom/ds/nsGkAtoms.cpp
+++ b/xpcom/ds/nsGkAtoms.cpp
@@ -1,20 +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 "nsGkAtoms.h"
 
-// Register an array of static atoms with the atom table.
-void
-NS_RegisterStaticAtoms(const nsStaticAtom* aAtoms, size_t aAtomsLen);
-
 namespace mozilla {
 namespace detail {
 
 // Because this is `constexpr` it ends up in read-only memory where it can be
 // shared between processes.
 extern constexpr GkAtoms gGkAtoms = {
   // The initialization of each atom's string.
   //
@@ -94,13 +90,8 @@ const nsStaticAtom* const nsGkAtoms::sAt
   type_* nsGkAtoms::name_ =                                                    \
     const_cast<type_*>(                                                        \
       static_cast<const type_*>(                                               \
         &mozilla::detail::gGkAtoms.mAtoms[                                     \
           static_cast<size_t>(mozilla::detail::GkAtoms::Atoms::name_)]));
 #include "nsGkAtomList.h"
 #undef GK_ATOM
 
-void nsGkAtoms::RegisterStaticAtoms()
-{
-  NS_RegisterStaticAtoms(sAtoms, sAtomsLen);
-}
-
--- a/xpcom/ds/nsGkAtoms.h
+++ b/xpcom/ds/nsGkAtoms.h
@@ -113,27 +113,27 @@ struct GkAtoms
 
 } // namespace detail
 } // namespace mozilla
 
 // This class holds the pointers to the individual atoms.
 class nsGkAtoms
 {
 private:
+  friend void NS_InitAtomTable();
+
   // This is a useful handle to the array of atoms, used below and also
   // possibly by Rust code.
   static const nsStaticAtom* const sAtoms;
 
   // The number of atoms, used below.
   static constexpr size_t sAtomsLen =
     static_cast<size_t>(mozilla::detail::GkAtoms::Atoms::AtomsCount);
 
 public:
-  static void RegisterStaticAtoms();
-
   static nsStaticAtom* GetAtomByIndex(size_t aIndex)
   {
     MOZ_ASSERT(aIndex < sAtomsLen);
     return const_cast<nsStaticAtom*>(&sAtoms[aIndex]);
   }
 
   // The declaration of the pointer to each static atom.
   //