Bug 789945: Part 4. Explicitly make some pref save be blocking calls. r?bsmedberg draft
authorMilan Sreckovic <milan@mozilla.com>
Wed, 14 Jun 2017 13:38:17 -0400
changeset 594219 6ee547a0ddf1048c7ebcd3a67712c8c9c3f2c3ab
parent 594218 0a8ee15fb58c14633da6de615349ef5a479faea7
child 633351 9a58efc238389bf946eb48cc4b24b648129f615d
push id63952
push userbmo:milan@mozilla.com
push dateWed, 14 Jun 2017 17:40:02 +0000
reviewersbsmedberg
bugs789945
milestone56.0a1
Bug 789945: Part 4. Explicitly make some pref save be blocking calls. r?bsmedberg MozReview-Commit-ID: 1MEp7o65HAV
gfx/src/DriverCrashGuard.cpp
modules/libpref/init/all.js
toolkit/components/places/Database.cpp
toolkit/components/startup/nsAppStartup.cpp
toolkit/xre/nsXREDirProvider.cpp
widget/android/nsAppShell.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)->SavePrefFileBlocking();
   }
 }
 
 void
 DriverCrashGuard::ForEachActiveCrashGuard(const CrashGuardCallback& aCallback)
 {
   if (!AreCrashGuardsEnabled()) {
     // Even if guards look active (via prefs), they can be ignored if globally
--- 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)->SavePrefFileBlocking())) {
     // 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())->SavePrefFileBlocking(); // 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,20 @@ 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
+  // save prefs to disk since we are tracking crashes.  This may be
+  // asynchronous, so a crash could sneak in that we would mistake for
+  // a start up crash. See bug 789945 and bug 1361262.
+  rv = prefs->SavePrefFile(nullptr);
 
   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
@@ -768,17 +768,20 @@ CreateContentProcessSandboxTempDir()
       // 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)))) {
       // 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/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 blocking save
+            prefs->SavePrefFileBlocking();
         }
     }
 
     static void OnResume()
     {
         MOZ_ASSERT(NS_IsMainThread());
 
         sPauseCount--;