Bug 1413400 (part 2) - Make Preferences::sPreferences a StaticRefPtr. r=froydnj.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 01 Nov 2017 13:55:28 +1100
changeset 389917 962343e8bdf0f90916487fcf81908589d70ff51f
parent 389916 352bbd41f1fa04267235faeec42dd463bcf8118a
child 389918 e255f93c08085eb8a4034d320c6af48827619908
push id96950
push usernnethercote@mozilla.com
push dateThu, 02 Nov 2017 23:20:36 +0000
treeherdermozilla-inbound@962343e8bdf0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1413400
milestone58.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 1413400 (part 2) - Make Preferences::sPreferences a StaticRefPtr. r=froydnj. The notable part of this change is Shutdown(). I've made it just null out sPreferences, contrary to the old comment, which was strange for a couple of reasons: - ~Preferences() used to null out sPreference, which is backwards compared to how this sort of thing normally works. - In both the before and after cases, as far as I can tell, Preferences::Shutdown() is called but ~Preferences() is never called; something keeps the singleton Preferences instance alive until process termination. MozReview-Commit-ID: Ab0ui31rVcI
modules/libpref/Preferences.cpp
modules/libpref/Preferences.h
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -3163,17 +3163,17 @@ Preferences::HandleDirty()
     sPreferences->mDirty = true;
 
     if (sPreferences->mCurrentFile && sPreferences->AllowOffMainThreadSave() &&
         !sPreferences->mSavePending) {
       sPreferences->mSavePending = true;
       static const int PREF_DELAY_MS = 500;
       NS_DelayedDispatchToCurrentThread(
         mozilla::NewRunnableMethod("Preferences::SavePrefFileAsynchronous",
-                                   sPreferences,
+                                   sPreferences.get(),
                                    &Preferences::SavePrefFileAsynchronous),
         PREF_DELAY_MS);
     }
   }
 }
 
 static nsresult
 openPrefFile(nsIFile* aFile);
@@ -3206,17 +3206,17 @@ static const char kPrefFileHeader[] =
   " * To make a manual change to preferences, you can visit the URL "
   "about:config"
   NS_LINEBREAK
   " */"
   NS_LINEBREAK
   NS_LINEBREAK;
 // clang-format on
 
-Preferences* Preferences::sPreferences = nullptr;
+StaticRefPtr<Preferences> Preferences::sPreferences;
 bool Preferences::sShutdown = false;
 
 // This globally enables or disables OMT pref writing, both sync and async.
 static int32_t sAllowOMTPrefWrite = -1;
 
 class ValueObserverHashKey : public PLDHashEntryHdr
 {
 public:
@@ -3703,22 +3703,20 @@ Preferences::GetInstanceForService()
   }
 
   if (sShutdown) {
     gCacheDataDesc = "shutting down in GetInstanceForService()";
     return nullptr;
   }
 
   sPreferences = new Preferences();
-  NS_ADDREF(sPreferences);
-
   Result<Ok, const char*> res = sPreferences->Init();
   if (res.isErr()) {
+    sPreferences = nullptr;
     gCacheDataDesc = res.unwrapErr();
-    NS_RELEASE(sPreferences);
     return nullptr;
   }
 
   gCacheData = new nsTArray<nsAutoPtr<CacheData>>();
   gCacheDataDesc = "set by GetInstanceForService()";
 
   gObserverTable = new nsRefPtrHashtable<ValueObserverHashKey, ValueObserver>();
 
@@ -3753,23 +3751,17 @@ Preferences::InitStaticMembers()
   return sPreferences != nullptr;
 }
 
 /* static */ void
 Preferences::Shutdown()
 {
   if (!sShutdown) {
     sShutdown = true; // Don't create the singleton instance after here.
-
-    // Don't set sPreferences to nullptr here. The instance may be grabbed by
-    // other modules. The utility methods of Preferences should be available
-    // until the singleton instance actually released.
-    if (sPreferences) {
-      sPreferences->Release();
-    }
+    sPreferences = nullptr;
   }
 }
 
 //-----------------------------------------------------------------------------
 
 //
 // Constructor/Destructor
 //
@@ -3777,26 +3769,24 @@ Preferences::Shutdown()
 Preferences::Preferences()
   : mRootBranch(new nsPrefBranch("", false))
   , mDefaultRootBranch(new nsPrefBranch("", true))
 {
 }
 
 Preferences::~Preferences()
 {
-  NS_ASSERTION(sPreferences == this, "Isn't this the singleton instance?");
+  MOZ_ASSERT(!sPreferences);
 
   delete gObserverTable;
   gObserverTable = nullptr;
 
   delete gCacheData;
   gCacheData = nullptr;
 
-  sPreferences = nullptr;
-
   PREF_Cleanup();
 }
 
 //
 // nsISupports Implementation
 //
 
 NS_IMPL_ADDREF(Preferences)
--- a/modules/libpref/Preferences.h
+++ b/modules/libpref/Preferences.h
@@ -8,16 +8,17 @@
 #define mozilla_Preferences_h
 
 #ifndef MOZILLA_INTERNAL_API
 #error "This header is only usable from within libxul (MOZILLA_INTERNAL_API)."
 #endif
 
 #include "mozilla/Atomics.h"
 #include "mozilla/MemoryReporting.h"
+#include "mozilla/StaticPtr.h"
 #include "nsCOMPtr.h"
 #include "nsIObserver.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
 #include "nsTArray.h"
 #include "nsWeakReference.h"
 
 class nsIFile;
@@ -401,17 +402,17 @@ private:
   bool mProfileShutdown = false;
   // We wait a bit after prefs are dirty before writing them. In this period,
   // mDirty and mSavePending will both be true.
   bool mSavePending = false;
 
   nsCOMPtr<nsIPrefBranch> mRootBranch;
   nsCOMPtr<nsIPrefBranch> mDefaultRootBranch;
 
-  static Preferences* sPreferences;
+  static StaticRefPtr<Preferences> sPreferences;
   static bool sShutdown;
 
   // Init static members. Returns true on success.
   static bool InitStaticMembers();
 };
 
 } // namespace mozilla