Bug 1247798 - Make nsUrlClassifierDBService::Init() register pref observers only in the success case. r=gcp
authorCykesiopka <cykesiopka.bmo@gmail.com>
Sun, 03 Apr 2016 00:58:43 -0700
changeset 292011 eb010c3c8e20dabf16a27af39a4aec9a942c7470
parent 292010 51688b3611d4f2879c4b3c2269500bb494bcc950
child 292012 4972c9a34bca7021427ef4d84fbec53b62b95098
push id18579
push usercbook@mozilla.com
push dateThu, 07 Apr 2016 13:28:28 +0000
treeherderfx-team@eb010c3c8e20 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1247798
milestone48.0a1
Bug 1247798 - Make nsUrlClassifierDBService::Init() register pref observers only in the success case. r=gcp 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. MozReview-Commit-ID: IoJ4PscS1u3
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -1199,35 +1199,16 @@ nsUrlClassifierDBService::Init()
   mCheckBlockedURIs = Preferences::GetBool(CHECK_BLOCKED_PREF,
     CHECK_BLOCKED_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, CHECK_BLOCKED_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, BLOCKED_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,
@@ -1266,16 +1247,38 @@ 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, CHECK_BLOCKED_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, BLOCKED_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;