Bug 789945: Part 4. Explicitly switch C++ SavePrefFile callers to main thread or off main thread versions of the function. Enable OMT pref save on by default, at which point, the sync saves are more expensive than before. r?bsmedberg draft
authorMilan Sreckovic <milan@mozilla.com>
Mon, 29 May 2017 11:55:01 -0400
changeset 586062 809eb7e9de3cb7a8fc91c32f8a73586e548152df
parent 586061 bb30a3777c7de4fe45601543a3b6bc14e5d955d2
child 630871 adc7a0c4d1c0ffe9c74c9b728150a1342996adb4
push id61275
push userbmo:milan@mozilla.com
push dateMon, 29 May 2017 16:09:16 +0000
reviewersbsmedberg
bugs789945
milestone55.0a1
Bug 789945: Part 4. Explicitly switch C++ SavePrefFile callers to main thread or off main thread versions of the function. Enable OMT pref save on by default, at which point, the sync saves are more expensive than before. r?bsmedberg MozReview-Commit-ID: 2ROXs8zuaoN
gfx/src/DriverCrashGuard.cpp
layout/tools/layout-debug/src/nsLayoutDebuggingTools.cpp
modules/libpref/Preferences.cpp
modules/libpref/init/all.js
toolkit/components/places/Database.cpp
toolkit/components/startup/nsAppStartup.cpp
toolkit/xre/nsXREDirProvider.cpp
widget/android/PrefsHelper.h
widget/android/nsAppShell.cpp
widget/nsIdleService.cpp
--- a/gfx/src/DriverCrashGuard.cpp
+++ b/gfx/src/DriverCrashGuard.cpp
@@ -383,17 +383,17 @@ DriverCrashGuard::SetStatus(DriverInitSt
 }
 
 void
 DriverCrashGuard::FlushPreferences()
 {
   MOZ_ASSERT(XRE_IsParentProcess());
 
   if (nsIPrefService* prefService = Preferences::GetService()) {
-    prefService->SavePrefFile(nullptr);
+    static_cast<Preferences *>(prefService)->SavePrefFileOnMainThread();
   }
 }
 
 void
 DriverCrashGuard::ForEachActiveCrashGuard(const CrashGuardCallback& aCallback)
 {
   if (!AreCrashGuardsEnabled()) {
     // Even if guards look active (via prefs), they can be ignored if globally
--- a/layout/tools/layout-debug/src/nsLayoutDebuggingTools.cpp
+++ b/layout/tools/layout-debug/src/nsLayoutDebuggingTools.cpp
@@ -525,14 +525,14 @@ nsLayoutDebuggingTools::SetBoolPrefAndRe
                                               bool aNewVal)
 {
     NS_ENSURE_TRUE(mDocShell, NS_ERROR_NOT_INITIALIZED);
 
     nsIPrefService* prefService = Preferences::GetService();
     NS_ENSURE_TRUE(prefService && aPrefName, NS_OK);
 
     Preferences::SetBool(aPrefName, aNewVal);
-    prefService->SavePrefFile(nullptr);
+    static_cast<Preferences *>(prefService)->SavePrefFileOffMainThread();
 
     ForceRefresh();
 
     return NS_OK;
 }
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -6,16 +6,17 @@
 
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/dom/PContent.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/HashFunctions.h"
 #include "mozilla/ServoStyleSet.h"
+#include "mozilla/StaticMutex.h"
 #include "mozilla/SystemGroup.h"
 #include "mozilla/UniquePtrExtensions.h"
 
 #include "nsXULAppAPI.h"
 
 #include "mozilla/Preferences.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsDataHashtable.h"
@@ -474,17 +475,17 @@ public:
     MOZ_RELEASE_ASSERT(sp);
     MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv));
     return sp.forget();
   }
 
 protected:
   virtual ~PWRunnable() {}
 };
-NS_IMPL_ISUPPORTS(PWRunnable, nsIAsyncShutdownBlocker)
+NS_IMPL_ISUPPORTS(PWRunnable, nsIRunnable, nsIAsyncShutdownBlocker)
 
 NS_IMETHODIMP
 PWRunnable::BlockShutdown(nsIAsyncShutdownClient*)
 {
     return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -908,42 +909,54 @@ Preferences::Observe(nsISupports *aSubje
 {
   if (MOZ_UNLIKELY(!XRE_IsParentProcess())) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsresult rv = NS_OK;
 
   if (!nsCRT::strcmp(aTopic, "profile-before-change")) {
-    rv = SavePrefFile(nullptr);
+    // Make sure we save any late changes to prefs
+    SavePrefFileOffMainThread();
   } else if (!strcmp(aTopic, "load-extension-defaults")) {
     pref_LoadPrefsInDirList(NS_EXT_PREFS_DEFAULTS_DIR_LIST);
   } else if (!nsCRT::strcmp(aTopic, "reload-default-prefs")) {
     // Reload the default prefs from file.
+    MOZ_RELEASE_ASSERT(NS_IsMainThread());
+    if (AllowOffMainThreadSave()) {
+      PWHelper::FlushPendingWrites();
+    }
     pref_InitInitialObjects();
   } else if (!nsCRT::strcmp(aTopic, "suspend_process_notification")) {
     // Our process is being suspended. The OS may wake our process later,
     // or it may kill the process. In case our process is going to be killed
     // from the suspended state, we save preferences before suspending.
-    rv = SavePrefFile(nullptr);
+    // Make sure we do a main thread save.
+    SavePrefFileOnMainThread();
   }
   return rv;
 }
 
 
 NS_IMETHODIMP
 Preferences::ReadUserPrefs(nsIFile *aFile)
 {
   if (MOZ_UNLIKELY(!XRE_IsParentProcess())) {
     NS_ERROR("must load prefs from parent process");
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsresult rv = NS_OK;
   if (nullptr == aFile) {
+    // We should not be re-reading the user preferences, but if we
+    // are going to try, make sure we are up to date:
+    if (AllowOffMainThreadSave()) {
+      PWHelper::FlushPendingWrites();
+    }
+
     rv = UseDefaultPrefFile();
     // A user pref file is optional.
     // Ignore all errors related to it, so we retain 'rv' value :-|
     (void) UseUserPrefFile();
 
     // Migrate the old prerelease telemetry pref
     if (!Preferences::GetBool(kOldTelemetryPref, true)) {
       Preferences::SetBool(kTelemetryPref, false);
@@ -1257,16 +1270,22 @@ Preferences::MakeBackupPrefFile(nsIFile 
 
 nsresult
 Preferences::ReadAndOwnUserPrefFile(nsIFile *aFile)
 {
   NS_ENSURE_ARG(aFile);
 
   if (mCurrentFile == aFile)
     return NS_OK;
+
+  // Since we're changing the pref file, we may have to make
+  // sure the outstanding writes are handled first.
+  if (AllowOffMainThreadSave()) {
+    PWHelper::FlushPendingWrites();
+  }
   mCurrentFile = aFile;
 
   nsresult rv = NS_OK;
   bool exists = false;
   mCurrentFile->Exists(&exists);
   if (exists) {
     rv = openPrefFile(mCurrentFile);
     if (NS_FAILED(rv)) {
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -15,16 +15,18 @@
 /*
  * SYNTAX HINTS:
  *
  *  - Dashes are delimiters; use underscores instead.
  *  - The first character after a period must be alphabetic.
  *  - Computed values (e.g. 50 * 1024) don't work.
  */
 
+pref("preferences.allow.omt-write", true);
+
 pref("keyword.enabled", false);
 pref("general.useragent.locale", "chrome://global/locale/intl.properties");
 pref("general.useragent.compatMode.firefox", false);
 
 // This pref exists only for testing purposes. In order to disable all
 // overrides by default, don't initialize UserAgentOverrides.jsm.
 pref("general.useragent.site_specific_overrides", true);
 
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -807,17 +807,17 @@ Database::BackupAndReplaceDatabaseFile(n
 }
 
 nsresult
 Database::ForceCrashAndReplaceDatabase(const nsCString& aReason)
 {
   Preferences::SetBool(PREF_FORCE_DATABASE_REPLACEMENT, true);
   // Ensure that prefs get saved, or we could crash before storing them.
   nsIPrefService* prefService = Preferences::GetService();
-  if (prefService && NS_SUCCEEDED(prefService->SavePrefFile(nullptr))) {
+  if (prefService && NS_SUCCEEDED(static_cast<Preferences *>(prefService)->SavePrefFileOnMainThread())) {
     // We could force an application restart here, but we'd like to get these
     // cases reported to us, so let's force a crash instead.
     MOZ_CRASH_UNSAFE_OOL(aReason.get());
   }
   return NS_ERROR_FAILURE;
 }
 
 nsresult
--- a/toolkit/components/startup/nsAppStartup.cpp
+++ b/toolkit/components/startup/nsAppStartup.cpp
@@ -909,17 +909,17 @@ nsAppStartup::TrackStartupCrashBegin(boo
     rv = Preferences::ClearUser(kPrefRecentCrashes);
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
   // recalculate since recent crashes count may have changed above
   mIsSafeModeNecessary = (recentCrashes > maxResumedCrashes && maxResumedCrashes != -1);
 
   nsCOMPtr<nsIPrefService> prefs = Preferences::GetService();
-  rv = prefs->SavePrefFile(nullptr); // flush prefs to disk since we are tracking crashes
+  rv = static_cast<Preferences *>(prefs.get())->SavePrefFileOnMainThread(); // flush prefs to disk since we are tracking crashes
   NS_ENSURE_SUCCESS(rv, rv);
 
   GetAutomaticSafeModeNecessary(aIsSafeModeNecessary);
   return rv;
 }
 
 NS_IMETHODIMP
 nsAppStartup::TrackStartupCrashEnd()
@@ -969,17 +969,17 @@ nsAppStartup::TrackStartupCrashEnd()
     rv = Preferences::SetInt(kPrefRecentCrashes, maxResumedCrashes);
     NS_ENSURE_SUCCESS(rv, rv);
   } else if (!inSafeMode) {
     // clear the count of recent crashes after a succesful startup when not in safe mode
     rv = Preferences::ClearUser(kPrefRecentCrashes);
     if (NS_FAILED(rv)) NS_WARNING("Could not clear startup crash count.");
   }
   nsCOMPtr<nsIPrefService> prefs = Preferences::GetService();
-  rv = prefs->SavePrefFile(nullptr); // flush prefs to disk since we are tracking crashes
+  rv = static_cast<Preferences *>(prefs.get())->SavePrefFileOnMainThread(); // flush prefs to disk since we are tracking crashes
 
   return rv;
 }
 
 NS_IMETHODIMP
 nsAppStartup::RestartInSafeMode(uint32_t aQuitMode)
 {
   PR_SetEnv("MOZ_SAFE_MODE_RESTART=1");
--- a/toolkit/xre/nsXREDirProvider.cpp
+++ b/toolkit/xre/nsXREDirProvider.cpp
@@ -770,19 +770,22 @@ CreateContentProcessSandboxTempDir()
                                  uuidChars);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       // If we fail to save the pref we don't want to create the temp dir,
       // because we won't be able to clean it up later.
       return nullptr;
     }
 
     nsCOMPtr<nsIPrefService> prefsvc = Preferences::GetService();
-    if (!prefsvc || NS_FAILED((rv = prefsvc->SavePrefFile(nullptr)))) {
+    if (!prefsvc || NS_FAILED((rv = static_cast<Preferences *>(prefsvc.get())->SavePrefFileOffMainThread()))) {
       // Again, if we fail to save the pref file we might not be able to clean
-      // up the temp directory, so don't create one.
+      // up the temp directory, so don't create one.  Note that in the case
+      // the preference values allows an off main thread save, the successful
+      // return from the call doesn't mean we actually saved the file.  See
+      // bug 1364496 for details.
       NS_WARNING("Failed to save pref file, cannot create temp dir.");
       return nullptr;
     }
   }
 
   nsCOMPtr<nsIFile> sandboxTempDir = GetContentProcessSandboxTempDir();
   if (!sandboxTempDir) {
     NS_WARNING("Failed to determine sandbox temp dir path.");
--- a/widget/android/PrefsHelper.h
+++ b/widget/android/PrefsHelper.h
@@ -112,17 +112,18 @@ class PrefsHelper
         if (NS_SUCCEEDED(rv)) {
             rv = aVariant->GetDataType(&varType);
         }
 
         // We use set-to-empty to signal the pref was handled.
         const bool handled = varType == nsIDataType::VTYPE_EMPTY;
 
         if (NS_SUCCEEDED(rv) && handled && aFlush) {
-            rv = Preferences::GetService()->SavePrefFile(nullptr);
+            // This will be an off main thread save
+            rv = static_cast<Preferences *>(Preferences::GetService())->SavePrefFileOffMainThread();
         }
 
         if (NS_SUCCEEDED(rv)) {
             return handled;
         }
 
         NS_WARNING(nsPrintfCString("Failed to set pref %s",
                                    aPrefName->ToCString().get()).get());
@@ -233,16 +234,17 @@ public:
             case java::PrefsHelper::PREF_STRING:
                 Preferences::SetString(name.get(), aStrVal->ToString());
                 break;
             default:
                 MOZ_ASSERT(false, "Invalid pref type");
         }
 
         if (aFlush) {
+            // This will be an off main thread save
             Preferences::GetService()->SavePrefFile(nullptr);
         }
     }
 
     static void AddObserver(const jni::Class::LocalRef& aCls,
                             jni::ObjectArray::Param aPrefNames,
                             jni::Object::Param aPrefHandler,
                             jni::ObjectArray::Param aPrefsToObserve)
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -189,19 +189,20 @@ public:
         // and re-init on foregrounding
         if (nsCacheService::GlobalInstance()) {
             nsCacheService::GlobalInstance()->Shutdown();
         }
 
         // We really want to send a notification like profile-before-change,
         // but profile-before-change ends up shutting some things down instead
         // of flushing data
-        nsIPrefService* prefs = Preferences::GetService();
+        Preferences* prefs = static_cast<Preferences *>(Preferences::GetService());
         if (prefs) {
-            prefs->SavePrefFile(nullptr);
+            // Force a main thread save
+            prefs->SavePrefFileOnMainThread();
         }
     }
 
     static void OnResume()
     {
         MOZ_ASSERT(NS_IsMainThread());
 
         sPauseCount--;
--- a/widget/nsIdleService.cpp
+++ b/widget/nsIdleService.cpp
@@ -120,17 +120,17 @@ nsIdleServiceDaily::Observe(nsISupports 
   // Set the last idle-daily time pref.
   int32_t nowSec = static_cast<int32_t>(PR_Now() / PR_USEC_PER_SEC);
   Preferences::SetInt(PREF_LAST_DAILY, nowSec);
 
   // Force that to be stored so we don't retrigger twice a day under
   // any circumstances.
   nsIPrefService* prefs = Preferences::GetService();
   if (prefs) {
-    prefs->SavePrefFile(nullptr);
+    static_cast<Preferences *>(prefs)->SavePrefFileOffMainThread();
   }
 
   MOZ_LOG(sLog, LogLevel::Debug,
          ("nsIdleServiceDaily: Storing last idle time as %d sec.", nowSec));
 #ifdef MOZ_WIDGET_ANDROID
   __android_log_print(LOG_LEVEL, LOG_TAG,
                       "Storing last idle time as %d", nowSec);
 #endif