Bug 1353853 - Cache preferences when doing channel classify draft
authorThomas Nguyen <tnguyen@mozilla.com>
Tue, 18 Apr 2017 17:00:27 +0800
changeset 566177 279896c0ac20e33e488e5dfd8dc590210ee80413
parent 564098 bb38d935d699e0529f9e0bb35578d381026415c4
child 625222 3c2c7bcb4bb65f895d2b13e8e640153b80365ec1
push id55118
push userbmo:tnguyen@mozilla.com
push dateFri, 21 Apr 2017 02:52:14 +0000
bugs1353853
milestone55.0a1
Bug 1353853 - Cache preferences when doing channel classify We will cache all preferences which will be read during classifing channel - Store them into static variables nsUrlClassifierDBService - Use a singleton class to manage/update preferrences in nsChannelClassifier MozReview-Commit-ID: GvyBI3rVpYh
netwerk/base/nsChannelClassifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
--- a/netwerk/base/nsChannelClassifier.cpp
+++ b/netwerk/base/nsChannelClassifier.cpp
@@ -43,46 +43,144 @@
 namespace mozilla {
 namespace net {
 
 //
 // MOZ_LOG=nsChannelClassifier:5
 //
 static LazyLogModule gChannelClassifierLog("nsChannelClassifier");
 
-// Whether channels should be annotated as being on the tracking protection
-// list.
-static bool sAnnotateChannelEnabled = false;
-// Whether the priority of the channels annotated as being on the tracking
-// protection list should be lowered.
-static bool sLowerNetworkPriority = false;
-static bool sIsInited = false;
 
 #undef LOG
 #define LOG(args)     MOZ_LOG(gChannelClassifierLog, LogLevel::Debug, args)
 #define LOG_ENABLED() MOZ_LOG_TEST(gChannelClassifierLog, LogLevel::Debug)
 
+#define URLCLASSIFIER_SKIP_HOSTNAMES       "urlclassifier.skipHostnames"
+#define URLCLASSIFIER_TRACKING_WHITELIST   "urlclassifier.trackingWhitelistTable"
+
+// Put CachedPrefs in anonymous namespace to avoid any collision from outside of
+// this file.
+namespace {
+
+/**
+ * It is not recommended to read from Preference everytime a channel is
+ * connected.
+ * That is not fast and we should cache preference values and reuse them
+ */
+class CachedPrefs final
+{
+public:
+  static CachedPrefs* GetInstance();
+
+  void Init();
+  bool IsAllowListExample() { return sAllowListExample;}
+  bool IsLowerNetworkPriority() { return sLowerNetworkPriority;}
+  bool IsAnnotateChannelEnabled() { return sAnnotateChannelEnabled;}
+  nsCString GetTrackingWhiteList() { return mTrackingWhitelist; }
+  void SetTrackingWhiteList(const nsACString& aList) { mTrackingWhitelist = aList; }
+  nsCString GetSkipHostnames() { return mSkipHostnames; }
+  void SetSkipHostnames(const nsACString& aHostnames) { mSkipHostnames = aHostnames; }
+
+private:
+  friend class StaticAutoPtr<CachedPrefs>;
+  CachedPrefs();
+  ~CachedPrefs();
+
+  static void OnPrefsChange(const char* aPrefName, void* );
+
+  // Whether channels should be annotated as being on the tracking protection
+  // list.
+  static bool sAnnotateChannelEnabled;
+  // Whether the priority of the channels annotated as being on the tracking
+  // protection list should be lowered.
+  static bool sLowerNetworkPriority;
+  static bool sAllowListExample;
+
+  nsCString mTrackingWhitelist;
+  nsCString mSkipHostnames;
+
+  static StaticAutoPtr<CachedPrefs> sInstance;
+};
+
+bool CachedPrefs::sAllowListExample = false;
+bool CachedPrefs::sLowerNetworkPriority = false;
+bool CachedPrefs::sAnnotateChannelEnabled = false;
+
+StaticAutoPtr<CachedPrefs> CachedPrefs::sInstance;
+
+// static
+void
+CachedPrefs::OnPrefsChange(const char* aPref, void* aClosure)
+{
+  CachedPrefs* prefs = static_cast<CachedPrefs*> (aClosure);
+
+  if (!strcmp(aPref, URLCLASSIFIER_SKIP_HOSTNAMES)) {
+    nsCString skipHostnames = Preferences::GetCString(URLCLASSIFIER_SKIP_HOSTNAMES);
+    ToLowerCase(skipHostnames);
+    prefs->SetSkipHostnames(skipHostnames);
+  } else if (!strcmp(aPref, URLCLASSIFIER_TRACKING_WHITELIST)) {
+    nsCString trackingWhitelist = Preferences::GetCString(URLCLASSIFIER_TRACKING_WHITELIST);
+    prefs->SetTrackingWhiteList(trackingWhitelist);
+  }
+}
+
+void
+CachedPrefs::Init()
+{
+  Preferences::AddBoolVarCache(&sAnnotateChannelEnabled,
+                               "privacy.trackingprotection.annotate_channels");
+  Preferences::AddBoolVarCache(&sLowerNetworkPriority,
+                               "privacy.trackingprotection.lower_network_priority");
+  Preferences::AddBoolVarCache(&sAllowListExample,
+                               "channelclassifier.allowlist_example");
+  Preferences::RegisterCallbackAndCall(CachedPrefs::OnPrefsChange,
+                                       URLCLASSIFIER_SKIP_HOSTNAMES, this);
+  Preferences::RegisterCallbackAndCall(CachedPrefs::OnPrefsChange,
+                                       URLCLASSIFIER_TRACKING_WHITELIST, this);
+
+}
+
+// static
+CachedPrefs*
+CachedPrefs::GetInstance()
+{
+  if (!sInstance) {
+    sInstance = new CachedPrefs();
+    sInstance->Init();
+    ClearOnShutdown(&sInstance);
+  }
+  MOZ_ASSERT(sInstance);
+  return sInstance;
+}
+
+CachedPrefs::CachedPrefs()
+{
+  MOZ_COUNT_CTOR(CachedPrefs);
+}
+
+CachedPrefs::~CachedPrefs()
+{
+  MOZ_COUNT_DTOR(CachedPrefs);
+
+  Preferences::UnregisterCallback(CachedPrefs::OnPrefsChange, URLCLASSIFIER_SKIP_HOSTNAMES, this);
+  Preferences::UnregisterCallback(CachedPrefs::OnPrefsChange, URLCLASSIFIER_TRACKING_WHITELIST, this);
+}
+} // anonymous namespace
+
 NS_IMPL_ISUPPORTS(nsChannelClassifier,
                   nsIURIClassifierCallback,
                   nsIObserver)
 
 nsChannelClassifier::nsChannelClassifier(nsIChannel *aChannel)
   : mIsAllowListed(false),
     mSuspendedChannel(false),
     mChannel(aChannel),
     mTrackingProtectionEnabled(Nothing())
 {
   MOZ_ASSERT(mChannel);
-  if (!sIsInited) {
-    sIsInited = true;
-    Preferences::AddBoolVarCache(&sAnnotateChannelEnabled,
-                                 "privacy.trackingprotection.annotate_channels");
-    Preferences::AddBoolVarCache(&sLowerNetworkPriority,
-                                 "privacy.trackingprotection.lower_network_priority");
-  }
 }
 
 nsresult
 nsChannelClassifier::ShouldEnableTrackingProtection(bool *result)
 {
   nsresult rv = ShouldEnableTrackingProtectionInternal(mChannel, result);
   mTrackingProtectionEnabled = Some(*result);
   return rv;
@@ -151,18 +249,17 @@ nsChannelClassifier::ShouldEnableTrackin
 
     if (AddonMayLoad(aChannel, chanURI)) {
         return NS_OK;
     }
 
     nsCOMPtr<nsIIOService> ios = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    const char ALLOWLIST_EXAMPLE_PREF[] = "channelclassifier.allowlist_example";
-    if (!topWinURI && Preferences::GetBool(ALLOWLIST_EXAMPLE_PREF, false)) {
+    if (!topWinURI && CachedPrefs::GetInstance()->IsAllowListExample()) {
       LOG(("nsChannelClassifier[%p]: Allowlisting test domain\n", this));
       rv = ios->NewURI(NS_LITERAL_CSTRING("http://allowlisted.example.com"),
                        nullptr, nullptr, getter_AddRefs(topWinURI));
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     // Take the host/port portion so we can allowlist by site. Also ignore the
     // scheme, since users who put sites on the allowlist probably don't expect
@@ -360,24 +457,21 @@ nsChannelClassifier::StartInternal()
     if (hasFlags) return NS_ERROR_UNEXPECTED;
 
     rv = NS_URIChainHasFlags(uri,
                              nsIProtocolHandler::URI_IS_LOCAL_RESOURCE,
                              &hasFlags);
     NS_ENSURE_SUCCESS(rv, rv);
     if (hasFlags) return NS_ERROR_UNEXPECTED;
 
-    // Skip whitelisted hostnames.
-    nsAutoCString whitelisted;
-    Preferences::GetCString("urlclassifier.skipHostnames", &whitelisted);
-    if (!whitelisted.IsEmpty()) {
-      ToLowerCase(whitelisted);
+    nsCString skipHostnames = CachedPrefs::GetInstance()->GetSkipHostnames();
+    if (!skipHostnames.IsEmpty()) {
       LOG(("nsChannelClassifier[%p]:StartInternal whitelisted hostnames = %s",
-           this, whitelisted.get()));
-      if (IsHostnameWhitelisted(uri, whitelisted)) {
+           this, skipHostnames.get()));
+      if (IsHostnameWhitelisted(uri, skipHostnames)) {
         return NS_ERROR_UNEXPECTED;
       }
     }
 
     nsCOMPtr<nsIURIClassifier> uriClassifier =
         do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv);
     if (rv == NS_ERROR_FACTORY_NOT_REGISTERED ||
         rv == NS_ERROR_NOT_AVAILABLE) {
@@ -406,17 +500,19 @@ nsChannelClassifier::StartInternal()
       nsCOMPtr<nsIURI> principalURI;
       principal->GetURI(getter_AddRefs(principalURI));
       LOG(("nsChannelClassifier[%p]: Classifying principal %s on channel with "
            "uri %s", this, principalURI->GetSpecOrDefault().get(),
            uri->GetSpecOrDefault().get()));
     }
     // The classify is running in parent process, no need to give a valid event
     // target
-    rv = uriClassifier->Classify(principal, nullptr, sAnnotateChannelEnabled | trackingProtectionEnabled,
+    rv = uriClassifier->Classify(principal, nullptr,
+                                 CachedPrefs::GetInstance()->IsAnnotateChannelEnabled() ||
+                                   trackingProtectionEnabled,
                                  this, &expectCallback);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     if (expectCallback) {
         // Suspend the channel, it will be resumed when we get the classifier
         // callback.
@@ -728,20 +824,18 @@ nsChannelClassifier::IsTrackerWhiteliste
                                           const nsACString& aProvider,
                                           const nsACString& aPrefix)
 {
   nsresult rv;
   nsCOMPtr<nsIURIClassifier> uriClassifier =
     do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoCString tables;
-  Preferences::GetCString("urlclassifier.trackingWhitelistTable", &tables);
-
-  if (tables.IsEmpty()) {
+  nsCString trackingWhitelist = CachedPrefs::GetInstance()->GetTrackingWhiteList();
+  if (trackingWhitelist.IsEmpty()) {
     LOG(("nsChannelClassifier[%p]:IsTrackerWhitelisted whitelist disabled",
          this));
     return NS_ERROR_TRACKING_URI;
   }
 
   nsCOMPtr<nsIHttpChannelInternal> chan = do_QueryInterface(mChannel, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -775,17 +869,17 @@ nsChannelClassifier::IsTrackerWhiteliste
   nsCOMPtr<nsIURI> whitelistURI;
   rv = NS_NewURI(getter_AddRefs(whitelistURI), whitelistEntry);
   NS_ENSURE_SUCCESS(rv, rv);
 
   RefPtr<IsTrackerWhitelistedCallback> cb =
     new IsTrackerWhitelistedCallback(this, aList, aProvider, aPrefix,
                                      whitelistEntry);
 
-  return uriClassifier->AsyncClassifyLocalWithTables(whitelistURI, tables, cb);
+  return uriClassifier->AsyncClassifyLocalWithTables(whitelistURI, trackingWhitelist, cb);
 }
 
 NS_IMETHODIMP
 nsChannelClassifier::OnClassifyComplete(nsresult aErrorCode,
                                         const nsACString& aList,
                                         const nsACString& aProvider,
                                         const nsACString& aPrefix)
 {
@@ -818,31 +912,31 @@ nsChannelClassifier::OnClassifyCompleteI
       MarkEntryClassified(aErrorCode);
 
       // The value of |mTrackingProtectionEnabled| should be assigned at
       // |ShouldEnableTrackingProtection| before.
       MOZ_ASSERT(mTrackingProtectionEnabled, "Should contain a value.");
 
       if (aErrorCode == NS_ERROR_TRACKING_URI &&
           !mTrackingProtectionEnabled.valueOr(false)) {
-        if (sAnnotateChannelEnabled) {
+        if (CachedPrefs::GetInstance()->IsAnnotateChannelEnabled()) {
           nsCOMPtr<nsIParentChannel> parentChannel;
           NS_QueryNotificationCallbacks(mChannel, parentChannel);
           if (parentChannel) {
             // This channel is a parent-process proxy for a child process
             // request. We should notify the child process as well.
             parentChannel->NotifyTrackingResource();
           }
           RefPtr<HttpBaseChannel> httpChannel = do_QueryObject(mChannel);
           if (httpChannel) {
             httpChannel->SetIsTrackingResource();
           }
         }
 
-        if (sLowerNetworkPriority) {
+        if (CachedPrefs::GetInstance()->IsLowerNetworkPriority()) {
           if (LOG_ENABLED()) {
             nsCOMPtr<nsIURI> uri;
             mChannel->GetURI(getter_AddRefs(uri));
             LOG(("nsChannelClassifier[%p]: lower the priority of channel %p"
                  ", since %s is a tracker", this, mChannel.get(),
                  uri->GetSpecOrDefault().get()));
           }
           nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mChannel);
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -21,19 +21,16 @@ class nsIThread;
 namespace mozilla {
 namespace safebrowsing {
 
 /**
  * Maintains the stores and LookupCaches for the url classifier.
  */
 class Classifier {
 public:
-  typedef nsClassHashtable<nsCStringHashKey, nsCString> ProviderDictType;
-
-public:
   Classifier();
   ~Classifier();
 
   nsresult Open(nsIFile& aCacheDirectory);
   void Close();
   void Reset(); // Not including any intermediary for update.
 
   /**
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -13,17 +13,16 @@
 #include "nsIDirectoryService.h"
 #include "nsIKeyModule.h"
 #include "nsIObserverService.h"
 #include "nsIPermissionManager.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
 #include "nsIProperties.h"
 #include "nsToolkitCompsCID.h"
-#include "nsIUrlClassifierUtils.h"
 #include "nsIXULRuntime.h"
 #include "nsUrlClassifierDBService.h"
 #include "nsUrlClassifierUtils.h"
 #include "nsUrlClassifierProxies.h"
 #include "nsURILoader.h"
 #include "nsString.h"
 #include "nsReadableUtils.h"
 #include "nsTArray.h"
@@ -62,18 +61,18 @@ namespace safebrowsing {
 
 nsresult
 TablesToResponse(const nsACString& tables)
 {
   if (tables.IsEmpty()) {
     return NS_OK;
   }
 
-  // We don't check mCheckMalware and friends because BuildTables never
-  // includes a table that is not enabled.
+  // We don't check mCheckMalware and friends because disabled tables are
+  // never included
   if (FindInReadable(NS_LITERAL_CSTRING("-malware-"), tables)) {
     return NS_ERROR_MALWARE_URI;
   }
   if (FindInReadable(NS_LITERAL_CSTRING("-phish-"), tables)) {
     return NS_ERROR_PHISHING_URI;
   }
   if (FindInReadable(NS_LITERAL_CSTRING("-unwanted-"), tables)) {
     return NS_ERROR_UNWANTED_URI;
@@ -93,39 +92,19 @@ TablesToResponse(const nsACString& table
 using namespace mozilla;
 using namespace mozilla::safebrowsing;
 
 // MOZ_LOG=UrlClassifierDbService:5
 LazyLogModule gUrlClassifierDbServiceLog("UrlClassifierDbService");
 #define LOG(args) MOZ_LOG(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
 
-// Prefs for implementing nsIURIClassifier to block page loads
-#define CHECK_MALWARE_PREF      "browser.safebrowsing.malware.enabled"
-#define CHECK_MALWARE_DEFAULT   false
-
-#define CHECK_PHISHING_PREF     "browser.safebrowsing.phishing.enabled"
-#define CHECK_PHISHING_DEFAULT  false
-
-#define CHECK_BLOCKED_PREF    "browser.safebrowsing.blockedURIs.enabled"
-#define CHECK_BLOCKED_DEFAULT false
-
 #define GETHASH_NOISE_PREF      "urlclassifier.gethashnoise"
 #define GETHASH_NOISE_DEFAULT   4
 
-// Comma-separated lists
-#define MALWARE_TABLE_PREF              "urlclassifier.malwareTable"
-#define PHISH_TABLE_PREF                "urlclassifier.phishTable"
-#define TRACKING_TABLE_PREF             "urlclassifier.trackingTable"
-#define TRACKING_WHITELIST_TABLE_PREF   "urlclassifier.trackingWhitelistTable"
-#define BLOCKED_TABLE_PREF              "urlclassifier.blockedTable"
-#define DOWNLOAD_BLOCK_TABLE_PREF       "urlclassifier.downloadBlockTable"
-#define DOWNLOAD_ALLOW_TABLE_PREF       "urlclassifier.downloadAllowTable"
-#define DISALLOW_COMPLETION_TABLE_PREF  "urlclassifier.disallow_completions"
-
 #define CONFIRM_AGE_PREF        "urlclassifier.max-complete-age"
 #define CONFIRM_AGE_DEFAULT_SEC (45 * 60)
 
 // 30 minutes as the maximum negative cache duration.
 #define MAXIMUM_NEGATIVE_CACHE_DURATION_SEC (30 * 60 * 1000)
 
 // TODO: The following two prefs are to be removed after we
 //       roll out full v4 hash completion. See Bug 1331534.
@@ -138,17 +117,18 @@ class nsUrlClassifierDBServiceWorker;
 static nsUrlClassifierDBService* sUrlClassifierDBService;
 
 nsIThread* nsUrlClassifierDBService::gDbBackgroundThread = nullptr;
 
 // Once we've committed to shutting down, don't do work in the background
 // thread.
 static bool gShuttingDownThread = false;
 
-static mozilla::Atomic<int32_t> gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC);
+static mozilla::Atomic<uint32_t, Relaxed> gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC);
+static uint32_t sGethashNoise = GETHASH_NOISE_DEFAULT;
 
 NS_IMPL_ISUPPORTS(nsUrlClassifierDBServiceWorker,
                   nsIUrlClassifierDBService)
 
 nsUrlClassifierDBServiceWorker::nsUrlClassifierDBServiceWorker()
   : mInStream(false)
   , mGethashNoise(0)
   , mPendingLookupLock("nsUrlClassifierDBServerWorker.mPendingLookupLock")
@@ -1587,58 +1567,73 @@ nsUrlClassifierDBService::nsUrlClassifie
 {
 }
 
 nsUrlClassifierDBService::~nsUrlClassifierDBService()
 {
   sUrlClassifierDBService = nullptr;
 }
 
+void
+AppendTables(const nsCString& aTables, nsCString &outTables)
+{
+  if (!aTables.IsEmpty()) {
+    if (!outTables.IsEmpty()) {
+      outTables.Append(',');
+    }
+    outTables.Append(aTables);
+  }
+}
+
 nsresult
 nsUrlClassifierDBService::ReadTablesFromPrefs()
 {
+  mCheckMalware = Preferences::GetBool(CHECK_MALWARE_PREF,
+    CHECK_MALWARE_DEFAULT);
+  mCheckPhishing = Preferences::GetBool(CHECK_PHISHING_PREF,
+    CHECK_PHISHING_DEFAULT);
+  mCheckBlockedURIs = Preferences::GetBool(CHECK_BLOCKED_PREF,
+    CHECK_BLOCKED_DEFAULT);
+
   nsCString allTables;
   nsCString tables;
+
+  mBaseTables.Truncate();
+  mTrackingProtectionTables.Truncate();
+
   Preferences::GetCString(PHISH_TABLE_PREF, &allTables);
+  if (mCheckPhishing) {
+    AppendTables(allTables, mBaseTables);
+  }
 
   Preferences::GetCString(MALWARE_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
+  AppendTables(tables, allTables);
+  if (mCheckMalware) {
+    AppendTables(tables, mBaseTables);
+  }
+
+  Preferences::GetCString(BLOCKED_TABLE_PREF, &tables);
+  AppendTables(tables, allTables);
+  if (mCheckBlockedURIs) {
+    AppendTables(tables, mBaseTables);
   }
 
   Preferences::GetCString(DOWNLOAD_BLOCK_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
-  }
+  AppendTables(tables, allTables);
 
   Preferences::GetCString(DOWNLOAD_ALLOW_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
-  }
+  AppendTables(tables, allTables);
 
   Preferences::GetCString(TRACKING_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
-  }
+  AppendTables(tables, allTables);
+  AppendTables(tables, mTrackingProtectionTables);
 
   Preferences::GetCString(TRACKING_WHITELIST_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
-  }
-
-  Preferences::GetCString(BLOCKED_TABLE_PREF, &tables);
-  if (!tables.IsEmpty()) {
-    allTables.Append(',');
-    allTables.Append(tables);
-  }
+  AppendTables(tables, allTables);
+  AppendTables(tables, mTrackingProtectionTables);
 
   Classifier::SplitTables(allTables, mGethashTables);
 
   Preferences::GetCString(DISALLOW_COMPLETION_TABLE_PREF, &tables);
   Classifier::SplitTables(tables, mDisallowCompletionsTables);
 
   return NS_OK;
 }
@@ -1666,29 +1661,21 @@ nsUrlClassifierDBService::Init()
     // Note that since we never register an observer, Shutdown() will also never
     // be called in the content process.
     return NS_OK;
   default:
     // No other process type is supported!
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  // Retrieve all the preferences.
-  mCheckMalware = Preferences::GetBool(CHECK_MALWARE_PREF,
-    CHECK_MALWARE_DEFAULT);
-  mCheckPhishing = Preferences::GetBool(CHECK_PHISHING_PREF,
-    CHECK_PHISHING_DEFAULT);
-  mCheckBlockedURIs = Preferences::GetBool(CHECK_BLOCKED_PREF,
-    CHECK_BLOCKED_DEFAULT);
-  uint32_t gethashNoise = Preferences::GetUint(GETHASH_NOISE_PREF,
+  sGethashNoise = Preferences::GetUint(GETHASH_NOISE_PREF,
     GETHASH_NOISE_DEFAULT);
   gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
     CONFIRM_AGE_DEFAULT_SEC);
   ReadTablesFromPrefs();
-
   nsresult rv;
 
   {
     // Force PSM loading on main thread
     nsCOMPtr<nsICryptoHash> dummy = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -1715,17 +1702,17 @@ nsUrlClassifierDBService::Init()
   rv = NS_NewNamedThread("URL Classifier", &gDbBackgroundThread);
   if (NS_FAILED(rv))
     return rv;
 
   mWorker = new nsUrlClassifierDBServiceWorker();
   if (!mWorker)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  rv = mWorker->Init(gethashNoise, cacheDir);
+  rv = mWorker->Init(sGethashNoise, cacheDir);
   if (NS_FAILED(rv)) {
     mWorker = nullptr;
     return rv;
   }
 
   // Proxy for calling the worker on the background thread
   mWorkerProxy = new UrlClassifierDBServiceWorkerProxy(mWorker);
   rv = mWorkerProxy->OpenDb();
@@ -1742,74 +1729,28 @@ nsUrlClassifierDBService::Init()
   // The application is about to quit
   observerService->AddObserver(this, "quit-application", false);
   observerService->AddObserver(this, "profile-before-change", 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_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, BLOCKED_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DOWNLOAD_BLOCK_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DOWNLOAD_ALLOW_TABLE_PREF);
-  Preferences::AddStrongObserver(this, DISALLOW_COMPLETION_TABLE_PREF);
+  Preferences::AddUintVarCache(&sGethashNoise, GETHASH_NOISE_PREF,
+    GETHASH_NOISE_DEFAULT);
+  Preferences::AddAtomicUintVarCache(&gFreshnessGuarantee, CONFIRM_AGE_PREF,
+    CONFIRM_AGE_DEFAULT_SEC);
+
+  for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
+    Preferences::AddStrongObserver(this, kObservedPrefs[i].get());
+  }
 
   return NS_OK;
 }
 
-void
-nsUrlClassifierDBService::BuildTables(bool aTrackingProtectionEnabled,
-                                      nsCString &tables)
-{
-  nsAutoCString malware;
-  // LookupURI takes a comma-separated list already.
-  Preferences::GetCString(MALWARE_TABLE_PREF, &malware);
-  if (mCheckMalware && !malware.IsEmpty()) {
-    tables.Append(malware);
-  }
-  nsAutoCString phishing;
-  Preferences::GetCString(PHISH_TABLE_PREF, &phishing);
-  if (mCheckPhishing && !phishing.IsEmpty()) {
-    tables.Append(',');
-    tables.Append(phishing);
-  }
-  if (aTrackingProtectionEnabled) {
-    nsAutoCString tracking, trackingWhitelist;
-    Preferences::GetCString(TRACKING_TABLE_PREF, &tracking);
-    if (!tracking.IsEmpty()) {
-      tables.Append(',');
-      tables.Append(tracking);
-    }
-    Preferences::GetCString(TRACKING_WHITELIST_TABLE_PREF, &trackingWhitelist);
-    if (!trackingWhitelist.IsEmpty()) {
-      tables.Append(',');
-      tables.Append(trackingWhitelist);
-    }
-  }
-  nsAutoCString blocked;
-  Preferences::GetCString(BLOCKED_TABLE_PREF, &blocked);
-  if (mCheckBlockedURIs && !blocked.IsEmpty()) {
-    tables.Append(',');
-    tables.Append(blocked);
-  }
-
-  if (StringBeginsWith(tables, NS_LITERAL_CSTRING(","))) {
-    tables.Cut(0, 1);
-  }
-}
-
 // nsChannelClassifier is the only consumer of this interface.
 NS_IMETHODIMP
 nsUrlClassifierDBService::Classify(nsIPrincipal* aPrincipal,
                                    nsIEventTarget* aEventTarget,
                                    bool aTrackingProtectionEnabled,
                                    nsIURIClassifierCallback* c,
                                    bool* result)
 {
@@ -1855,18 +1796,20 @@ nsUrlClassifierDBService::Classify(nsIPr
     return NS_OK;
   }
 
   RefPtr<nsUrlClassifierClassifyCallback> callback =
     new nsUrlClassifierClassifyCallback(c);
 
   if (!callback) return NS_ERROR_OUT_OF_MEMORY;
 
-  nsAutoCString tables;
-  BuildTables(aTrackingProtectionEnabled, tables);
+  nsCString tables = mBaseTables;
+  if (aTrackingProtectionEnabled) {
+    AppendTables(mTrackingProtectionTables, tables);
+  }
 
   nsresult rv = LookupURI(aPrincipal, tables, callback, false, result);
   if (rv == NS_ERROR_MALFORMED_URI) {
     *result = false;
     // The URI had no hostname, don't try to classify it.
     return NS_OK;
   }
   NS_ENSURE_SUCCESS(rv, rv);
@@ -2333,39 +2276,18 @@ nsUrlClassifierDBService::Observe(nsISup
                                   const char16_t *aData)
 {
   if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) {
     nsresult rv;
     nsCOMPtr<nsIPrefBranch> prefs(do_QueryInterface(aSubject, &rv));
     NS_ENSURE_SUCCESS(rv, rv);
     Unused << prefs;
 
-    if (NS_LITERAL_STRING(CHECK_MALWARE_PREF).Equals(aData)) {
-      mCheckMalware = Preferences::GetBool(CHECK_MALWARE_PREF,
-        CHECK_MALWARE_DEFAULT);
-    } else if (NS_LITERAL_STRING(CHECK_PHISHING_PREF).Equals(aData)) {
-      mCheckPhishing = Preferences::GetBool(CHECK_PHISHING_PREF,
-        CHECK_PHISHING_DEFAULT);
-    } else if (NS_LITERAL_STRING(CHECK_BLOCKED_PREF).Equals(aData)) {
-      mCheckBlockedURIs = Preferences::GetBool(CHECK_BLOCKED_PREF,
-        CHECK_BLOCKED_DEFAULT);
-    } else if (
-      NS_LITERAL_STRING(PHISH_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(MALWARE_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(TRACKING_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(TRACKING_WHITELIST_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(BLOCKED_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(DOWNLOAD_BLOCK_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(DOWNLOAD_ALLOW_TABLE_PREF).Equals(aData) ||
-      NS_LITERAL_STRING(DISALLOW_COMPLETION_TABLE_PREF).Equals(aData)) {
-      // Just read everything again.
+    if (kObservedPrefs.Contains(NS_ConvertUTF16toUTF8(aData))) {
       ReadTablesFromPrefs();
-    } else if (NS_LITERAL_STRING(CONFIRM_AGE_PREF).Equals(aData)) {
-      gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
-        CONFIRM_AGE_DEFAULT_SEC);
     }
   } else if (!strcmp(aTopic, "quit-application")) {
     // Tell the update thread to finish as soon as possible.
     gShuttingDownThread = true;
   } else if (!strcmp(aTopic, "profile-before-change")) {
     gShuttingDownThread = true;
     Shutdown();
   } else {
@@ -2387,28 +2309,19 @@ nsUrlClassifierDBService::Shutdown()
   }
 
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_SHUTDOWN_TIME> timer;
 
   mCompleters.Clear();
 
   nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefs) {
-    prefs->RemoveObserver(CHECK_MALWARE_PREF, this);
-    prefs->RemoveObserver(CHECK_PHISHING_PREF, this);
-    prefs->RemoveObserver(CHECK_BLOCKED_PREF, this);
-    prefs->RemoveObserver(PHISH_TABLE_PREF, this);
-    prefs->RemoveObserver(MALWARE_TABLE_PREF, this);
-    prefs->RemoveObserver(TRACKING_TABLE_PREF, this);
-    prefs->RemoveObserver(TRACKING_WHITELIST_TABLE_PREF, this);
-    prefs->RemoveObserver(BLOCKED_TABLE_PREF, this);
-    prefs->RemoveObserver(DOWNLOAD_BLOCK_TABLE_PREF, this);
-    prefs->RemoveObserver(DOWNLOAD_ALLOW_TABLE_PREF, this);
-    prefs->RemoveObserver(DISALLOW_COMPLETION_TABLE_PREF, this);
-    prefs->RemoveObserver(CONFIRM_AGE_PREF, this);
+    for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
+      prefs->RemoveObserver(kObservedPrefs[i].get(), this);
+    }
   }
 
   // 1. Synchronize with worker thread and update thread by
   //    *synchronously* dispatching an event to worker thread
   //    for shutting down the update thread. The reason not
   //    shutting down update thread directly from main thread
   //    is to avoid racing for Classifier::mUpdateThread
   //    between main thread and the worker thread. (Both threads
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -36,16 +36,36 @@
 #define DOMAIN_LENGTH 4
 
 // The hash length of a partial hash entry.
 #define PARTIAL_LENGTH 4
 
 // The hash length of a complete hash entry.
 #define COMPLETE_LENGTH 32
 
+// Prefs for implementing nsIURIClassifier to block page loads
+#define CHECK_MALWARE_PREF      "browser.safebrowsing.malware.enabled"
+#define CHECK_MALWARE_DEFAULT   false
+
+#define CHECK_PHISHING_PREF     "browser.safebrowsing.phishing.enabled"
+#define CHECK_PHISHING_DEFAULT  false
+
+#define CHECK_BLOCKED_PREF      "browser.safebrowsing.blockedURIs.enabled"
+#define CHECK_BLOCKED_DEFAULT   false
+
+// Comma-separated lists
+#define MALWARE_TABLE_PREF              "urlclassifier.malwareTable"
+#define PHISH_TABLE_PREF                "urlclassifier.phishTable"
+#define TRACKING_TABLE_PREF             "urlclassifier.trackingTable"
+#define TRACKING_WHITELIST_TABLE_PREF   "urlclassifier.trackingWhitelistTable"
+#define BLOCKED_TABLE_PREF              "urlclassifier.blockedTable"
+#define DOWNLOAD_BLOCK_TABLE_PREF       "urlclassifier.downloadBlockTable"
+#define DOWNLOAD_ALLOW_TABLE_PREF       "urlclassifier.downloadAllowTable"
+#define DISALLOW_COMPLETION_TABLE_PREF  "urlclassifier.disallow_completions"
+
 using namespace mozilla::safebrowsing;
 
 class nsUrlClassifierDBServiceWorker;
 class nsIThread;
 class nsIURI;
 class UrlClassifierDBServiceWorkerProxy;
 namespace mozilla {
 namespace safebrowsing {
@@ -85,16 +105,31 @@ public:
   nsresult CacheCompletions(mozilla::safebrowsing::CacheResultArray *results);
   nsresult CacheMisses(mozilla::safebrowsing::PrefixArray *results);
 
   static nsIThread* BackgroundThread();
 
   static bool ShutdownHasStarted();
 
 private:
+
+  const nsTArray<nsCString> kObservedPrefs = {
+    NS_LITERAL_CSTRING(CHECK_MALWARE_PREF),
+    NS_LITERAL_CSTRING(CHECK_PHISHING_PREF),
+    NS_LITERAL_CSTRING(CHECK_BLOCKED_PREF),
+    NS_LITERAL_CSTRING(MALWARE_TABLE_PREF),
+    NS_LITERAL_CSTRING(PHISH_TABLE_PREF),
+    NS_LITERAL_CSTRING(TRACKING_TABLE_PREF),
+    NS_LITERAL_CSTRING(TRACKING_WHITELIST_TABLE_PREF),
+    NS_LITERAL_CSTRING(BLOCKED_TABLE_PREF),
+    NS_LITERAL_CSTRING(DOWNLOAD_BLOCK_TABLE_PREF),
+    NS_LITERAL_CSTRING(DOWNLOAD_ALLOW_TABLE_PREF),
+    NS_LITERAL_CSTRING(DISALLOW_COMPLETION_TABLE_PREF)
+  };
+
   // No subclassing
   ~nsUrlClassifierDBService();
 
   // Disallow copy constructor
   nsUrlClassifierDBService(nsUrlClassifierDBService&);
 
   nsresult LookupURI(nsIPrincipal* aPrincipal,
                      const nsACString& tables,
@@ -103,22 +138,18 @@ private:
 
   // Close db connection and join the background thread if it exists.
   nsresult Shutdown();
 
   // Check if the key is on a known-clean host.
   nsresult CheckClean(const nsACString &lookupKey,
                       bool *clean);
 
-  // Read everything into mGethashTables and mDisallowCompletionTables
   nsresult ReadTablesFromPrefs();
 
-  // Build a comma-separated list of tables to check
-  void BuildTables(bool trackingProtectionEnabled, nsCString& tables);
-
   RefPtr<nsUrlClassifierDBServiceWorker> mWorker;
   RefPtr<UrlClassifierDBServiceWorkerProxy> mWorkerProxy;
 
   nsInterfaceHashtable<nsCStringHashKey, nsIUrlClassifierHashCompleter> mCompleters;
 
   // TRUE if the nsURIClassifier implementation should check for malware
   // uris on document loads.
   bool mCheckMalware;
@@ -138,16 +169,20 @@ private:
   bool mInUpdate;
 
   // The list of tables that can use the default hash completer object.
   nsTArray<nsCString> mGethashTables;
 
   // The list of tables that should never be hash completed.
   nsTArray<nsCString> mDisallowCompletionsTables;
 
+  // Comma-separated list of tables to use in lookups.
+  nsCString mTrackingProtectionTables;
+  nsCString mBaseTables;
+
   // Thread that we do the updates on.
   static nsIThread* gDbBackgroundThread;
 };
 
 class nsUrlClassifierDBServiceWorker final : public nsIUrlClassifierDBService
 {
 public:
   nsUrlClassifierDBServiceWorker();