Bug 1032414: Always return failure in OnStopRequest on network error (r=gcp)
☠☠ backed out by f2dd2c376a05 ☠ ☠
authorMonica Chew <mmc@mozilla.com>
Fri, 07 Nov 2014 07:12:37 -0800
changeset 214601 527d5142b7c5887de752d6223f0f8052117a9de2
parent 214600 dcac8f919b6e8ba21eaf3dca2bab1f329f2d508b
child 214602 ceca39a1a15480e8427eb2f7c50c63f79f79f6d1
push id51517
push usermchew@mozilla.com
push dateFri, 07 Nov 2014 15:12:48 +0000
treeherdermozilla-inbound@527d5142b7c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1032414
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1032414: Always return failure in OnStopRequest on network error (r=gcp)
toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
--- a/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierStreamUpdater.cpp
@@ -365,16 +365,18 @@ nsUrlClassifierStreamUpdater::UpdateSucc
 
   // DownloadDone() clears mSuccessCallback, so we save it off here.
   nsCOMPtr<nsIUrlClassifierCallback> successCallback = mDownloadError ? nullptr : mSuccessCallback.get();
   DownloadDone();
 
   nsAutoCString strTimeout;
   strTimeout.AppendInt(requestedTimeout);
   if (successCallback) {
+    LOG(("nsUrlClassifierStreamUpdater::UpdateSuccess callback [this=%p]",
+         this));
     successCallback->HandleEvent(strTimeout);
   }
   // Now fetch the next request
   LOG(("stream updater: calling into fetch next request"));
   FetchNextRequest();
 
   return NS_OK;
 }
@@ -434,50 +436,52 @@ nsUrlClassifierStreamUpdater::AddRequest
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::OnStartRequest(nsIRequest *request,
                                              nsISupports* context)
 {
   nsresult rv;
   bool downloadError = false;
   nsAutoCString strStatus;
   nsresult status = NS_OK;
+  LOG(("nsUrlClassifierStreamUpdater::OnStartRequest"));
 
   // Only update if we got http success header
   nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(request);
   if (httpChannel) {
     rv = httpChannel->GetStatus(&status);
+    LOG(("nsUrlClassifierStreamUpdater::OnStartRequest (status=%x)", status));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    if (NS_ERROR_CONNECTION_REFUSED == status ||
-        NS_ERROR_NET_TIMEOUT == status) {
+    if (NS_FAILED(status)) {
       // Assume we're overloading the server and trigger backoff.
       downloadError = true;
-    }
-
-    if (NS_SUCCEEDED(status)) {
+    } else {
       bool succeeded = false;
       rv = httpChannel->GetRequestSucceeded(&succeeded);
       NS_ENSURE_SUCCESS(rv, rv);
 
+      LOG(("nsUrlClassifierStreamUpdater::OnStartRequest (%s)", succeeded ?
+           "succeeded" : "failed"));
       if (!succeeded) {
         // 404 or other error, pass error status back
         LOG(("HTTP request returned failure code."));
 
         uint32_t requestStatus;
         rv = httpChannel->GetResponseStatus(&requestStatus);
         LOG(("HTTP request returned failure code: %d.", requestStatus));
         NS_ENSURE_SUCCESS(rv, rv);
 
         strStatus.AppendInt(requestStatus);
         downloadError = true;
       }
     }
   }
 
   if (downloadError) {
+    LOG(("nsUrlClassifierStreamUpdater::Download error"));
     mDownloadErrorCallback->HandleEvent(strStatus);
     mDownloadError = true;
     status = NS_ERROR_ABORT;
   } else if (NS_SUCCEEDED(status)) {
     mBeganStream = true;
     rv = mDBService->BeginStream(mStreamTable);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -535,17 +539,22 @@ nsUrlClassifierStreamUpdater::OnStopRequ
     // The fetch failed, but we didn't start the stream (probably a
     // server or connection error).  We can commit what we've applied
     // so far, and request again later.
     rv = mDBService->FinishUpdate();
   }
 
   mChannel = nullptr;
 
-  return rv;
+  // If the fetch failed, return the network status rather than NS_OK, the
+  // result of finishing a possibly-empty update
+  if (NS_SUCCEEDED(aStatus)) {
+    return rv;
+  }
+  return aStatus;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // nsIObserver implementation
 
 NS_IMETHODIMP
 nsUrlClassifierStreamUpdater::Observe(nsISupports *aSubject, const char *aTopic,
                                       const char16_t *aData)