Bug 1344629 - Part 9: Bonus fix: Remove heap allocation in nsChromeTreeOwner. r=dbaron
authorDavid Major <dmajor@mozilla.com>
Tue, 14 Mar 2017 15:26:37 +1300
changeset 397935 a06239255301c9f6e1200d82d7bfe0459be37f6a
parent 397934 c3873cde0276917f09ed5872aeeb712067a7a481
child 397936 6066c5ca8f705fe06004a1123678dd427905fb2b
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1344629
milestone55.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 1344629 - Part 9: Bonus fix: Remove heap allocation in nsChromeTreeOwner. r=dbaron Now that nsLiteralStrings are "free", they can just be static globals. MozReview-Commit-ID: 4d4ZObxiHF8
xpfe/appshell/nsAppShellFactory.cpp
xpfe/appshell/nsChromeTreeOwner.cpp
xpfe/appshell/nsChromeTreeOwner.h
--- a/xpfe/appshell/nsAppShellFactory.cpp
+++ b/xpfe/appshell/nsAppShellFactory.cpp
@@ -26,31 +26,15 @@ static const mozilla::Module::CIDEntry k
 };
 
 static const mozilla::Module::ContractIDEntry kAppShellContracts[] = {
   { NS_APPSHELLSERVICE_CONTRACTID, &kNS_APPSHELLSERVICE_CID },
   { NS_WINDOWMEDIATOR_CONTRACTID, &kNS_WINDOWMEDIATOR_CID },
   { nullptr }
 };
 
-static nsresult
-nsAppShellModuleConstructor()
-{
-  return nsChromeTreeOwner::InitGlobals();
-}
-
-static void
-nsAppShellModuleDestructor()
-{
-  nsChromeTreeOwner::FreeGlobals();
-}
-
 static const mozilla::Module kAppShellModule = {
   mozilla::Module::kVersion,
   kAppShellCIDs,
   kAppShellContracts,
-  nullptr,
-  nullptr,
-  nsAppShellModuleConstructor,
-  nsAppShellModuleDestructor
 };
 
 NSMODULE_DEFN(appshell) = &kAppShellModule;
--- a/xpfe/appshell/nsChromeTreeOwner.cpp
+++ b/xpfe/appshell/nsChromeTreeOwner.cpp
@@ -30,53 +30,23 @@
 #include "mozilla/dom/Element.h"
 
 using namespace mozilla;
 
 //*****************************************************************************
 // nsChromeTreeOwner string literals
 //*****************************************************************************
 
-struct nsChromeTreeOwnerLiterals
-{
-  const nsLiteralString kPersist;
-  const nsLiteralString kScreenX;
-  const nsLiteralString kScreenY;
-  const nsLiteralString kWidth;
-  const nsLiteralString kHeight;
-  const nsLiteralString kSizemode;
-  const nsLiteralString kSpace;
-
-  nsChromeTreeOwnerLiterals()
-    : NS_LITERAL_STRING_INIT(kPersist,"persist")
-    , NS_LITERAL_STRING_INIT(kScreenX,"screenX")
-    , NS_LITERAL_STRING_INIT(kScreenY,"screenY")
-    , NS_LITERAL_STRING_INIT(kWidth,"width")
-    , NS_LITERAL_STRING_INIT(kHeight,"height")
-    , NS_LITERAL_STRING_INIT(kSizemode,"sizemode")
-    , NS_LITERAL_STRING_INIT(kSpace," ")
-  {}
-};
-
-static nsChromeTreeOwnerLiterals *gLiterals;
-
-nsresult
-nsChromeTreeOwner::InitGlobals()
-{
-  NS_ASSERTION(gLiterals == nullptr, "already initialized");
-  gLiterals = new nsChromeTreeOwnerLiterals();
-  return NS_OK;
-}
-
-void
-nsChromeTreeOwner::FreeGlobals()
-{
-  delete gLiterals;
-  gLiterals = nullptr;
-}
+const nsLiteralString kPersist(u"persist");
+const nsLiteralString kScreenX(u"screenX");
+const nsLiteralString kScreenY(u"screenY");
+const nsLiteralString kWidth(u"width");
+const nsLiteralString kHeight(u"height");
+const nsLiteralString kSizemode(u"sizemode");
+const nsLiteralString kSpace(u" ");
 
 //*****************************************************************************
 //***    nsChromeTreeOwner: Object Management
 //*****************************************************************************
 
 nsChromeTreeOwner::nsChromeTreeOwner() : mXULWindow(nullptr)
 {
 }
@@ -224,67 +194,67 @@ nsChromeTreeOwner::SetPersistence(bool a
                                   bool aPersistSizeMode)
 {
   NS_ENSURE_STATE(mXULWindow);
   nsCOMPtr<dom::Element> docShellElement = mXULWindow->GetWindowDOMElement();
   if (!docShellElement)
     return NS_ERROR_FAILURE;
 
   nsAutoString persistString;
-  docShellElement->GetAttribute(gLiterals->kPersist, persistString);
+  docShellElement->GetAttribute(kPersist, persistString);
 
   bool saveString = false;
   int32_t index;
 
 #define FIND_PERSIST_STRING(aString, aCond)            \
   index = persistString.Find(aString);                 \
   if (!aCond && index > kNotFound) {                   \
     persistString.Cut(index, aString.Length());        \
     saveString = true;                              \
   } else if (aCond && index == kNotFound) {            \
-    persistString.Append(gLiterals->kSpace + aString); \
+    persistString.Append(kSpace + aString); \
     saveString = true;                              \
   }
-  FIND_PERSIST_STRING(gLiterals->kScreenX,  aPersistPosition);
-  FIND_PERSIST_STRING(gLiterals->kScreenY,  aPersistPosition);
-  FIND_PERSIST_STRING(gLiterals->kWidth,    aPersistSize);
-  FIND_PERSIST_STRING(gLiterals->kHeight,   aPersistSize);
-  FIND_PERSIST_STRING(gLiterals->kSizemode, aPersistSizeMode);
+  FIND_PERSIST_STRING(kScreenX,  aPersistPosition);
+  FIND_PERSIST_STRING(kScreenY,  aPersistPosition);
+  FIND_PERSIST_STRING(kWidth,    aPersistSize);
+  FIND_PERSIST_STRING(kHeight,   aPersistSize);
+  FIND_PERSIST_STRING(kSizemode, aPersistSizeMode);
 
   ErrorResult rv;
   if (saveString) {
-    docShellElement->SetAttribute(gLiterals->kPersist, persistString, rv);
+    docShellElement->SetAttribute(kPersist, persistString, rv);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsChromeTreeOwner::GetPersistence(bool* aPersistPosition,
                                   bool* aPersistSize,
                                   bool* aPersistSizeMode)
 {
   NS_ENSURE_STATE(mXULWindow);
   nsCOMPtr<dom::Element> docShellElement = mXULWindow->GetWindowDOMElement();
   if (!docShellElement)
     return NS_ERROR_FAILURE;
 
   nsAutoString persistString;
-  docShellElement->GetAttribute(gLiterals->kPersist, persistString);
+  docShellElement->GetAttribute(kPersist, persistString);
 
   // data structure doesn't quite match the question, but it's close enough
   // for what we want (since this method is never actually called...)
   if (aPersistPosition)
-    *aPersistPosition = persistString.Find(gLiterals->kScreenX) > kNotFound ||
-                        persistString.Find(gLiterals->kScreenY) > kNotFound;
+    *aPersistPosition = persistString.Find(kScreenX) > kNotFound ||
+                        persistString.Find(kScreenY) > kNotFound;
   if (aPersistSize)
-    *aPersistSize = persistString.Find(gLiterals->kWidth) > kNotFound ||
-                    persistString.Find(gLiterals->kHeight) > kNotFound;
+    *aPersistSize = persistString.Find(kWidth) > kNotFound ||
+                    persistString.Find(kHeight) > kNotFound;
   if (aPersistSizeMode)
-    *aPersistSizeMode = persistString.Find(gLiterals->kSizemode) > kNotFound;
+    *aPersistSizeMode = persistString.Find(kSizemode) > kNotFound;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsChromeTreeOwner::GetTabCount(uint32_t* aResult)
 {
   if (mXULWindow) {
--- a/xpfe/appshell/nsChromeTreeOwner.h
+++ b/xpfe/appshell/nsChromeTreeOwner.h
@@ -31,19 +31,16 @@ friend class nsXULWindow;
 public:
    NS_DECL_ISUPPORTS
 
    NS_DECL_NSIINTERFACEREQUESTOR
    NS_DECL_NSIBASEWINDOW
    NS_DECL_NSIDOCSHELLTREEOWNER
    NS_DECL_NSIWEBPROGRESSLISTENER
 
-   static nsresult InitGlobals();
-   static void     FreeGlobals();
-
 protected:
    nsChromeTreeOwner();
    virtual ~nsChromeTreeOwner();
 
    void XULWindow(nsXULWindow* aXULWindow);
    nsXULWindow* XULWindow();
 
 protected: