Bug 1522265 - Moving malware, phishing and blocked URIs to features - part 3 - DBService updated, r=dimi
☠☠ backed out by 98040e23391e ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 29 Jan 2019 10:11:34 +0100
changeset 455943 38b824df9d028df7a3e3e8e5e11e35b069056054
parent 455942 6085d51681f8c763132973f65f305f1f15058135
child 455944 38b4179568c71c380cb83d26c54d4f088a7295d3
push id35463
push usershindli@mozilla.com
push dateTue, 29 Jan 2019 21:38:17 +0000
treeherdermozilla-central@4440fbf71c72 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdimi
bugs1522265
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1522265 - Moving malware, phishing and blocked URIs to features - part 3 - DBService updated, r=dimi
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.h
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -399,40 +399,16 @@ void Classifier::TableRequest(nsACString
     aResult.Append(metadata);
   }
 
   // Update the TableRequest result in-memory cache.
   mTableRequestResult = aResult;
   mIsTableRequestResultOutdated = false;
 }
 
-nsresult Classifier::CheckURI(const nsACString& aSpec,
-                              const nsTArray<nsCString>& aTables,
-                              LookupResultArray& aResults) {
-  Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_CHECK_TIME> timer;
-
-  // Get the set of fragments based on the url. This is necessary because we
-  // only look up at most 5 URLs per aSpec, even if aSpec has more than 5
-  // components.
-  nsTArray<nsCString> fragments;
-  nsresult rv = LookupCache::GetLookupFragments(aSpec, &fragments);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  LookupCacheArray cacheArray;
-  for (const nsCString& table : aTables) {
-    LookupResultArray results;
-    rv = CheckURIFragments(fragments, table, results);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    aResults.AppendElements(results);
-  }
-
-  return NS_OK;
-}
-
 nsresult Classifier::CheckURIFragments(
     const nsTArray<nsCString>& aSpecFragments, const nsACString& aTable,
     LookupResultArray& aResults) {
   // A URL can form up to 30 different fragments
   MOZ_ASSERT(aSpecFragments.Length() <=
              (MAX_HOST_COMPONENTS * (MAX_PATH_COMPONENTS + 2)));
 
   LOG(("Checking table %s", aTable.BeginReading()));
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -52,22 +52,16 @@ class Classifier {
   void TableRequest(nsACString& aResult);
 
   /*
    * Get all tables that we know about.
    */
   nsresult ActiveTables(nsTArray<nsCString>& aTables) const;
 
   /**
-   * Check a URL against the specified tables.
-   */
-  nsresult CheckURI(const nsACString& aSpec, const nsTArray<nsCString>& tables,
-                    LookupResultArray& aResults);
-
-  /**
    * Check URL fragments against a specified table.
    * The fragments should be generated by |LookupCache::GetLookupFragments|
    */
   nsresult CheckURIFragments(const nsTArray<nsCString>& aSpecFragments,
                              const nsACString& table,
                              LookupResultArray& aResults);
 
   /**
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -88,31 +88,30 @@ nsresult TablesToResponse(const nsACStri
     return NS_ERROR_BLOCKED_URI;
   }
   return NS_OK;
 }
 
 }  // namespace safebrowsing
 }  // namespace mozilla
 
-namespace {
-
 // This class holds a list of features, their tables, and it stores the lookup
 // results.
-class FeatureHolder final {
+class nsUrlClassifierDBService::FeatureHolder final {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FeatureHolder);
 
   // In order to avoid multiple lookup for the same table, we have a special
   // array for tables and their results. The Features are stored in a separate
   // array together with the references to their tables.
 
   class TableData {
    public:
-    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FeatureHolder::TableData);
+    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(
+        nsUrlClassifierDBService::FeatureHolder::TableData);
 
     explicit TableData(const nsACString& aTable) : mTable(aTable) {}
 
     nsCString mTable;
     LookupResultArray mResults;
 
    private:
     ~TableData() = default;
@@ -153,16 +152,20 @@ class FeatureHolder final {
     return holder.forget();
   }
 
   nsresult DoLocalLookup(const nsACString& aSpec,
                          nsUrlClassifierDBServiceWorker* aWorker) {
     MOZ_ASSERT(!NS_IsMainThread());
     MOZ_ASSERT(aWorker);
 
+    mozilla::Telemetry::AutoTimer<
+        mozilla::Telemetry::URLCLASSIFIER_CL_CHECK_TIME>
+        timer;
+
     // Get the set of fragments based on the url. This is necessary because we
     // only look up at most 5 URLs per aSpec, even if aSpec has more than 5
     // components.
     nsTArray<nsCString> fragments;
     nsresult rv = LookupCache::GetLookupFragments(aSpec, &fragments);
     NS_ENSURE_SUCCESS(rv, rv);
 
     for (TableData* tableData : mTableData) {
@@ -201,16 +204,30 @@ class FeatureHolder final {
 
       RefPtr<mozilla::net::UrlClassifierFeatureResult> result =
           new mozilla::net::UrlClassifierFeatureResult(
               mURI, featureData.mFeature, list);
       aResults.AppendElement(result);
     }
   }
 
+  mozilla::UniquePtr<LookupResultArray> GetTableResults() const {
+    mozilla::UniquePtr<LookupResultArray> results =
+        mozilla::MakeUnique<LookupResultArray>();
+    if (NS_WARN_IF(!results)) {
+      return nullptr;
+    }
+
+    for (TableData* tableData : mTableData) {
+      results->AppendElements(tableData->mResults);
+    }
+
+    return results;
+  }
+
  private:
   explicit FeatureHolder(nsIURI* aURI) : mURI(aURI) {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
   ~FeatureHolder() {
     for (FeatureData& featureData : mFeatureData) {
       NS_ReleaseOnMainThreadSystemGroup("FeatureHolder:mFeatureData",
@@ -232,18 +249,16 @@ class FeatureHolder final {
     return tableData;
   }
 
   nsCOMPtr<nsIURI> mURI;
   nsTArray<FeatureData> mFeatureData;
   nsTArray<RefPtr<TableData>> mTableData;
 };
 
-}  // namespace
-
 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() \
@@ -289,30 +304,34 @@ nsresult nsUrlClassifierDBServiceWorker:
   mDBService = aDBService;
 
   ResetUpdate();
 
   return NS_OK;
 }
 
 nsresult nsUrlClassifierDBServiceWorker::QueueLookup(
-    const nsACString& spec, const nsACString& tables,
-    nsIUrlClassifierLookupCallback* callback) {
+    const nsACString& aKey,
+    nsUrlClassifierDBService::FeatureHolder* aFeatureHolder,
+    nsIUrlClassifierLookupCallback* aCallback) {
+  MOZ_ASSERT(aFeatureHolder);
+  MOZ_ASSERT(aCallback);
+
   MutexAutoLock lock(mPendingLookupLock);
   if (gShuttingDownThread) {
     return NS_ERROR_ABORT;
   }
 
   PendingLookup* lookup = mPendingLookups.AppendElement(fallible);
-  if (!lookup) return NS_ERROR_OUT_OF_MEMORY;
+  if (NS_WARN_IF(!lookup)) return NS_ERROR_OUT_OF_MEMORY;
 
   lookup->mStartTime = TimeStamp::Now();
-  lookup->mKey = spec;
-  lookup->mCallback = callback;
-  lookup->mTables = tables;
+  lookup->mKey = aKey;
+  lookup->mCallback = aCallback;
+  lookup->mFeatureHolder = aFeatureHolder;
 
   return NS_OK;
 }
 
 nsresult nsUrlClassifierDBServiceWorker::DoSingleLocalLookupWithURIFragments(
     const nsTArray<nsCString>& aSpecFragments, const nsACString& aTable,
     LookupResultArray& aResults) {
   if (gShuttingDownThread) {
@@ -333,91 +352,58 @@ nsresult nsUrlClassifierDBServiceWorker:
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   LOG(("Found %zu results.", aResults.Length()));
   return NS_OK;
 }
 
-nsresult nsUrlClassifierDBServiceWorker::DoLocalLookupWithURI(
-    const nsACString& aSpec, const nsTArray<nsCString>& aTables,
-    LookupResultArray& aResults) {
-  if (gShuttingDownThread) {
-    return NS_ERROR_ABORT;
-  }
-
-  MOZ_ASSERT(
-      !NS_IsMainThread(),
-      "DoSingleLocalLookupWithURIFragments must be on background thread");
-
-  // Bail if we haven't been initialized on the background thread.
-  if (!mClassifier) {
-    return NS_ERROR_NOT_AVAILABLE;
-  }
-
-  nsresult rv = mClassifier->CheckURI(aSpec, aTables, aResults);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  LOG(("Found %zu results.", aResults.Length()));
-  return NS_OK;
-}
-
 /**
  * Lookup up a key in the database is a two step process:
  *
  * a) First we look for any Entries in the database that might apply to this
  *    url.  For each URL there are one or two possible domain names to check:
  *    the two-part domain name (example.com) and the three-part name
  *    (www.example.com).  We check the database for both of these.
  * b) If we find any entries, we check the list of fragments for that entry
  *    against the possible subfragments of the URL as described in the
  *    "Simplified Regular Expression Lookup" section of the protocol doc.
  */
 nsresult nsUrlClassifierDBServiceWorker::DoLookup(
-    const nsACString& spec, const nsACString& tables,
+    const nsACString& spec,
+    nsUrlClassifierDBService::FeatureHolder* aFeatureHolder,
     nsIUrlClassifierLookupCallback* c) {
   if (gShuttingDownThread) {
     c->LookupComplete(nullptr);
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   PRIntervalTime clockStart = 0;
   if (LOG_ENABLED()) {
     clockStart = PR_IntervalNow();
   }
 
-  UniquePtr<LookupResultArray> results = MakeUnique<LookupResultArray>();
-  if (!results) {
-    c->LookupComplete(nullptr);
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  nsTArray<nsCString> tableArray;
-  Classifier::SplitTables(tables, tableArray);
-
-  nsresult rv = DoLocalLookupWithURI(spec, tableArray, *results);
-  if (NS_FAILED(rv)) {
-    MOZ_ASSERT(
-        results->IsEmpty(),
-        "DoLocalLookupWithURI() should not return any results if it fails.");
-    c->LookupComplete(nullptr);
-    return rv;
-  }
-
-  LOG(("Found %zu results.", results->Length()));
+  nsresult rv = aFeatureHolder->DoLocalLookup(spec, this);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   if (LOG_ENABLED()) {
     PRIntervalTime clockEnd = PR_IntervalNow();
     LOG(("query took %dms\n",
          PR_IntervalToMilliseconds(clockEnd - clockStart)));
   }
 
+  UniquePtr<LookupResultArray> results = aFeatureHolder->GetTableResults();
+  if (NS_WARN_IF(!results)) {
+    c->LookupComplete(nullptr);
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+
+  LOG(("Found %zu results.", results->Length()));
+
   for (const RefPtr<const LookupResult> lookupResult : *results) {
     if (!lookupResult->Confirmed() &&
         mDBService->CanComplete(lookupResult->mTableName)) {
       // We're going to be doing a gethash request, add some extra entries.
       // Note that we cannot pass the first two by reference, because we
       // add to completes, which can cause completes to reallocate and move.
       AddNoise(lookupResult->hash.fixedLengthPrefix, lookupResult->mTableName,
                mGethashNoise, *results);
@@ -437,17 +423,17 @@ nsresult nsUrlClassifierDBServiceWorker:
   }
 
   MutexAutoLock lock(mPendingLookupLock);
   while (mPendingLookups.Length() > 0) {
     PendingLookup lookup = mPendingLookups[0];
     mPendingLookups.RemoveElementAt(0);
     {
       MutexAutoUnlock unlock(mPendingLookupLock);
-      DoLookup(lookup.mKey, lookup.mTables, lookup.mCallback);
+      DoLookup(lookup.mKey, lookup.mFeatureHolder, lookup.mCallback);
     }
     double lookupTime = (TimeStamp::Now() - lookup.mStartTime).ToMilliseconds();
     Telemetry::Accumulate(Telemetry::URLCLASSIFIER_LOOKUP_TIME_2,
                           static_cast<uint32_t>(lookupTime));
   }
 
   return NS_OK;
 }
@@ -1612,82 +1598,25 @@ NS_INTERFACE_MAP_END
     }
   } else {
     // Already exists, just add a ref
     NS_ADDREF(sUrlClassifierDBService);  // addref the return result
   }
   return sUrlClassifierDBService;
 }
 
-nsUrlClassifierDBService::nsUrlClassifierDBService()
-    : mCheckMalware(CHECK_MALWARE_DEFAULT),
-      mCheckPhishing(CHECK_PHISHING_DEFAULT),
-      mCheckBlockedURIs(CHECK_BLOCKED_DEFAULT),
-      mInUpdate(false) {}
+nsUrlClassifierDBService::nsUrlClassifierDBService() : mInUpdate(false) {}
 
 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);
-
-  nsAutoCString allTables;
+nsresult nsUrlClassifierDBService::ReadDisallowCompletionsTablesFromPrefs() {
   nsAutoCString tables;
 
-  mBaseTables.Truncate();
-
-  Preferences::GetCString(PHISH_TABLE_PREF, allTables);
-  if (mCheckPhishing) {
-    AppendTables(allTables, mBaseTables);
-  }
-
-  Preferences::GetCString(MALWARE_TABLE_PREF, 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);
-  AppendTables(tables, allTables);
-
-  Preferences::GetCString(DOWNLOAD_ALLOW_TABLE_PREF, tables);
-  AppendTables(tables, allTables);
-
-  Preferences::GetCString(PASSWORD_ALLOW_TABLE_PREF, tables);
-  AppendTables(tables, allTables);
-
-  Preferences::GetCString(TRACKING_TABLE_PREF, tables);
-  AppendTables(tables, allTables);
-
-  Preferences::GetCString(TRACKING_WHITELIST_TABLE_PREF, tables);
-  AppendTables(tables, allTables);
-
-  Classifier::SplitTables(allTables, mGethashTables);
-
   Preferences::GetCString(DISALLOW_COMPLETION_TABLE_PREF, tables);
   Classifier::SplitTables(tables, mDisallowCompletionsTables);
 
   return NS_OK;
 }
 
 nsresult nsUrlClassifierDBService::Init() {
   MOZ_ASSERT(NS_IsMainThread(), "Must initialize DB service on main thread");
@@ -1713,17 +1642,17 @@ nsresult nsUrlClassifierDBService::Init(
       return NS_OK;
     default:
       // No other process type is supported!
       return NS_ERROR_NOT_AVAILABLE;
   }
 
   sGethashNoise =
       Preferences::GetUint(GETHASH_NOISE_PREF, GETHASH_NOISE_DEFAULT);
-  ReadTablesFromPrefs();
+  ReadDisallowCompletionsTablesFromPrefs();
   nsresult rv;
 
   {
     // Force nsIUrlClassifierUtils loading on main thread.
     nsCOMPtr<nsIUrlClassifierUtils> dummy =
         do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -1766,87 +1695,124 @@ nsresult nsUrlClassifierDBService::Init(
   nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
   if (!observerService) return NS_ERROR_FAILURE;
 
   // 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::AddUintVarCache(&sGethashNoise, GETHASH_NOISE_PREF,
                                GETHASH_NOISE_DEFAULT);
-
-  for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
-    Preferences::AddStrongObserver(this, kObservedPrefs[i]);
-  }
+  Preferences::AddStrongObserver(this, DISALLOW_COMPLETION_TABLE_PREF);
 
   return NS_OK;
 }
 
 // nsChannelClassifier is the only consumer of this interface.
 NS_IMETHODIMP
 nsUrlClassifierDBService::Classify(nsIPrincipal* aPrincipal,
                                    nsIEventTarget* aEventTarget,
-                                   nsIURIClassifierCallback* c, bool* result) {
+                                   nsIURIClassifierCallback* c, bool* aResult) {
   NS_ENSURE_ARG(aPrincipal);
+  NS_ENSURE_ARG(aResult);
+
+  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
+    *aResult = false;
+    return NS_OK;
+  }
+
+  nsCOMPtr<nsIPermissionManager> permissionManager =
+      services::GetPermissionManager();
+  if (NS_WARN_IF(!permissionManager)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  uint32_t perm;
+  nsresult rv = permissionManager->TestPermissionFromPrincipal(
+      aPrincipal, "safe-browsing", &perm);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (perm == nsIPermissionManager::ALLOW_ACTION) {
+    *aResult = false;
+    return NS_OK;
+  }
 
   if (XRE_IsContentProcess()) {
     using namespace mozilla::dom;
 
     ContentChild* content = ContentChild::GetSingleton();
     MOZ_ASSERT(content);
 
     auto actor = static_cast<URLClassifierChild*>(
-        content->AllocPURLClassifierChild(IPC::Principal(aPrincipal), result));
+        content->AllocPURLClassifierChild(IPC::Principal(aPrincipal), aResult));
     MOZ_ASSERT(actor);
 
     if (aEventTarget) {
       content->SetEventTargetForActor(actor, aEventTarget);
     } else {
       // In the case null event target we should use systemgroup event target
       NS_WARNING(
           ("Null event target, we should use SystemGroup to do labelling"));
       nsCOMPtr<nsIEventTarget> systemGroupEventTarget =
           mozilla::SystemGroup::EventTargetFor(mozilla::TaskCategory::Other);
       content->SetEventTargetForActor(actor, systemGroupEventTarget);
     }
     if (!content->SendPURLClassifierConstructor(
-            actor, IPC::Principal(aPrincipal), result)) {
-      *result = false;
+            actor, IPC::Principal(aPrincipal), aResult)) {
+      *aResult = false;
       return NS_ERROR_FAILURE;
     }
 
     actor->SetCallback(c);
     return NS_OK;
   }
 
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
-  if (!(mCheckMalware || mCheckPhishing || mCheckBlockedURIs)) {
-    *result = false;
+  nsTArray<RefPtr<nsIUrlClassifierFeature>> features;
+  mozilla::net::UrlClassifierFeatureFactory::GetFeaturesNoChannel(features);
+  if (features.IsEmpty()) {
+    *aResult = false;
     return NS_OK;
   }
 
+  nsCOMPtr<nsIURI> uri;
+  rv = aPrincipal->GetURI(getter_AddRefs(uri));
+  NS_ENSURE_SUCCESS(rv, rv);
+  NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
+
+  // Let's keep the features alive and release them on the correct thread.
+  RefPtr<FeatureHolder> holder =
+      FeatureHolder::Create(uri, features, nsIUrlClassifierFeature::blacklist);
+  if (NS_WARN_IF(!holder)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  uri = NS_GetInnermostURI(uri);
+  NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
+
+  nsAutoCString key;
+  // Canonicalize the url
+  nsCOMPtr<nsIUrlClassifierUtils> utilsService =
+      do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
+  rv = utilsService->GetKeyForURI(uri, key);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   RefPtr<nsUrlClassifierClassifyCallback> callback =
       new (fallible) nsUrlClassifierClassifyCallback(c);
-
-  if (!callback) return NS_ERROR_OUT_OF_MEMORY;
-
-  nsresult rv = LookupURI(aPrincipal, mBaseTables, 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;
+  if (NS_WARN_IF(!callback)) {
+    return NS_ERROR_OUT_OF_MEMORY;
   }
+
+  // The rest is done async.
+  rv = LookupURI(key, holder, callback);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  *aResult = true;
   return NS_OK;
 }
 
 class ThreatHitReportListener final : public nsIStreamListener {
  public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
@@ -2066,84 +2032,78 @@ nsUrlClassifierDBService::SendThreatHitR
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::Lookup(nsIPrincipal* aPrincipal,
                                  const nsACString& tables,
                                  nsIUrlClassifierCallback* c) {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
-  bool dummy;
-  return LookupURI(aPrincipal, tables, c, true, &dummy);
-}
-
-nsresult nsUrlClassifierDBService::LookupURI(nsIPrincipal* aPrincipal,
-                                             const nsACString& tables,
-                                             nsIUrlClassifierCallback* c,
-                                             bool forceLookup,
-                                             bool* didLookup) {
-  NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
-  NS_ENSURE_ARG(aPrincipal);
-
   if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
-    *didLookup = false;
+    // FIXME: we don't call 'c' here!
     return NS_OK;
   }
 
+  nsTArray<nsCString> tableArray;
+  Classifier::SplitTables(tables, tableArray);
+
+  nsCOMPtr<nsIUrlClassifierFeature> feature;
+  nsresult rv =
+      CreateFeatureWithTables(NS_LITERAL_CSTRING("lookup"), tableArray,
+                              nsTArray<nsCString>(), getter_AddRefs(feature));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsCOMPtr<nsIURI> uri;
-  nsresult rv = aPrincipal->GetURI(getter_AddRefs(uri));
+  rv = aPrincipal->GetURI(getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
 
+  nsTArray<RefPtr<nsIUrlClassifierFeature>> features;
+  features.AppendElement(feature.get());
+
+  // Let's keep the features alive and release them on the correct thread.
+  RefPtr<FeatureHolder> holder =
+      FeatureHolder::Create(uri, features, nsIUrlClassifierFeature::blacklist);
+  if (NS_WARN_IF(!holder)) {
+    return NS_ERROR_FAILURE;
+  }
+
   uri = NS_GetInnermostURI(uri);
   NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE);
 
   nsAutoCString key;
   // Canonicalize the url
   nsCOMPtr<nsIUrlClassifierUtils> utilsService =
       do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
   rv = utilsService->GetKeyForURI(uri, key);
-  if (NS_FAILED(rv)) return rv;
-
-  if (forceLookup) {
-    *didLookup = true;
-  } else {
-    nsCOMPtr<nsIPermissionManager> permissionManager =
-        services::GetPermissionManager();
-    if (NS_WARN_IF(!permissionManager)) {
-      return NS_ERROR_FAILURE;
-    }
-
-    uint32_t perm;
-    rv = permissionManager->TestPermissionFromPrincipal(aPrincipal,
-                                                        "safe-browsing", &perm);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    bool clean = (perm == nsIPermissionManager::ALLOW_ACTION);
-    *didLookup = !clean;
-    if (clean) {
-      return NS_OK;
-    }
-  }
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return LookupURI(key, holder, c);
+}
+
+nsresult nsUrlClassifierDBService::LookupURI(
+    const nsACString& aKey, FeatureHolder* aHolder,
+    nsIUrlClassifierCallback* aCallback) {
+  MOZ_ASSERT(aHolder);
+  MOZ_ASSERT(aCallback);
+
+  NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   // Create an nsUrlClassifierLookupCallback object.  This object will
   // take care of confirming partial hash matches if necessary before
   // calling the client's callback.
   nsCOMPtr<nsIUrlClassifierLookupCallback> callback =
-      new (fallible) nsUrlClassifierLookupCallback(this, c);
-  if (!callback) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+      new nsUrlClassifierLookupCallback(this, aCallback);
 
   nsCOMPtr<nsIUrlClassifierLookupCallback> proxyCallback =
       new UrlClassifierLookupCallbackProxy(callback);
 
   // Queue this lookup and call the lookup function to flush the queue if
   // necessary.
-  rv = mWorker->QueueLookup(key, tables, proxyCallback);
+  nsresult rv = mWorker->QueueLookup(aKey, aHolder, proxyCallback);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // This seems to just call HandlePendingLookups.
   nsAutoCString dummy;
   return mWorkerProxy->Lookup(nullptr, dummy, nullptr);
 }
 
 NS_IMETHODIMP
@@ -2302,18 +2262,17 @@ nsUrlClassifierDBService::GetCacheInfo(
 nsresult nsUrlClassifierDBService::CacheCompletions(
     const ConstCacheResultArray& results) {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   return mWorkerProxy->CacheCompletions(results);
 }
 
 bool nsUrlClassifierDBService::CanComplete(const nsACString& aTableName) {
-  return mGethashTables.Contains(aTableName) &&
-         !mDisallowCompletionsTables.Contains(aTableName);
+  return !mDisallowCompletionsTables.Contains(aTableName);
 }
 
 bool nsUrlClassifierDBService::GetCompleter(
     const nsACString& tableName, nsIUrlClassifierHashCompleter** completer) {
   // If we have specified a completer, go ahead and query it. This is only
   // used by tests.
   if (mCompleters.Get(tableName, completer)) {
     return true;
@@ -2327,24 +2286,17 @@ bool nsUrlClassifierDBService::GetComple
   return NS_SUCCEEDED(
       CallGetService(NS_URLCLASSIFIERHASHCOMPLETER_CONTRACTID, completer));
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::Observe(nsISupports* aSubject, const char* aTopic,
                                   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 (kObservedPrefs.Contains(NS_ConvertUTF16toUTF8(aData))) {
-      ReadTablesFromPrefs();
-    }
+    ReadDisallowCompletionsTablesFromPrefs();
   } else if (!strcmp(aTopic, "quit-application")) {
     // Tell the update thread to finish as soon as possible.
     gShuttingDownThread = true;
 
     // The code in ::Shutdown() is run on a 'profile-before-change' event and
     // ensures that objects are freed by blocking on this freeing.
     // We can however speed up the shutdown time by using the worker thread to
     // release, in an earlier event, any objects that cannot affect an ongoing
@@ -2384,19 +2336,17 @@ nsresult nsUrlClassifierDBService::Shutd
   }
 
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_SHUTDOWN_TIME> timer;
 
   mCompleters.Clear();
 
   nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
   if (prefs) {
-    for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
-      prefs->RemoveObserver(kObservedPrefs[i], this);
-    }
+    prefs->RemoveObserver(DISALLOW_COMPLETION_TABLE_PREF, 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
@@ -38,36 +38,18 @@
 #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"
-#define PASSWORD_ALLOW_TABLE_PREF "urlclassifier.passwordAllowTable"
 
 using namespace mozilla::safebrowsing;
 
 class nsUrlClassifierDBServiceWorker;
 class nsIThread;
 class nsIURI;
 class UrlClassifierDBServiceWorkerProxy;
 
@@ -91,16 +73,18 @@ class AsyncUrlChannelClassifier;
 // calls to the background thread.
 class nsUrlClassifierDBService final : public nsIUrlClassifierDBService,
                                        public nsIURIClassifier,
                                        public nsIUrlClassifierInfo,
                                        public nsIObserver {
   friend class mozilla::net::AsyncUrlChannelClassifier;
 
  public:
+  class FeatureHolder;
+
   // This is thread safe. It throws an exception if the thread is busy.
   nsUrlClassifierDBService();
 
   nsresult Init();
 
   static nsUrlClassifierDBService* GetInstance(nsresult* result);
 
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_URLCLASSIFIERDBSERVICE_CID)
@@ -121,119 +105,84 @@ class nsUrlClassifierDBService final : p
 
   static bool ShutdownHasStarted();
 
  private:
   // This method is used only by AsyncUrlChannelClassifier. If you want to use
   // it, please contact a safebrowsing/URL-Classifier peer.
   static nsUrlClassifierDBServiceWorker* GetWorker();
 
-  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,
-                     nsIUrlClassifierCallback* c, bool forceCheck,
-                     bool* didCheck);
+  nsresult LookupURI(const nsACString& aKey, FeatureHolder* aHolder,
+                     nsIUrlClassifierCallback* c);
 
   // Post an event to worker thread to release objects when receive
   // 'quit-application'
   nsresult PreShutdown();
 
   // Close db connection and join the background thread if it exists.
   nsresult Shutdown();
 
-  nsresult ReadTablesFromPrefs();
+  nsresult ReadDisallowCompletionsTablesFromPrefs();
 
   // This method checks if the classification can be done just using
   // preferences. It returns true if the operation has been completed.
   bool AsyncClassifyLocalWithFeaturesUsingPreferences(
       nsIURI* aURI, const nsTArray<RefPtr<nsIUrlClassifierFeature>>& aFeatures,
       nsIUrlClassifierFeature::listType aListType,
       nsIUrlClassifierFeatureCallback* aCallback);
 
   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;
-
-  // TRUE if the nsURIClassifier implementation should check for phishing
-  // uris on document loads.
-  bool mCheckPhishing;
-
-  // TRUE if the nsURIClassifier implementation should check for blocked
-  // uris on document loads.
-  bool mCheckBlockedURIs;
-
   // TRUE if a BeginUpdate() has been called without an accompanying
   // CancelUpdate()/FinishUpdate().  This is used to prevent competing
   // updates, not to determine whether an update is still being
   // processed.
   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 mBaseTables;
-
   // Thread that we do the updates on.
   static nsIThread* gDbBackgroundThread;
 };
 
 class nsUrlClassifierDBServiceWorker final : public nsIUrlClassifierDBService {
  public:
   nsUrlClassifierDBServiceWorker();
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIURLCLASSIFIERDBSERVICE
 
   nsresult Init(uint32_t aGethashNoise, nsCOMPtr<nsIFile> aCacheDir,
                 nsUrlClassifierDBService* aDBService);
 
   // Queue a lookup for the worker to perform, called in the main thread.
-  // tables is a comma-separated list of tables to query
-  nsresult QueueLookup(const nsACString& lookupKey, const nsACString& tables,
-                       nsIUrlClassifierLookupCallback* callback);
+  nsresult QueueLookup(const nsACString& aLookupKey,
+                       nsUrlClassifierDBService::FeatureHolder* aFeatureHolder,
+                       nsIUrlClassifierLookupCallback* aLallback);
 
   // Handle any queued-up lookups.  We call this function during long-running
   // update operations to prevent lookups from blocking for too long.
   nsresult HandlePendingLookups();
 
   // Perform a blocking classifier lookup for a given url fragments set.
   // Can be called on either the main thread or the worker thread.
   nsresult DoSingleLocalLookupWithURIFragments(
       const nsTArray<nsCString>& aSpecFragments, const nsACString& aTable,
       LookupResultArray& aResults);
-  nsresult DoLocalLookupWithURI(const nsACString& aSpec,
-                                const nsTArray<nsCString>& aTables,
-                                LookupResultArray& aResults);
 
   // Open the DB connection
   nsresult GCC_MANGLING_WORKAROUND OpenDb();
 
   // Provide a way to forcibly close the db connection.
   nsresult GCC_MANGLING_WORKAROUND CloseDb();
 
   nsresult GCC_MANGLING_WORKAROUND PreShutdown();
@@ -270,17 +219,18 @@ class nsUrlClassifierDBServiceWorker fin
 
   // Reset the in-progress update stream
   void ResetStream();
 
   // Reset the in-progress update
   void ResetUpdate();
 
   // Perform a classifier lookup for a given url.
-  nsresult DoLookup(const nsACString& spec, const nsACString& tables,
+  nsresult DoLookup(const nsACString& spec,
+                    nsUrlClassifierDBService::FeatureHolder* aFeatureHolder,
                     nsIUrlClassifierLookupCallback* c);
 
   nsresult AddNoise(const Prefix aPrefix, const nsCString tableName,
                     uint32_t aCount, LookupResultArray& results);
 
   nsresult CacheResultToTableUpdate(RefPtr<const CacheResult> aCacheResult,
                                     RefPtr<TableUpdate> aUpdate);
 
@@ -314,17 +264,17 @@ class nsUrlClassifierDBServiceWorker fin
   // Pending lookups are stored in a queue for processing.  The queue
   // is protected by mPendingLookupLock.
   mozilla::Mutex mPendingLookupLock;
 
   class PendingLookup {
    public:
     mozilla::TimeStamp mStartTime;
     nsCString mKey;
-    nsCString mTables;
+    RefPtr<nsUrlClassifierDBService::FeatureHolder> mFeatureHolder;
     nsCOMPtr<nsIUrlClassifierLookupCallback> mCallback;
   };
 
   // list of pending lookups
   nsTArray<PendingLookup> mPendingLookups;
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   // The raw update response for debugging.
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -98,39 +98,16 @@ NS_IMETHODIMP
 UrlClassifierDBServiceWorkerProxy::FinishStream() {
   nsCOMPtr<nsIRunnable> r =
       NewRunnableMethod("nsUrlClassifierDBServiceWorker::FinishStream", mTarget,
                         &nsUrlClassifierDBServiceWorker::FinishStream);
   return DispatchToWorkerThread(r);
 }
 
 NS_IMETHODIMP
-UrlClassifierDBServiceWorkerProxy::DoLocalLookupRunnable::Run() {
-  mTarget->DoLocalLookupWithURI(mSpec, mTables, mResults);
-  return NS_OK;
-}
-
-nsresult UrlClassifierDBServiceWorkerProxy::DoLocalLookupWithURI(
-    const nsACString& spec, const nsTArray<nsCString>& tables,
-    LookupResultArray& results) const
-
-{
-  // Run synchronously on background thread. NS_DISPATCH_SYNC does *not* do
-  // what we want -- it continues processing events on the main thread loop
-  // before the Dispatch returns.
-  nsCOMPtr<nsIRunnable> r =
-      new DoLocalLookupRunnable(mTarget, spec, tables, results);
-  nsIThread* t = nsUrlClassifierDBService::BackgroundThread();
-  if (!t) return NS_ERROR_FAILURE;
-
-  mozilla::SyncRunnable::DispatchToThread(t, r);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 UrlClassifierDBServiceWorkerProxy::FinishUpdate() {
   nsCOMPtr<nsIRunnable> r =
       NewRunnableMethod("nsUrlClassifierDBServiceWorker::FinishUpdate", mTarget,
                         &nsUrlClassifierDBServiceWorker::FinishUpdate);
   return DispatchToWorkerThread(r);
 }
 
 NS_IMETHODIMP
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -127,37 +127,16 @@ class UrlClassifierDBServiceWorkerProxy 
 
     NS_DECL_NSIRUNNABLE
 
    private:
     const RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
     const mozilla::safebrowsing::ConstCacheResultArray mEntries;
   };
 
-  class DoLocalLookupRunnable : public mozilla::Runnable {
-   public:
-    DoLocalLookupRunnable(nsUrlClassifierDBServiceWorker* aTarget,
-                          const nsACString& spec,
-                          const nsTArray<nsCString>& tables,
-                          mozilla::safebrowsing::LookupResultArray& results)
-        : mozilla::Runnable(
-              "UrlClassifierDBServiceWorkerProxy::DoLocalLookupRunnable"),
-          mTarget(aTarget),
-          mSpec(spec),
-          mTables(tables),
-          mResults(results) {}
-
-    NS_DECL_NSIRUNNABLE
-   private:
-    const RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
-    const nsCString mSpec;
-    const nsTArray<nsCString> mTables;
-    mozilla::safebrowsing::LookupResultArray& mResults;
-  };
-
   class ClearLastResultsRunnable : public mozilla::Runnable {
    public:
     explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
         : mozilla::Runnable(
               "UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable"),
           mTarget(aTarget) {}
 
     NS_DECL_NSIRUNNABLE
@@ -200,20 +179,16 @@ class UrlClassifierDBServiceWorkerProxy 
 
     NS_DECL_NSIRUNNABLE
    private:
     nsCOMPtr<nsIUrlClassifierCacheInfo> mCache;
     const nsMainThreadPtrHandle<nsIUrlClassifierGetCacheCallback> mCallback;
   };
 
  public:
-  nsresult DoLocalLookupWithURI(
-      const nsACString& spec, const nsTArray<nsCString>& tables,
-      mozilla::safebrowsing::LookupResultArray& results) const;
-
   nsresult OpenDb() const;
   nsresult CloseDb() const;
   nsresult PreShutdown() const;
 
   nsresult CacheCompletions(
       const mozilla::safebrowsing::ConstCacheResultArray& aEntries) const;
 
   nsresult GetCacheInfo(const nsACString& aTable,