Bug 1247798 - Make nsUrlClassifierDBService::Init() register pref observers only in the success case. r=gcp a=sylvestre
authorCykesiopka <cykesiopka.bmo@gmail.com>
Thu, 26 May 2016 10:04:54 -0700
changeset 312065 0099c0ca89fd32af6841b34e8754e8ae8f37a6db
parent 312064 1a018cf4f4d1f4d002ea0fecea0b38e9188cef50
child 312066 02df988a56aec832631463de58cd52f3a8f7c007
push id140
push userkwierso@gmail.com
push dateThu, 26 May 2016 17:07:22 +0000
treeherdermozilla-esr45@3c2bd9158ad3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp, sylvestre
bugs1247798
milestone45.2
Bug 1247798 - Make nsUrlClassifierDBService::Init() register pref observers only in the success case. r=gcp a=sylvestre These observers should only be added when everything else has succeeded. Failing to do so can cause long shutdown hangs in certain situations such as during periodic HSTS update runs.
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -1156,33 +1156,16 @@ nsUrlClassifierDBService::Init()
   mCheckForbiddenURIs = Preferences::GetBool(CHECK_FORBIDDEN_PREF,
     CHECK_FORBIDDEN_DEFAULT);
   uint32_t gethashNoise = Preferences::GetUint(GETHASH_NOISE_PREF,
     GETHASH_NOISE_DEFAULT);
   gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
     CONFIRM_AGE_DEFAULT_SEC);
   ReadTablesFromPrefs();
 
-  // Do we *really* need to be able to change all of these at runtime?
-  Preferences::AddStrongObserver(this, CHECK_MALWARE_PREF);
-  Preferences::AddStrongObserver(this, CHECK_PHISHING_PREF);
-  Preferences::AddStrongObserver(this, CHECK_TRACKING_PREF);
-  Preferences::AddStrongObserver(this, CHECK_TRACKING_PB_PREF);
-  Preferences::AddStrongObserver(this, CHECK_FORBIDDEN_PREF);
-  Preferences::AddStrongObserver(this, GETHASH_NOISE_PREF);
-  Preferences::AddStrongObserver(this, CONFIRM_AGE_PREF);
-  Preferences::AddStrongObserver(this, PHISH_TABLE_PREF);
-  Preferences::AddStrongObserver(this, MALWARE_TABLE_PREF);
-  Preferences::AddStrongObserver(this, TRACKING_TABLE_PREF);
-  Preferences::AddStrongObserver(this, TRACKING_WHITELIST_TABLE_PREF);
-  Preferences::AddStrongObserver(this, FORBIDDEN_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DOWNLOAD_BLOCK_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DOWNLOAD_ALLOW_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DISALLOW_COMPLETION_TABLE_PREF);
-
   // Force PSM loading on main thread
   nsresult rv;
   nsCOMPtr<nsICryptoHash> dummy = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Directory providers must also be accessed on the main thread.
   nsCOMPtr<nsIFile> cacheDir;
   rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_LOCAL_50_DIR,
@@ -1221,16 +1204,36 @@ nsUrlClassifierDBService::Init()
   nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
   if (!observerService)
     return NS_ERROR_FAILURE;
 
   observerService->AddObserver(this, "profile-before-change", false);
   observerService->AddObserver(this, "xpcom-shutdown-threads", false);
 
+  // XXX: Do we *really* need to be able to change all of these at runtime?
+  // Note: These observers should only be added when everything else above has
+  //       succeeded. Failing to do so can cause long shutdown times in certain
+  //       situations. See Bug 1247798 and Bug 1244803.
+  Preferences::AddStrongObserver(this, CHECK_MALWARE_PREF);
+  Preferences::AddStrongObserver(this, CHECK_PHISHING_PREF);
+  Preferences::AddStrongObserver(this, CHECK_TRACKING_PREF);
+  Preferences::AddStrongObserver(this, CHECK_TRACKING_PB_PREF);
+  Preferences::AddStrongObserver(this, CHECK_FORBIDDEN_PREF);
+  Preferences::AddStrongObserver(this, GETHASH_NOISE_PREF);
+  Preferences::AddStrongObserver(this, CONFIRM_AGE_PREF);
+  Preferences::AddStrongObserver(this, PHISH_TABLE_PREF);
+  Preferences::AddStrongObserver(this, MALWARE_TABLE_PREF);
+  Preferences::AddStrongObserver(this, TRACKING_TABLE_PREF);
+  Preferences::AddStrongObserver(this, TRACKING_WHITELIST_TABLE_PREF);
+  Preferences::AddStrongObserver(this, FORBIDDEN_TABLE_PREF);
+  Preferences::AddStrongObserver(this, DOWNLOAD_BLOCK_TABLE_PREF);
+  Preferences::AddStrongObserver(this, DOWNLOAD_ALLOW_TABLE_PREF);
+  Preferences::AddStrongObserver(this, DISALLOW_COMPLETION_TABLE_PREF);
+
   return NS_OK;
 }
 
 void
 nsUrlClassifierDBService::BuildTables(bool aTrackingProtectionEnabled,
                                       nsCString &tables)
 {
   nsAutoCString malware;