Bug 1408396 - Fix a deadlock issue when about:url-classifier accesses GetCacheInfo API. r?francois
When about:url-classifier ask cache information by using |GetCacheInfo| API, this API
will send a SYNC request to off-main thread. If NSS module is not initialized then off-main thread
will send init request to main thread, and this causes a deadlock.
This patch change |GetCacheInfo| API to asynchronous so main thread will not be
blocked.
MozReview-Commit-ID: 3y2hOjCoRUj
--- a/toolkit/components/url-classifier/nsIUrlClassifierInfo.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierInfo.idl
@@ -56,18 +56,25 @@ interface nsIUrlClassifierCacheInfo : ns
readonly attribute ACString table;
/*
* An array of nsIUrlClassifierCacheEntry.
*/
readonly attribute nsIArray entries;
};
+[scriptable, uuid(26e12ea4-14ff-4c77-858f-6745998b7659)]
+interface nsIUrlClassifierGetCacheCallback : nsISupports {
+
+ void onGetCacheComplete(in nsIUrlClassifierCacheInfo info);
+};
+
/**
* Interface to query url-classifier information.
*/
[scriptable, function, uuid(411bbff4-1b88-4687-aa36-e2bbdd93f6e8)]
interface nsIUrlClassifierInfo : nsISupports {
/**
- * A synchronous call to return cache information for the table.
+ * An asynchronous call to return cache information for the table.
*/
- nsIUrlClassifierCacheInfo getCacheInfo(in ACString table);
+ void getCacheInfo(in ACString table,
+ in nsIUrlClassifierGetCacheCallback callback);
};
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -2364,21 +2364,22 @@ nsUrlClassifierDBService::ClearCache()
NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
return mWorkerProxy->ClearCache();
}
NS_IMETHODIMP
nsUrlClassifierDBService::GetCacheInfo(const nsACString& aTable,
- nsIUrlClassifierCacheInfo** aCache)
+ nsIUrlClassifierGetCacheCallback* aCallback)
{
NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
- return mWorkerProxy->GetCacheInfo(aTable, aCache);
+ return mWorkerProxy->GetCacheInfo(aTable, aCallback);
+ return NS_OK;
}
nsresult
nsUrlClassifierDBService::CacheCompletions(CacheResultArray *results)
{
NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
return mWorkerProxy->CacheCompletions(results);
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -238,35 +238,44 @@ UrlClassifierDBServiceWorkerProxy::Clear
NS_IMETHODIMP
UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable::Run()
{
return mTarget->ClearLastResults();
}
nsresult
UrlClassifierDBServiceWorkerProxy::GetCacheInfo(const nsACString& aTable,
- nsIUrlClassifierCacheInfo** aCache)
+ nsIUrlClassifierGetCacheCallback* aCallback)
{
- nsCOMPtr<nsIRunnable> r = new GetCacheInfoRunnable(mTarget, aTable, aCache);
-
- nsIThread* t = nsUrlClassifierDBService::BackgroundThread();
- if (!t) {
- return NS_ERROR_FAILURE;
- }
-
- // This blocks main thread but since 'GetCacheInfo' is only used by
- // about:url-classifier so it should be fine.
- mozilla::SyncRunnable::DispatchToThread(t, r);
- return NS_OK;
+ nsCOMPtr<nsIRunnable> r = new GetCacheInfoRunnable(mTarget, aTable, aCallback);
+ return DispatchToWorkerThread(r);
}
NS_IMETHODIMP
UrlClassifierDBServiceWorkerProxy::GetCacheInfoRunnable::Run()
{
- return mTarget->GetCacheInfo(mTable, mCache);
+ MOZ_ASSERT(mCallback);
+
+ mTarget->GetCacheInfo(mTable, &mCache);
+
+ nsCOMPtr<nsIRunnable> r = new GetCacheInfoCallbackRunnable(mCache, mCallback);
+ return NS_DispatchToMainThread(r);
+}
+
+
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::GetCacheInfoCallbackRunnable::Run()
+{
+ MOZ_ASSERT(NS_IsMainThread(), "Must be called on main thread");
+ MOZ_ASSERT(mCallback);
+
+ mCallback->OnGetCacheComplete(mCache);
+ NS_RELEASE(mCache);
+
+ return NS_OK;
}
NS_IMPL_ISUPPORTS(UrlClassifierLookupCallbackProxy,
nsIUrlClassifierLookupCallback)
NS_IMETHODIMP
UrlClassifierLookupCallbackProxy::LookupComplete
(LookupResultArray * aResults)
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -182,43 +182,63 @@ public:
RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
};
class GetCacheInfoRunnable: public mozilla::Runnable
{
public:
explicit GetCacheInfoRunnable(nsUrlClassifierDBServiceWorker* aTarget,
const nsACString& aTable,
- nsIUrlClassifierCacheInfo** aCache)
+ nsIUrlClassifierGetCacheCallback* aCallback)
: mozilla::Runnable(
"UrlClassifierDBServiceWorkerProxy::GetCacheInfoRunnable")
, mTarget(aTarget)
, mTable(aTable)
- , mCache(aCache)
+ , mCache(nullptr)
+ , mCallback(new nsMainThreadPtrHolder<nsIUrlClassifierGetCacheCallback>(
+ "nsIUrlClassifierGetCacheCallback", aCallback))
{ }
NS_DECL_NSIRUNNABLE
private:
RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
nsCString mTable;
- nsIUrlClassifierCacheInfo** mCache;
+ nsIUrlClassifierCacheInfo* mCache;
+ nsMainThreadPtrHandle<nsIUrlClassifierGetCacheCallback> mCallback;
+ };
+
+ class GetCacheInfoCallbackRunnable: public mozilla::Runnable
+ {
+ public:
+ explicit GetCacheInfoCallbackRunnable(nsIUrlClassifierCacheInfo* aCache,
+ nsMainThreadPtrHandle<nsIUrlClassifierGetCacheCallback>& aCallback)
+ : mozilla::Runnable(
+ "UrlClassifierDBServiceWorkerProxy::GetCacheInfoCallbackRunnable")
+ , mCache(aCache)
+ , mCallback(aCallback)
+ { }
+
+ NS_DECL_NSIRUNNABLE
+ private:
+ nsIUrlClassifierCacheInfo* mCache;
+ nsMainThreadPtrHandle<nsIUrlClassifierGetCacheCallback> mCallback;
};
public:
nsresult DoLocalLookup(const nsACString& spec,
const nsACString& tables,
mozilla::safebrowsing::LookupResultArray* results);
nsresult OpenDb();
nsresult CloseDb();
nsresult CacheCompletions(mozilla::safebrowsing::CacheResultArray * aEntries);
nsresult GetCacheInfo(const nsACString& aTable,
- nsIUrlClassifierCacheInfo** aCache);
+ nsIUrlClassifierGetCacheCallback* aCallback);
private:
~UrlClassifierDBServiceWorkerProxy() {}
RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
};
// The remaining classes here are all proxies to the main thread
--- a/toolkit/content/aboutUrlClassifier.js
+++ b/toolkit/content/aboutUrlClassifier.js
@@ -262,67 +262,70 @@ var Cache = {
let dbservice = Cc["@mozilla.org/url-classifier/dbservice;1"]
.getService(Ci.nsIUrlClassifierInfo);
for (let provider of Provider.providers) {
let pref = "browser.safebrowsing.provider." + provider + ".lists";
let tables = Services.prefs.getCharPref(pref, "").split(",");
for (let table of tables) {
- let cache = dbservice.getCacheInfo(table);
- let entries = cache.entries;
- if (entries.length === 0) {
- this.showCacheEnties.delete(table);
- continue;
- }
+ dbservice.getCacheInfo(table, {
+ onGetCacheComplete: (aCache) => {
+ let entries = aCache.entries;
+ if (entries.length === 0) {
+ this.showCacheEnties.delete(table);
+ return;
+ }
- let positiveCacheCount = 0;
- for (let i = 0; i < entries.length ; i++) {
- let entry = entries.queryElementAt(i, Ci.nsIUrlClassifierCacheEntry);
- let matches = entry.matches;
- positiveCacheCount += matches.length;
+ let positiveCacheCount = 0;
+ for (let i = 0; i < entries.length ; i++) {
+ let entry = entries.queryElementAt(i, Ci.nsIUrlClassifierCacheEntry);
+ let matches = entry.matches;
+ positiveCacheCount += matches.length;
- // If we don't have to show cache entries for this table then just
- // skip the following code.
- if (!this.showCacheEnties.has(table)) {
- continue;
- }
+ // If we don't have to show cache entries for this table then just
+ // skip the following code.
+ if (!this.showCacheEnties.has(table)) {
+ continue;
+ }
- let tds = [table, entry.prefix, new Date(entry.expiry * 1000).toString()];
- let j = 0;
- do {
- if (matches.length >= 1) {
- let match =
- matches.queryElementAt(j, Ci.nsIUrlClassifierPositiveCacheEntry);
- let list = [match.fullhash, new Date(match.expiry * 1000).toString()];
- tds = tds.concat(list);
- } else {
- tds = tds.concat([STR_NA, STR_NA]);
+ let tds = [table, entry.prefix, new Date(entry.expiry * 1000).toString()];
+ let j = 0;
+ do {
+ if (matches.length >= 1) {
+ let match =
+ matches.queryElementAt(j, Ci.nsIUrlClassifierPositiveCacheEntry);
+ let list = [match.fullhash, new Date(match.expiry * 1000).toString()];
+ tds = tds.concat(list);
+ } else {
+ tds = tds.concat([STR_NA, STR_NA]);
+ }
+ createRow(tds, document.getElementById("cache-entries-table-body"), 5);
+ j++;
+ tds = [""];
+ } while (j < matches.length);
}
- createRow(tds, document.getElementById("cache-entries-table-body"), 5);
- j++;
- tds = [""];
- } while (j < matches.length);
- }
- // Create cache information entries.
- let chk = document.createElement("input");
- chk.type = "checkbox";
- chk.checked = this.showCacheEnties.has(table);
- chk.addEventListener("click", () => {
- if (chk.checked) {
- this.showCacheEnties.add(table);
- } else {
- this.showCacheEnties.delete(table);
+ // Create cache information entries.
+ let chk = document.createElement("input");
+ chk.type = "checkbox";
+ chk.checked = this.showCacheEnties.has(table);
+ chk.addEventListener("click", () => {
+ if (chk.checked) {
+ this.showCacheEnties.add(table);
+ } else {
+ this.showCacheEnties.delete(table);
+ }
+ this.refresh();
+ });
+
+ let tds = [table, entries.length, positiveCacheCount, chk];
+ createRow(tds, document.getElementById("cache-table-body"), tds.length);
}
- this.refresh();
});
-
- let tds = [table, entries.length, positiveCacheCount, chk];
- createRow(tds, document.getElementById("cache-table-body"), tds.length);
}
}
let entries_div = document.getElementById("cache-entries");
entries_div.style.display = this.showCacheEnties.size == 0 ? "none" : "block";
},
};