Bug 420370: nsUrlClassifierHashCompleter not configured immediately at startup. r=tony, blocking-firefox3=beltzner
authordcamp@mozilla.com
Tue, 04 Mar 2008 14:05:05 -0800
changeset 12571 94274772805743209595cf785112d74e1e780eab
parent 12570 b41efc45db63d90ffd2009daa5e6316643faa4c7
child 12572 6dcd7b9f65ae935c6f0f37dbd66b473852fc768a
push idunknown
push userunknown
push dateunknown
reviewerstony
bugs420370
milestone1.9b5pre
Bug 420370: nsUrlClassifierHashCompleter not configured immediately at startup. r=tony, blocking-firefox3=beltzner
toolkit/components/url-classifier/content/listmanager.js
toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/src/nsUrlClassifierDBService.h
toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
toolkit/components/url-classifier/tests/unit/test_partial.js
--- a/toolkit/components/url-classifier/content/listmanager.js
+++ b/toolkit/components/url-classifier/content/listmanager.js
@@ -100,17 +100,17 @@ function PROT_ListManager() {
                                    10*60*1000 /* error time, 10min */,
                                    60*60*1000 /* backoff interval, 60min */,
                                    6*60*60*1000 /* max backoff, 6hr */);
 
   this.dbService_ = Cc["@mozilla.org/url-classifier/dbservice;1"]
                    .getService(Ci.nsIUrlClassifierDBService);
 
   this.hashCompleter_ = Cc["@mozilla.org/url-classifier/hashcompleter;1"]
-                        .createInstance(Ci.nsIUrlClassifierHashCompleter);
+                        .getService(Ci.nsIUrlClassifierHashCompleter);
 }
 
 /**
  * xpcom-shutdown callback
  * Delete all of our data tables which seem to leak otherwise.
  */
 PROT_ListManager.prototype.shutdown_ = function() {
   for (var name in this.tablesData) {
@@ -173,17 +173,16 @@ PROT_ListManager.prototype.setKeyUrl = f
  * @param tableName - the name of the table
  * @param opt_requireMac true if a mac is required on update, false otherwise
  * @returns true if the table could be created; false otherwise
  */
 PROT_ListManager.prototype.registerTable = function(tableName, 
                                                     opt_requireMac) {
   this.tablesData[tableName] = {};
   this.tablesData[tableName].needsUpdate = false;
-  this.dbService_.setHashCompleter(tableName, this.hashCompleter_);
 
   return true;
 }
 
 /**
  * Enable updates for some tables
  * @param tables - an array of table names that need updating
  */
--- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp
@@ -3715,16 +3715,28 @@ nsUrlClassifierDBService::ResetDatabase(
 nsresult
 nsUrlClassifierDBService::CacheCompletions(nsTArray<nsUrlClassifierLookupResult> *results)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   return mWorkerProxy->CacheCompletions(results);
 }
 
+PRBool
+nsUrlClassifierDBService::GetCompleter(const nsACString &tableName,
+                                       nsIUrlClassifierHashCompleter **completer)
+{
+  if (mCompleters.Get(tableName, completer)) {
+    return PR_TRUE;
+  }
+
+  return NS_SUCCEEDED(CallGetService(NS_URLCLASSIFIERHASHCOMPLETER_CONTRACTID,
+                                     completer));
+}
+
 NS_IMETHODIMP
 nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic,
                                   const PRUnichar *aData)
 {
   if (!strcmp(aTopic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) {
     nsresult rv;
     nsCOMPtr<nsIPrefBranch> prefs(do_QueryInterface(aSubject, &rv));
     NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.h
@@ -82,20 +82,17 @@ public:
 #endif
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIURLCLASSIFIERDBSERVICE
   NS_DECL_NSIURICLASSIFIER
   NS_DECL_NSIOBSERVER
 
   PRBool GetCompleter(const nsACString& tableName,
-                      nsIUrlClassifierHashCompleter** completer) {
-    return mCompleters.Get(tableName, completer);
-  }
-
+                      nsIUrlClassifierHashCompleter** completer);
   nsresult CacheCompletions(nsTArray<nsUrlClassifierLookupResult> *results);
 
 private:
   // No subclassing
   ~nsUrlClassifierDBService();
 
   // Disallow copy constructor
   nsUrlClassifierDBService(nsUrlClassifierDBService&);
--- a/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
+++ b/toolkit/components/url-classifier/src/nsUrlClassifierHashCompleter.cpp
@@ -525,27 +525,37 @@ nsUrlClassifierHashCompleter::Complete(c
   // We batch all of the requested completions in a single request until the
   // next time we reach the main loop.
   if (!mRequest) {
     mRequest = new nsUrlClassifierHashCompleterRequest(this);
     if (!mRequest) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    // Schedule ourselves to start this request on the main loop.
-    NS_DispatchToCurrentThread(this);
+    // If we don't have a gethash url yet, don't bother scheduling
+    // the request until we have one.
+    if (!mGethashUrl.IsEmpty()) {
+      // Schedule ourselves to start this request on the main loop.
+      NS_DispatchToCurrentThread(this);
+    }
   }
 
   return mRequest->Add(partialHash, c);
 }
 
 NS_IMETHODIMP
 nsUrlClassifierHashCompleter::SetGethashUrl(const nsACString &url)
 {
   mGethashUrl = url;
+
+  if (mRequest) {
+    // Schedule any pending request.
+    NS_DispatchToCurrentThread(this);
+  }
+
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierHashCompleter::GetGethashUrl(nsACString &url)
 {
   url = mGethashUrl;
   return NS_OK;
--- a/toolkit/components/url-classifier/tests/unit/test_partial.js
+++ b/toolkit/components/url-classifier/tests/unit/test_partial.js
@@ -313,42 +313,16 @@ function testMixedSizesDifferentDomains(
     "urlsExist" : add1Urls.concat(add2Urls),
     // ... but the completer should only be queried for the partial entry
     "completerQueried" : [completer, add1Urls]
   };
 
   doTest([update1, update2], assertions);
 }
 
-function testMixedSizesNoCompleter() {
-  var add1Urls = [ "foo.com/a" ];
-  var add2Urls = [ "foo.com/b" ];
-
-  var update1 = buildPhishingUpdate(
-    [
-      { "chunkNum" : 1,
-        "urls" : add1Urls }],
-    4);
-  var update2 = buildPhishingUpdate(
-    [
-      { "chunkNum" : 2,
-        "urls" : add2Urls }],
-    32);
-
-  var assertions = {
-    "tableData" : "test-phish-simple;a:1-2",
-    // add1Urls shouldn't work, because there is no completer.
-    "urlsDontExist" : add1Urls,
-    // but add2Urls were complete, they should work.
-    "urlsExist" : add2Urls
-  };
-
-  doTest([update1, update2], assertions);
-}
-
 function testInvalidHashSize()
 {
   var addUrls = [ "foo.com/a", "foo.com/b", "bar.com/c" ];
   var update = buildPhishingUpdate(
         [
           { "chunkNum" : 1,
             "urls" : addUrls
           }],
@@ -525,17 +499,16 @@ function run_test()
   runTests([
     testPartialAdds,
     testPartialAddsWithConflicts,
     testFalsePositives,
     testEmptyCompleter,
     testCompleterFailure,
     testMixedSizesSameDomain,
     testMixedSizesDifferentDomains,
-    testMixedSizesNoCompleter,
     testInvalidHashSize,
     testCachedResults,
     testCachedResultsWithSub,
     testCachedResultsWithExpire,
     testUncachedResults,
   ]);
 }