Bug 1254323 - Reduce identical gethash requests done by the URL Classifier. r=gcp

MozReview-Commit-ID: KNNL1dBqXx0

--HG--
extra : rebase_source : fd18c3dac2a9fc331f78d492154283472ec2957d

toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js

@@ -166,6 +166,9 @@ function HashCompleter() {
   // to null. It may be used by multiple calls to |complete| in succession to
   // avoid creating multiple requests to the same gethash URL.
   this._currentRequest = null;
+  // An Array of ongoing gethash requests which is used to find requests for
+  // the same hash prefix.
+  this._ongoingRequests = [];
   // A map of gethashUrls to HashCompleterRequests that haven't yet begun.
   this._pendingRequests = {};
 
@@ -201,6 +204,15 @@ HashCompleter.prototype = {
       throw Cr.NS_ERROR_NOT_INITIALIZED;
     }
 
+    // Check ongoing requests before creating a new HashCompleteRequest
+    for (let r of this._ongoingRequests) {
+      if (r.find(aPartialHash, aGethashUrl, aTableName)) {
+        log("Merge gethash request in " + aTableName + " for prefix : " + btoa(aPartialHash));
+        r.add(aPartialHash, aCallback, aTableName);
+        return;
+      }
+    }
+
     if (!this._currentRequest) {
       this._currentRequest = new HashCompleterRequest(this, aGethashUrl);
     }
@@ -262,7 +274,9 @@ HashCompleter.prototype = {
 
     if (this._currentRequest) {
       try {
-        this._currentRequest.begin();
+        if (this._currentRequest.begin()) {
+          this._ongoingRequests.push(this._currentRequest);
+        }
       } finally {
         // If |begin| fails, we should get rid of our request.
         this._currentRequest = null;
@@ -272,8 +286,10 @@ HashCompleter.prototype = {
 
   // Pass the server response status to the RequestBackoff for the given
   // gethashUrl and fetch the next pending request, if there is one.
-  finishRequest(url, aStatus) {
-    this._backoffs[url].noteServerResponse(aStatus);
+  finishRequest(aRequest, aStatus) {
+    this._ongoingRequests = this._ongoingRequests.filter(v => v != aRequest);
+
+    this._backoffs[aRequest.gethashUrl].noteServerResponse(aStatus);
     Services.tm.dispatchToMainThread(this);
   },
 
@@ -351,7 +367,9 @@ HashCompleterRequest.prototype = {
         log('ERROR: Cannot mix "proto" tables with other types within ' +
             "the same gethash URL.");
       }
-      this.tableNames.set(aTableName);
+      if (!this.tableNames.has(aTableName)) {
+        this.tableNames.set(aTableName);
+      }
 
       // Assuming all tables with the same gethash URL have the same provider
       if (this.provider == "") {
@@ -364,6 +382,17 @@ HashCompleterRequest.prototype = {
     }
   },
 
+  find: function HCR_find(aPartialHash, aGetHashUrl, aTableName) {
+    if (this.gethashUrl != aGetHashUrl ||
+        !this.tableNames.has(aTableName)) {
+      return false;
+    }
+
+    return this._requests.find(function(r) {
+      return r.partialHash === aPartialHash;
+    });
+  },
+
   fillTableStatesBase64: function HCR_fillTableStatesBase64(aCallback) {
     gDbService.getTables(aTableData => {
       aTableData.split("\n").forEach(line => {
@@ -392,7 +421,7 @@ HashCompleterRequest.prototype = {
     if (!this._completer.canMakeRequest(this.gethashUrl)) {
       log("Can't make request to " + this.gethashUrl + "\n");
       this.notifyFailure(Cr.NS_ERROR_ABORT);
-      return;
+      return false;
     }
 
     Services.obs.addObserver(this, "quit-application");
@@ -408,10 +437,14 @@ HashCompleterRequest.prototype = {
         // point, finishRequest must be called.
         this._completer.noteRequest(this.gethashUrl);
       } catch (err) {
+        this._completer._ongoingRequests =
+          this._completer._ongoingRequests.filter(v => v != this);
         this.notifyFailure(err);
         throw err;
       }
     });
+
+    return true;
   },
 
   notify: function HCR_notify() {
@@ -776,7 +809,7 @@ HashCompleterRequest.prototype = {
     }
     let success = Components.isSuccessCode(aStatusCode);
     log("Received a " + httpStatus + " status code from the " + this.provider +
-        " gethash server (success=" + success + ").");
+        " gethash server (success=" + success + "): " + btoa(this._response));
 
     Services.telemetry.getKeyedHistogramById("URLCLASSIFIER_COMPLETE_REMOTE_STATUS2").
       add(this.telemetryProvider, httpStatusToBucket(httpStatus));
@@ -789,7 +822,7 @@ HashCompleterRequest.prototype = {
       add(this.telemetryProvider, 0);
 
     // Notify the RequestBackoff once a response is received.
-    this._completer.finishRequest(this.gethashUrl, httpStatus);
+    this._completer.finishRequest(this, httpStatus);
 
     if (success) {
       try {