Bug 1434206 - Keep LookupResult objects in smart pointers. r?gcp draft
authorFrancois Marier <francois@mozilla.com>
Tue, 29 May 2018 14:55:27 -0700
changeset 467876 4140cd9c67b0
parent 467874 c927a5205abe
push id181
push userfmarier@mozilla.com
push dateTue, 29 May 2018 22:16:07 +0000
reviewersgcp
bugs1434206
milestone62.0a1
Bug 1434206 - Keep LookupResult objects in smart pointers. r?gcp Replace raw pointers to LookupResult with RefPtrs and eplace the nsAutoPtr objects + raw pointers params with UniquePtrs. Also remove unnecessarily paranoid OOM checks when creating single LookupResult objects since those are pretty small.
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.h
toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -454,20 +454,19 @@ Classifier::Check(const nsACString& aSpe
       RefPtr<LookupCache> cache = cacheArray[i];
       bool has, confirmed;
       uint32_t matchLength;
 
       rv = cache->Has(lookupHash, &has, &matchLength, &confirmed);
       NS_ENSURE_SUCCESS(rv, rv);
 
       if (has) {
-        LookupResult *result = aResults.AppendElement(fallible);
-        if (!result) {
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
+        RefPtr<LookupResult> result = new LookupResult;
+        aResults.AppendElement(result);
+
         LOG(("Found a result in %s: %s",
              cache->TableName().get(),
              confirmed ? "confirmed." : "Not confirmed."));
 
         result->hash.complete = lookupHash;
         result->mConfirmed = confirmed;
         result->mTableName.Assign(cache->TableName());
         result->mPartialHashLength = confirmed ? COMPLETE_SIZE : matchLength;
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -26,16 +26,18 @@ namespace safebrowsing {
 #define MAX_PATH_COMPONENTS 4
 
 class LookupResult {
 public:
   LookupResult() : mNoise(false), mProtocolConfirmed(false),
                    mPartialHashLength(0), mConfirmed(false),
                    mProtocolV2(true) {}
 
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(LookupResult);
+
   // The fragment that matched in the LookupCache
   union {
     Prefix fixedLengthPrefix;
     Completion complete;
   } hash;
 
   const Completion &CompleteHash() const {
     MOZ_ASSERT(!mNoise);
@@ -78,19 +80,21 @@ public:
   nsCString mTableName;
 
   uint32_t mPartialHashLength;
 
   // True as long as this lookup is complete and hasn't expired.
   bool mConfirmed;
 
   bool mProtocolV2;
+private:
+  ~LookupResult() {}
 };
 
-typedef nsTArray<LookupResult> LookupResultArray;
+typedef nsTArray<RefPtr<LookupResult>> LookupResultArray;
 
 class CacheResult {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CacheResult);
 
   enum { V2, V4 };
 
   virtual int Ver() const = 0;
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -2,18 +2,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 
 %{C++
 #include "Entries.h"
 #include "LookupCache.h"
+#include "mozilla/UniquePtr.h"
 %}
-[ptr] native ResultArray(nsTArray<mozilla::safebrowsing::LookupResult>);
+native ResultArray(mozilla::UniquePtr<mozilla::safebrowsing::LookupResultArray>);
 
 interface nsIUrlClassifierHashCompleter;
 interface nsIPrincipal;
 
 // Interface for JS function callbacks
 [scriptable, function, uuid(4ca27b6b-a674-4b3d-ab30-d21e2da2dffb)]
 interface nsIUrlClassifierCallback : nsISupports {
   void handleEvent(in ACString value);
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -171,49 +171,47 @@ nsUrlClassifierDBServiceWorker::QueueLoo
   lookup->mTables = tables;
 
   return NS_OK;
 }
 
 nsresult
 nsUrlClassifierDBServiceWorker::DoLocalLookup(const nsACString& spec,
                                               const nsACString& tables,
-                                              LookupResultArray* results)
+                                              LookupResultArray& results)
 {
   if (gShuttingDownThread) {
     return NS_ERROR_ABORT;
   }
 
   MOZ_ASSERT(!NS_IsMainThread(), "DoLocalLookup must be on background thread");
-  if (!results) {
-    return NS_ERROR_FAILURE;
-  }
+
   // Bail if we haven't been initialized on the background thread.
   if (!mClassifier) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   // We ignore failures from Check because we'd rather return the
   // results that were found than fail.
-  mClassifier->Check(spec, tables, *results);
-
-  LOG(("Found %zu results.", results->Length()));
+  mClassifier->Check(spec, tables, results);
+
+  LOG(("Found %zu results.", results.Length()));
   return NS_OK;
 }
 
 static nsresult
-ProcessLookupResults(LookupResultArray* results, nsTArray<nsCString>& tables)
+ProcessLookupResults(const LookupResultArray& results, nsTArray<nsCString>& tables)
 {
   // Build the result array.
-  for (uint32_t i = 0; i < results->Length(); i++) {
-    LookupResult& result = results->ElementAt(i);
-    MOZ_ASSERT(!result.mNoise, "Lookup results should not have noise added");
-    LOG(("Found result from table %s", result.mTableName.get()));
-    if (tables.IndexOf(result.mTableName) == nsTArray<nsCString>::NoIndex) {
-      tables.AppendElement(result.mTableName);
+  for (auto iter = results.cbegin(); iter != results.cend(); ++iter) {
+    RefPtr<const LookupResult> result = *iter;
+    MOZ_ASSERT(!result->mNoise, "Lookup results should not have noise added");
+    LOG(("Found result from table %s", result->mTableName.get()));
+    if (tables.IndexOf(result->mTableName) == nsTArray<nsCString>::NoIndex) {
+      tables.AppendElement(result->mTableName);
     }
   }
   return NS_OK;
 }
 
 /**
  * Lookup up a key in the database is a two step process:
  *
@@ -235,55 +233,57 @@ nsUrlClassifierDBServiceWorker::DoLookup
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   PRIntervalTime clockStart = 0;
   if (LOG_ENABLED()) {
     clockStart = PR_IntervalNow();
   }
 
-  nsAutoPtr<LookupResultArray> results(new (fallible) LookupResultArray());
+  UniquePtr<LookupResultArray> results = MakeUnique<LookupResultArray>();
   if (!results) {
     c->LookupComplete(nullptr);
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  nsresult rv = DoLocalLookup(spec, tables, results);
+  nsresult rv = DoLocalLookup(spec, tables, *results);
   if (NS_FAILED(rv)) {
+    MOZ_ASSERT(results->IsEmpty(),
+               "DoLocalLookup() should not return any results if it fails.");
     c->LookupComplete(nullptr);
     return rv;
   }
 
   LOG(("Found %zu results.", results->Length()));
 
 
   if (LOG_ENABLED()) {
     PRIntervalTime clockEnd = PR_IntervalNow();
     LOG(("query took %dms\n",
          PR_IntervalToMilliseconds(clockEnd - clockStart)));
   }
 
-  for (uint32_t i = 0; i < results->Length(); i++) {
-    const LookupResult& lookupResult = results->ElementAt(i);
-
-    if (!lookupResult.Confirmed() &&
-        mDBService->CanComplete(lookupResult.mTableName)) {
+  for (auto iter = results->cbegin(); iter != results->cend(); ++iter) {
+    RefPtr<const LookupResult> lookupResult = *iter;
+
+    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, whicah can cause completes to reallocate and move.
-      AddNoise(lookupResult.hash.fixedLengthPrefix,
-               lookupResult.mTableName,
+      // add to completes, which can cause completes to reallocate and move.
+      AddNoise(lookupResult->hash.fixedLengthPrefix,
+               lookupResult->mTableName,
                mGethashNoise, *results);
       break;
     }
   }
 
   // At this point ownership of 'results' is handed to the callback.
-  c->LookupComplete(results.forget());
+  c->LookupComplete(std::move(results));
 
   return NS_OK;
 }
 
 nsresult
 nsUrlClassifierDBServiceWorker::HandlePendingLookups()
 {
   if (gShuttingDownThread) {
@@ -320,23 +320,21 @@ nsUrlClassifierDBServiceWorker::AddNoise
     return NS_OK;
   }
 
   PrefixArray noiseEntries;
   nsresult rv = mClassifier->ReadNoiseEntries(aPrefix, tableName,
                                               aCount, noiseEntries);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (uint32_t i = 0; i < noiseEntries.Length(); i++) {
-    LookupResult *result = results.AppendElement(fallible);
-    if (!result) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
-
-    result->hash.fixedLengthPrefix = noiseEntries[i];
+  for (auto iter = noiseEntries.cbegin(); iter != noiseEntries.cend(); ++iter) {
+    RefPtr<LookupResult> result = new LookupResult;
+    results.AppendElement(result);
+
+    result->hash.fixedLengthPrefix = *iter;
     result->mNoise = true;
     result->mPartialHashLength = PREFIX_SIZE; // Noise is always 4-byte,
     result->mTableName.Assign(tableName);
   }
 
   return NS_OK;
 }
 
@@ -1055,17 +1053,17 @@ public:
 private:
   ~nsUrlClassifierLookupCallback();
 
   nsresult HandleResults();
   nsresult ProcessComplete(RefPtr<CacheResult> aCacheResult);
   nsresult CacheMisses();
 
   RefPtr<nsUrlClassifierDBService> mDBService;
-  nsAutoPtr<LookupResultArray> mResults;
+  UniquePtr<LookupResultArray> mResults;
 
   // Completed results to send back to the worker for caching.
   ConstCacheResultArray mCacheResults;
 
   uint32_t mPendingCompletions;
   nsCOMPtr<nsIUrlClassifierCallback> mCallback;
 };
 
@@ -1077,69 +1075,69 @@ nsUrlClassifierLookupCallback::~nsUrlCla
 {
   if (mCallback) {
     NS_ReleaseOnMainThreadSystemGroup(
       "nsUrlClassifierLookupCallback::mCallback", mCallback.forget());
   }
 }
 
 NS_IMETHODIMP
-nsUrlClassifierLookupCallback::LookupComplete(nsTArray<LookupResult>* results)
+nsUrlClassifierLookupCallback::LookupComplete(UniquePtr<LookupResultArray> results)
 {
   NS_ASSERTION(mResults == nullptr,
                "Should only get one set of results per nsUrlClassifierLookupCallback!");
 
   if (!results) {
     HandleResults();
     return NS_OK;
   }
 
-  mResults = results;
+  mResults = std::move(results);
 
   // Check the results entries that need to be completed.
-  for (uint32_t i = 0; i < results->Length(); i++) {
-    LookupResult& result = results->ElementAt(i);
+  for (auto iter = mResults->cbegin(); iter != mResults->cend(); ++iter) {
+    RefPtr<LookupResult> result = *iter;
 
     // We will complete partial matches and matches that are stale.
-    if (!result.Confirmed()) {
+    if (!result->Confirmed()) {
       nsCOMPtr<nsIUrlClassifierHashCompleter> completer;
       nsCString gethashUrl;
       nsresult rv;
       nsCOMPtr<nsIUrlListManager> listManager = do_GetService(
         "@mozilla.org/url-classifier/listmanager;1", &rv);
       NS_ENSURE_SUCCESS(rv, rv);
-      rv = listManager->GetGethashUrl(result.mTableName, gethashUrl);
+      rv = listManager->GetGethashUrl(result->mTableName, gethashUrl);
       NS_ENSURE_SUCCESS(rv, rv);
       LOG(("The match from %s needs to be completed at %s",
-           result.mTableName.get(), gethashUrl.get()));
+           result->mTableName.get(), gethashUrl.get()));
       // gethashUrls may be empty in 2 cases: test tables, and on startup where
       // we may have found a prefix in an existing table before the listmanager
       // has registered the table. In the second case we should not call
       // complete.
       if ((!gethashUrl.IsEmpty() ||
-           StringBeginsWith(result.mTableName, NS_LITERAL_CSTRING("test"))) &&
-          mDBService->GetCompleter(result.mTableName,
+           StringBeginsWith(result->mTableName, NS_LITERAL_CSTRING("test"))) &&
+          mDBService->GetCompleter(result->mTableName,
                                    getter_AddRefs(completer))) {
 
         // Bug 1323953 - Send the first 4 bytes for completion no matter how
         // long we matched the prefix.
-        nsresult rv = completer->Complete(result.PartialHash(),
+        nsresult rv = completer->Complete(result->PartialHash(),
                                           gethashUrl,
-                                          result.mTableName,
+                                          result->mTableName,
                                           this);
         if (NS_SUCCEEDED(rv)) {
           mPendingCompletions++;
         }
       } else {
         // For tables with no hash completer, a complete hash match is
         // good enough, we'll consider it is valid.
-        if (result.Complete()) {
-          result.mConfirmed = true;
+        if (result->Complete()) {
+          result->mConfirmed = true;
           LOG(("Skipping completion in a table without a valid completer (%s).",
-               result.mTableName.get()));
+               result->mTableName.get()));
         } else {
           NS_WARNING("Partial match in a table without a valid completer, ignoring partial match.");
         }
       }
     }
   }
 
   LOG(("nsUrlClassifierLookupCallback::LookupComplete [%p] "
@@ -1239,28 +1237,30 @@ nsUrlClassifierLookupCallback::Completio
   }
 
   return ProcessComplete(result);
 }
 
 nsresult
 nsUrlClassifierLookupCallback::ProcessComplete(RefPtr<CacheResult> aCacheResult)
 {
+  NS_ENSURE_ARG_POINTER(mResults);
+
   // OK if this fails, we just won't cache the item.
   mCacheResults.AppendElement(aCacheResult, fallible);
 
   // Check if this matched any of our results.
-  for (uint32_t i = 0; i < mResults->Length(); i++) {
-    LookupResult& result = mResults->ElementAt(i);
+  for (auto iter = mResults->cbegin(); iter != mResults->cend(); ++iter) {
+    RefPtr<LookupResult> result = *iter;
 
     // Now, see if it verifies a lookup
-    if (!result.mNoise
-        && result.mTableName.Equals(aCacheResult->table)
-        && aCacheResult->findCompletion(result.CompleteHash())) {
-      result.mProtocolConfirmed = true;
+    if (!result->mNoise
+        && result->mTableName.Equals(aCacheResult->table)
+        && aCacheResult->findCompletion(result->CompleteHash())) {
+      result->mProtocolConfirmed = true;
     }
   }
 
   return NS_OK;
 }
 
 nsresult
 nsUrlClassifierLookupCallback::HandleResults()
@@ -1276,45 +1276,45 @@ nsUrlClassifierLookupCallback::HandleRes
   LOG(("nsUrlClassifierLookupCallback::HandleResults [%p, %zu results]",
        this, mResults->Length()));
 
   nsCOMPtr<nsIUrlClassifierClassifyCallback> classifyCallback =
     do_QueryInterface(mCallback);
 
   nsTArray<nsCString> tables;
   // Build a stringified list of result tables.
-  for (uint32_t i = 0; i < mResults->Length(); i++) {
-    LookupResult& result = mResults->ElementAt(i);
+  for (auto iter = mResults->cbegin(); iter != mResults->cend(); ++iter) {
+    RefPtr<const LookupResult> result = *iter;
 
     // Leave out results that weren't confirmed, as their existence on
     // the list can't be verified.  Also leave out randomly-generated
     // noise.
-    if (result.mNoise) {
+    if (result->mNoise) {
       LOG(("Skipping result %s from table %s (noise)",
-           result.PartialHashHex().get(), result.mTableName.get()));
+           result->PartialHashHex().get(), result->mTableName.get()));
       continue;
     }
 
-    if (!result.Confirmed()) {
+    if (!result->Confirmed()) {
       LOG(("Skipping result %s from table %s (not confirmed)",
-           result.PartialHashHex().get(), result.mTableName.get()));
+           result->PartialHashHex().get(), result->mTableName.get()));
       continue;
     }
 
     LOG(("Confirmed result %s from table %s",
-         result.PartialHashHex().get(), result.mTableName.get()));
-
-    if (tables.IndexOf(result.mTableName) == nsTArray<nsCString>::NoIndex) {
-      tables.AppendElement(result.mTableName);
+         result->PartialHashHex().get(), result->mTableName.get()));
+
+    if (tables.IndexOf(result->mTableName) == nsTArray<nsCString>::NoIndex) {
+      tables.AppendElement(result->mTableName);
     }
 
     if (classifyCallback) {
       nsCString fullHashString;
-      result.hash.complete.ToString(fullHashString);
-      classifyCallback->HandleResult(result.mTableName, fullHashString);
+      result->hash.complete.ToString(fullHashString);
+      classifyCallback->HandleResult(result->mTableName, fullHashString);
     }
   }
 
   // Some parts of this gethash request generated no hits at all.
   // Save the prefixes we checked to prevent repeated requests.
   CacheMisses();
 
   // This hands ownership of the cache results array back to the worker
@@ -1330,28 +1330,30 @@ nsUrlClassifierLookupCallback::HandleRes
   }
 
   return mCallback->HandleEvent(tableStr);
 }
 
 nsresult
 nsUrlClassifierLookupCallback::CacheMisses()
 {
-  for (uint32_t i = 0; i < mResults->Length(); i++) {
-    const LookupResult &result = mResults->ElementAt(i);
+  MOZ_ASSERT(mResults);
+
+  for (auto iter = mResults->cbegin(); iter != mResults->cend(); ++iter) {
+    RefPtr<const LookupResult> result = *iter;
     // Skip V4 because cache information is already included in the
     // fullhash response so we don't need to manually add it here.
-    if (!result.mProtocolV2 || result.Confirmed() || result.mNoise) {
+    if (!result->mProtocolV2 || result->Confirmed() || result->mNoise) {
       continue;
     }
 
     RefPtr<CacheResultV2> cacheResult = new CacheResultV2();
 
-    cacheResult->table = result.mTableName;
-    cacheResult->prefix = result.hash.fixedLengthPrefix;
+    cacheResult->table = result->mTableName;
+    cacheResult->prefix = result->hash.fixedLengthPrefix;
     cacheResult->miss = true;
     if (!mCacheResults.AppendElement(cacheResult, fallible)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
   return NS_OK;
 }
 
@@ -1839,26 +1841,24 @@ nsUrlClassifierDBService::AsyncClassifyL
     new nsMainThreadPtrHolder<nsIURIClassifierCallback>(
       "nsIURIClassifierCallback", aCallback));
 
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
     "nsUrlClassifierDBService::AsyncClassifyLocalWithTables",
     [worker, key, tables, callback, startTime]() -> void {
 
       nsCString matchedLists;
-      nsAutoPtr<LookupResultArray> results(new LookupResultArray());
-      if (results) {
-        nsresult rv = worker->DoLocalLookup(key, tables, results);
-        if (NS_SUCCEEDED(rv)) {
-          for (uint32_t i = 0; i < results->Length(); i++) {
-            if (i > 0) {
-              matchedLists.AppendLiteral(",");
-            }
-            matchedLists.Append(results->ElementAt(i).mTableName);
+      LookupResultArray results;
+      nsresult rv = worker->DoLocalLookup(key, tables, results);
+      if (NS_SUCCEEDED(rv)) {
+        for (uint32_t i = 0; i < results.Length(); i++) {
+          if (i > 0) {
+            matchedLists.AppendLiteral(",");
           }
+          matchedLists.Append(results[i]->mTableName);
         }
       }
 
       nsCOMPtr<nsIRunnable> cbRunnable = NS_NewRunnableFunction(
         "nsUrlClassifierDBService::AsyncClassifyLocalWithTables",
         [callback, matchedLists, startTime]() -> void {
           // Measure the time diff between calling and callback.
           AccumulateTimeDelta(Telemetry::URLCLASSIFIER_ASYNC_CLASSIFYLOCAL_TIME,
@@ -1913,20 +1913,17 @@ nsUrlClassifierDBService::ClassifyLocalW
 
   nsAutoCString key;
   // Canonicalize the url
   nsCOMPtr<nsIUrlClassifierUtils> utilsService =
     do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
   rv = utilsService->GetKeyForURI(uri, key);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsAutoPtr<LookupResultArray> results(new (fallible) LookupResultArray());
-  if (!results) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+  LookupResultArray results;
 
   // In unittests, we may not have been initalized, so don't crash.
   rv = mWorkerProxy->DoLocalLookup(key, aTables, results);
   if (NS_SUCCEEDED(rv)) {
     rv = ProcessLookupResults(results, aTableResults);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -206,17 +206,17 @@ public:
   // 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. Can be called on
   // either the main thread or the worker thread.
   nsresult DoLocalLookup(const nsACString& spec,
                          const nsACString& tables,
-                         LookupResultArray* results);
+                         LookupResultArray& results);
 
   // 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();
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -124,17 +124,17 @@ UrlClassifierDBServiceWorkerProxy::DoLoc
 {
   mTarget->DoLocalLookup(mSpec, mTables, mResults);
   return NS_OK;
 }
 
 nsresult
 UrlClassifierDBServiceWorkerProxy::DoLocalLookup(const nsACString& spec,
                                                  const nsACString& tables,
-                                                 LookupResultArray* results) const
+                                                 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)
@@ -282,27 +282,27 @@ UrlClassifierDBServiceWorkerProxy::GetCa
 
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS(UrlClassifierLookupCallbackProxy,
                   nsIUrlClassifierLookupCallback)
 
 NS_IMETHODIMP
-UrlClassifierLookupCallbackProxy::LookupComplete
-  (LookupResultArray * aResults)
+UrlClassifierLookupCallbackProxy::LookupComplete(UniquePtr<LookupResultArray> aResults)
 {
-  nsCOMPtr<nsIRunnable> r = new LookupCompleteRunnable(mTarget, aResults);
+  nsCOMPtr<nsIRunnable> r = new LookupCompleteRunnable(mTarget,
+                                                       std::move(aResults));
   return NS_DispatchToMainThread(r);
 }
 
 NS_IMETHODIMP
 UrlClassifierLookupCallbackProxy::LookupCompleteRunnable::Run()
 {
-  mTarget->LookupComplete(mResults);
+  mTarget->LookupComplete(std::move(mResults));
   return NS_OK;
 }
 
 NS_IMPL_ISUPPORTS(UrlClassifierCallbackProxy,
                   nsIUrlClassifierCallback)
 
 NS_IMETHODIMP
 UrlClassifierCallbackProxy::HandleEvent(const nsACString& aValue)
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -145,31 +145,31 @@ public:
   };
 
   class DoLocalLookupRunnable : public mozilla::Runnable
   {
   public:
     DoLocalLookupRunnable(nsUrlClassifierDBServiceWorker* aTarget,
                           const nsACString& spec,
                           const nsACString& tables,
-                          mozilla::safebrowsing::LookupResultArray* results)
+                          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 nsCString mTables;
-    mozilla::safebrowsing::LookupResultArray* const mResults;
+    mozilla::safebrowsing::LookupResultArray& mResults;
   };
 
   class ClearLastResultsRunnable : public mozilla::Runnable
   {
   public:
     explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
       : mozilla::Runnable(
           "UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable")
@@ -219,17 +219,17 @@ public:
   private:
     nsIUrlClassifierCacheInfo* mCache;
     const nsMainThreadPtrHandle<nsIUrlClassifierGetCacheCallback> mCallback;
   };
 
 public:
   nsresult DoLocalLookup(const nsACString& spec,
                          const nsACString& tables,
-                         mozilla::safebrowsing::LookupResultArray* results) const;
+                         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,
@@ -254,28 +254,28 @@ public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIURLCLASSIFIERLOOKUPCALLBACK
 
   class LookupCompleteRunnable : public mozilla::Runnable
   {
   public:
     LookupCompleteRunnable(
       const nsMainThreadPtrHandle<nsIUrlClassifierLookupCallback>& aTarget,
-      mozilla::safebrowsing::LookupResultArray* aResults)
+      mozilla::UniquePtr<mozilla::safebrowsing::LookupResultArray> aResults)
       : mozilla::Runnable(
           "UrlClassifierLookupCallbackProxy::LookupCompleteRunnable")
       , mTarget(aTarget)
-      , mResults(aResults)
+      , mResults(std::move(aResults))
     { }
 
     NS_DECL_NSIRUNNABLE
 
   private:
     const nsMainThreadPtrHandle<nsIUrlClassifierLookupCallback> mTarget;
-    mozilla::safebrowsing::LookupResultArray* const mResults;
+    mozilla::UniquePtr<mozilla::safebrowsing::LookupResultArray> mResults;
   };
 
 private:
   ~UrlClassifierLookupCallbackProxy() {}
 
   const nsMainThreadPtrHandle<nsIUrlClassifierLookupCallback> mTarget;
 };
 
--- a/toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
@@ -74,31 +74,31 @@ SetupLookupCacheV2(Classifier* classifie
 static void
 TestReadNoiseEntries(Classifier* classifier,
                      const _PrefixArray& aPrefixArray,
                      const nsCString& aTable,
                      const nsCString& aFragment)
 {
   Completion lookupHash;
   lookupHash.FromPlaintext(aFragment);
-  LookupResult result;
-  result.hash.complete = lookupHash;
+  RefPtr<LookupResult> result = new LookupResult;
+  result->hash.complete = lookupHash;
 
   PrefixArray noiseEntries;
   uint32_t noiseCount = 3;
   nsresult rv;
-  rv = classifier->ReadNoiseEntries(result.hash.fixedLengthPrefix,
+  rv = classifier->ReadNoiseEntries(result->hash.fixedLengthPrefix,
                                     aTable, noiseCount,
                                     noiseEntries);
   ASSERT_TRUE(rv == NS_OK);
   EXPECT_TRUE(noiseEntries.Length() > 0);
 
   for (uint32_t i = 0; i < noiseEntries.Length(); i++) {
     // Test the noise entry should not equal the "real" hash request
-    EXPECT_NE(noiseEntries[i], result.hash.fixedLengthPrefix);
+    EXPECT_NE(noiseEntries[i], result->hash.fixedLengthPrefix);
     // Test the noise entry should exist in the cached prefix array
     nsAutoCString partialHash;
     partialHash.Assign(reinterpret_cast<char*>(&noiseEntries[i]), PREFIX_SIZE);
     EXPECT_TRUE(aPrefixArray.Contains(partialHash));
   }
 
 }