Bug 455311. Better handling of Cancel and IsPending() on nsBaseChannel redirects, plus unit tests. r+sr=biesi

netwerk/base/src/nsBaseChannel.cpp

@@ -89,6 +89,7 @@ nsBaseChannel::nsBaseChannel()
   , mQueriedProgressSink(PR_TRUE)
   , mSynthProgressEvents(PR_FALSE)
   , mWasOpened(PR_FALSE)
+  , mWaitingOnAsyncRedirect(PR_FALSE)
 {
   mContentType.AssignLiteral(UNKNOWN_CONTENT_TYPE);
 }
@@ -227,8 +228,12 @@ nsBaseChannel::BeginPumpingData()
 
   NS_ASSERTION(!stream || !channel, "Got both a channel and a stream?");
 
-  if (channel)
-      return NS_DispatchToCurrentThread(new RedirectRunnable(this, channel));
+  if (channel) {
+      rv = NS_DispatchToCurrentThread(new RedirectRunnable(this, channel));
+      if (NS_SUCCEEDED(rv))
+          mWaitingOnAsyncRedirect = PR_TRUE;
+      return rv;
+  }
 
   // By assigning mPump, we flag this channel as pending (see IsPending).  It's
   // important that the pending flag is set when we call into the stream (the
@@ -248,11 +253,17 @@ void
 nsBaseChannel::HandleAsyncRedirect(nsIChannel* newChannel)
 {
   NS_ASSERTION(!mPump, "Shouldn't have gotten here");
-  nsresult rv = Redirect(newChannel, nsIChannelEventSink::REDIRECT_INTERNAL,
-                         PR_TRUE);
-  if (NS_FAILED(rv)) {
+  if (NS_SUCCEEDED(mStatus)) {
+      nsresult rv = Redirect(newChannel, nsIChannelEventSink::REDIRECT_INTERNAL,
+                             PR_TRUE);
+      if (NS_FAILED(rv))
+          Cancel(rv);
+  }
+
+  mWaitingOnAsyncRedirect = PR_FALSE;
+
+  if (NS_FAILED(mStatus)) {
     // Notify our consumer ourselves
-    Cancel(rv);
     mListener->OnStartRequest(this, mListenerContext);
     mListener->OnStopRequest(this, mListenerContext, mStatus);
     mListener = nsnull;

netwerk/base/src/nsBaseChannel.h

@@ -179,7 +179,7 @@ public:
 
   // This is a short-cut to calling nsIRequest::IsPending()
   PRBool IsPending() const {
-    return (mPump != nsnull);
+    return mPump || mWaitingOnAsyncRedirect;
   }
 
   // Set the content length that should be reported for this channel.  Pass -1
@@ -285,6 +285,7 @@ private:
   PRPackedBool                        mQueriedProgressSink;
   PRPackedBool                        mSynthProgressEvents;
   PRPackedBool                        mWasOpened;
+  PRPackedBool                        mWaitingOnAsyncRedirect;
 };
 
 #endif // !nsBaseChannel_h__

netwerk/test/unit/test_bug455311.js

@@ -0,0 +1,124 @@
+const Ci = Components.interfaces;
+const Cc = Components.classes;
+const Cr = Components.results;
+const isWindows = ("@mozilla.org/windows-registry-key;1" in Cc);
+const isLinux = ("@mozilla.org/gnome-gconf-service;1" in Cc);
+
+function getLinkFile()
+{
+  if (isWindows) {
+    return do_get_file("netwerk/test/unit/test_link.url");
+  }
+  if (isLinux) {
+    return do_get_file("netwerk/test/unit/test_link.desktop");
+  }
+  do_throw("Unexpected platform");
+  return null;
+}
+
+const ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+const link = getLinkFile();
+const linkURI = ios.newFileURI(link);
+const newURI = ios.newURI("http://www.mozilla.org/", null, null);
+  
+function NotificationCallbacks(origURI, newURI)
+{
+    this._origURI = origURI;
+  this._newURI = newURI;
+}
+NotificationCallbacks.prototype = {
+  QueryInterface: function(iid)
+  {
+    if (iid.equals(Ci.nsISupports) ||
+	iid.equals(Ci.nsIInterfaceRequestor) ||
+	iid.equals(Ci.nsIChannelEventSink)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+  getInterface: function (iid)
+  {
+    return this.QueryInterface(iid);
+  },
+  onChannelRedirect: function(oldChan, newChan, flags)
+  {
+    do_check_eq(oldChan.URI.spec, this._origURI.spec);
+    do_check_eq(oldChan.URI, this._origURI);
+    do_check_eq(oldChan.originalURI.spec, this._origURI.spec);
+    do_check_eq(oldChan.originalURI, this._origURI);
+    do_check_eq(newChan.originalURI.spec, this._origURI.spec);
+    do_check_eq(newChan.originalURI, this._origURI);
+    do_check_eq(newChan.URI.spec, this._newURI.spec);
+    throw Cr.NS_ERROR_ABORT;
+  }
+};
+
+function RequestObserver(origURI, newURI, nextTest)
+{
+  this._origURI = origURI;
+  this._newURI = newURI;
+  this._nextTest = nextTest;
+}
+RequestObserver.prototype = {
+  QueryInterface: function(iid)
+  {
+    if (iid.equals(Ci.nsISupports) ||
+	iid.equals(Ci.nsIRequestObserver) ||
+	iid.equals(Ci.nsIStreamListener)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+  onStartRequest: function (req, ctx)
+  {
+    var chan = req.QueryInterface(Ci.nsIChannel);
+    do_check_eq(chan.URI.spec, this._origURI.spec);
+    do_check_eq(chan.URI, this._origURI);
+    do_check_eq(chan.originalURI.spec, this._origURI.spec);
+    do_check_eq(chan.originalURI, this._origURI);
+  },
+  onDataAvailable: function(req, ctx, stream, offset, count)
+  {
+    do_throw("Unexpected call to onDataAvailable");
+  },
+  onStopRequest: function (req, ctx, status)
+  {
+    var chan = req.QueryInterface(Ci.nsIChannel);
+    try {
+      do_check_eq(chan.URI.spec, this._origURI.spec);
+      do_check_eq(chan.URI, this._origURI);
+      do_check_eq(chan.originalURI.spec, this._origURI.spec);
+      do_check_eq(chan.originalURI, this._origURI);
+      do_check_eq(status, Cr.NS_ERROR_ABORT);
+      do_check_false(chan.isPending());
+    } catch(e) {}
+    this._nextTest();
+  }
+};
+
+function test_cancel()
+{
+  var chan = ios.newChannelFromURI(linkURI);
+  do_check_eq(chan.URI, linkURI);
+  do_check_eq(chan.originalURI, linkURI);
+  chan.asyncOpen(new RequestObserver(linkURI, newURI, do_test_finished), null);
+  do_check_true(chan.isPending());
+  chan.cancel(Cr.NS_ERROR_ABORT);
+  do_check_true(chan.isPending());
+}
+
+function run_test()
+{
+  if (!isWindows && !isLinux) {
+    return;
+  }
+
+  do_test_pending();
+
+  var chan = ios.newChannelFromURI(linkURI);
+  do_check_eq(chan.URI, linkURI);
+  do_check_eq(chan.originalURI, linkURI);
+  chan.notificationCallbacks = new NotificationCallbacks(linkURI, newURI);
+  chan.asyncOpen(new RequestObserver(linkURI, newURI, test_cancel), null);
+  do_check_true(chan.isPending());
+}

netwerk/test/unit/test_file_protocol.js

@@ -69,9 +69,9 @@ FileStreamListener.prototype = {
   },
 
   QueryInterface: function(iid) {
-    if (iid.Equals(Ci.nsIStreamListener) ||
-        iid.Equals(Ci.nsIRequestObserver) ||
-        iid.Equals(Ci.nsISupports))
+    if (iid.equals(Ci.nsIStreamListener) ||
+        iid.equals(Ci.nsIRequestObserver) ||
+        iid.equals(Ci.nsISupports))
       return this;
     throw Cr.NS_ERROR_NO_INTERFACE;
   },

netwerk/test/unit/test_gzipped_206.js

@@ -52,9 +52,9 @@ function Canceler() {
 
 Canceler.prototype = {
   QueryInterface: function(iid) {
-    if (iid.Equals(Ci.nsIStreamListener) ||
-        iid.Equals(Ci.nsIRequestObserver) ||
-        iid.Equals(Ci.nsISupports))
+    if (iid.equals(Ci.nsIStreamListener) ||
+        iid.equals(Ci.nsIRequestObserver) ||
+        iid.equals(Ci.nsISupports))
       return this;
     throw Components.results.NS_ERROR_NO_INTERFACE;
   },

netwerk/test/unit/test_link.desktop

@@ -0,0 +1,3 @@
+[Desktop Entry]
+Type=Link
+URL=http://www.mozilla.org/

netwerk/test/unit/test_link.url

@@ -0,0 +1,5 @@
+[InternetShortcut]
+URL=http://www.mozilla.org/
+IDList=
+[{000214A0-0000-0000-C000-000000000046}]
+Prop3=19,2

netwerk/test/unit/test_resumable_truncate.js

@@ -45,9 +45,9 @@ function Canceler(continueFn) {
 
 Canceler.prototype = {
   QueryInterface: function(iid) {
-    if (iid.Equals(Ci.nsIStreamListener) ||
-        iid.Equals(Ci.nsIRequestObserver) ||
-        iid.Equals(Ci.nsISupports))
+    if (iid.equals(Ci.nsIStreamListener) ||
+        iid.equals(Ci.nsIRequestObserver) ||
+        iid.equals(Ci.nsISupports))
       return this;
     throw Components.results.NS_ERROR_NO_INTERFACE;
   },