Bug 1164518 - Avoid unnecessary DB updates when caching Safe Browsing results. r?gcp
MozReview-Commit-ID: HYNaTdCRohL
--- a/toolkit/components/url-classifier/Entries.h
+++ b/toolkit/components/url-classifier/Entries.h
@@ -176,16 +176,23 @@ struct AddComplete {
template<class T>
int Compare(const T& other) const {
int cmp = complete.Compare(other.CompleteHash());
if (cmp != 0) {
return cmp;
}
return addChunk - other.addChunk;
}
+
+ bool operator!=(const AddComplete& aOther) const {
+ if (addChunk != aOther.addChunk) {
+ return true;
+ }
+ return complete != aOther.complete;
+ }
};
struct SubPrefix {
// The hash to subtract.
Prefix prefix;
// The chunk number of the add chunk to which the hash belonged.
uint32_t addChunk;
// The chunk number of this sub chunk.
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -62,16 +62,23 @@ public:
nsCString mTableName;
};
typedef nsTArray<LookupResult> LookupResultArray;
struct CacheResult {
AddComplete entry;
nsCString table;
+
+ bool operator==(const CacheResult& aOther) const {
+ if (entry != aOther.entry) {
+ return false;
+ }
+ return table == aOther.table;
+ }
};
typedef nsTArray<CacheResult> CacheResultArray;
class LookupCache {
public:
// Check for a canonicalized IP address.
static bool IsCanonicalizedIP(const nsACString& aHost);
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -102,16 +102,21 @@ interface nsIUrlClassifierDBService : ns
/**
* Set the last update time for the given table. We use this to
* remember freshness past restarts. Time is in milliseconds since epoch.
*/
void setLastUpdateTime(in ACString tableName,
in unsigned long long lastUpdateTime);
+ /**
+ * Forget the results that were used in the last DB update.
+ */
+ void clearLastResults();
+
////////////////////////////////////////////////////////////////////////////
// Incremental update methods.
//
// An update to the database has the following steps:
//
// 1) The update process is started with beginUpdate(). The client
// passes an nsIUrlClassifierUpdateObserver object which will be
// notified as the update is processed by the dbservice.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -655,16 +655,21 @@ nsUrlClassifierDBServiceWorker::CacheCom
{
LOG(("nsUrlClassifierDBServiceWorker::CacheCompletions [%p]", this));
if (!mClassifier)
return NS_OK;
// Ownership is transferred in to us
nsAutoPtr<CacheResultArray> resultsPtr(results);
+ if (mLastResults == *resultsPtr) {
+ LOG(("Skipping completions that have just been cached already."));
+ return NS_OK;
+ }
+
nsAutoPtr<ProtocolParser> pParse(new ProtocolParser());
nsTArray<TableUpdate*> updates;
// Only cache results for tables that we have, don't take
// in tables we might accidentally have hit during a completion.
// This happens due to goog vs googpub lists existing.
nsTArray<nsCString> tables;
nsresult rv = mClassifier->ActiveTables(tables);
@@ -697,16 +702,17 @@ nsUrlClassifierDBServiceWorker::CacheCom
updates.AppendElement(tu);
pParse->ForgetTableUpdates();
} else {
LOG(("Completion received, but table is not active, so not caching."));
}
}
mClassifier->ApplyUpdates(&updates);
+ mLastResults = *resultsPtr;
return NS_OK;
}
NS_IMETHODIMP
nsUrlClassifierDBServiceWorker::CacheMisses(PrefixArray *results)
{
LOG(("nsUrlClassifierDBServiceWorker::CacheMisses [%p] %d",
this, results->Length()));
@@ -753,16 +759,25 @@ nsUrlClassifierDBServiceWorker::SetLastU
MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
MOZ_ASSERT(mClassifier, "Classifier connection must be opened");
mClassifier->SetLastUpdateTime(table, updateTime);
return NS_OK;
}
+NS_IMETHODIMP
+nsUrlClassifierDBServiceWorker::ClearLastResults()
+{
+ MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
+ mLastResults.Clear();
+ return NS_OK;
+}
+
+
// -------------------------------------------------------------------------
// nsUrlClassifierLookupCallback
//
// This class takes the results of a lookup found on the worker thread
// and handles any necessary partial hash expansions before calling
// the client callback.
class nsUrlClassifierLookupCallback final : public nsIUrlClassifierLookupCallback
@@ -1453,30 +1468,38 @@ NS_IMETHODIMP
nsUrlClassifierDBService::SetHashCompleter(const nsACString &tableName,
nsIUrlClassifierHashCompleter *completer)
{
if (completer) {
mCompleters.Put(tableName, completer);
} else {
mCompleters.Remove(tableName);
}
-
+ ClearLastResults();
return NS_OK;
}
NS_IMETHODIMP
nsUrlClassifierDBService::SetLastUpdateTime(const nsACString &tableName,
uint64_t lastUpdateTime)
{
NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
return mWorkerProxy->SetLastUpdateTime(tableName, lastUpdateTime);
}
NS_IMETHODIMP
+nsUrlClassifierDBService::ClearLastResults()
+{
+ NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
+
+ return mWorkerProxy->ClearLastResults();
+}
+
+NS_IMETHODIMP
nsUrlClassifierDBService::BeginUpdate(nsIUrlClassifierUpdateObserver *observer,
const nsACString &updateTables)
{
NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
if (mInUpdate) {
LOG(("Already updating, not available"));
return NS_ERROR_NOT_AVAILABLE;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -207,16 +207,19 @@ private:
nsTArray<mozilla::safebrowsing::TableUpdate*> mTableUpdates;
int32_t mUpdateWait;
// Entries that cannot be completed. We expect them to die at
// the next update
PrefixArray mMissCache;
+ // Stores the last results that triggered a table update.
+ CacheResultArray mLastResults;
+
nsresult mUpdateStatus;
nsTArray<nsCString> mUpdateTables;
nsCOMPtr<nsIUrlClassifierUpdateObserver> mUpdateObserver;
bool mInStream;
// The number of noise entries to add to the set of lookup results.
uint32_t mGethashNoise;
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -227,16 +227,29 @@ UrlClassifierDBServiceWorkerProxy::SetLa
NS_IMETHODIMP
UrlClassifierDBServiceWorkerProxy::SetLastUpdateTimeRunnable::Run()
{
mTarget->SetLastUpdateTime(mTable, mUpdateTime);
return NS_OK;
}
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResults()
+{
+ nsCOMPtr<nsIRunnable> r = new ClearLastResultsRunnable(mTarget);
+ return DispatchToWorkerThread(r);
+}
+
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable::Run()
+{
+ return mTarget->ClearLastResults();
+}
+
NS_IMPL_ISUPPORTS(UrlClassifierLookupCallbackProxy,
nsIUrlClassifierLookupCallback)
NS_IMETHODIMP
UrlClassifierLookupCallbackProxy::LookupComplete
(LookupResultArray * aResults)
{
nsCOMPtr<nsIRunnable> r = new LookupCompleteRunnable(mTarget, aResults);
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -186,16 +186,28 @@ public:
NS_DECL_NSIRUNNABLE
private:
RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
nsCString mTable;
uint64_t mUpdateTime;
};
+ class ClearLastResultsRunnable : public nsRunnable
+ {
+ public:
+ explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
+ : mTarget(aTarget)
+ { }
+
+ NS_DECL_NSIRUNNABLE
+ private:
+ RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
+ };
+
public:
nsresult DoLocalLookup(const nsACString& spec,
const nsACString& tables,
mozilla::safebrowsing::LookupResultArray* results);
private:
~UrlClassifierDBServiceWorkerProxy() {}