From d017ca6b69e0fd6405f6a3c644fbdfada2199cc3 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 22 Oct 2008 11:42:32 -0400 Subject: [PATCH] Bug 455311. Better handling of Cancel and IsPending() on nsBaseChannel redirects, plus unit tests. r+sr=biesi --- netwerk/base/src/nsBaseChannel.cpp | 23 +++- netwerk/base/src/nsBaseChannel.h | 3 +- netwerk/test/unit/test_bug455311.js | 124 +++++++++++++++++++ netwerk/test/unit/test_file_protocol.js | 6 +- netwerk/test/unit/test_gzipped_206.js | 6 +- netwerk/test/unit/test_link.desktop | 3 + netwerk/test/unit/test_link.url | 5 + netwerk/test/unit/test_resumable_truncate.js | 6 +- 8 files changed, 160 insertions(+), 16 deletions(-) create mode 100644 netwerk/test/unit/test_bug455311.js create mode 100644 netwerk/test/unit/test_link.desktop create mode 100644 netwerk/test/unit/test_link.url diff --git a/netwerk/base/src/nsBaseChannel.cpp b/netwerk/base/src/nsBaseChannel.cpp index df453e91dfe..7d4f2ad3ff7 100644 --- a/netwerk/base/src/nsBaseChannel.cpp +++ b/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; diff --git a/netwerk/base/src/nsBaseChannel.h b/netwerk/base/src/nsBaseChannel.h index 6878e8cf6ae..4b0dd426dba 100644 --- a/netwerk/base/src/nsBaseChannel.h +++ b/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__ diff --git a/netwerk/test/unit/test_bug455311.js b/netwerk/test/unit/test_bug455311.js new file mode 100644 index 00000000000..962e977194c --- /dev/null +++ b/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()); +} diff --git a/netwerk/test/unit/test_file_protocol.js b/netwerk/test/unit/test_file_protocol.js index 577bbfd1be9..208d85ca3da 100644 --- a/netwerk/test/unit/test_file_protocol.js +++ b/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; }, diff --git a/netwerk/test/unit/test_gzipped_206.js b/netwerk/test/unit/test_gzipped_206.js index 5fe79c13ca6..cfc9dc3224e 100644 --- a/netwerk/test/unit/test_gzipped_206.js +++ b/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; }, diff --git a/netwerk/test/unit/test_link.desktop b/netwerk/test/unit/test_link.desktop new file mode 100644 index 00000000000..b1798202e3f --- /dev/null +++ b/netwerk/test/unit/test_link.desktop @@ -0,0 +1,3 @@ +[Desktop Entry] +Type=Link +URL=http://www.mozilla.org/ diff --git a/netwerk/test/unit/test_link.url b/netwerk/test/unit/test_link.url new file mode 100644 index 00000000000..05f82755449 --- /dev/null +++ b/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 diff --git a/netwerk/test/unit/test_resumable_truncate.js b/netwerk/test/unit/test_resumable_truncate.js index 9336ec3a33e..ed8313e5ec0 100644 --- a/netwerk/test/unit/test_resumable_truncate.js +++ b/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; },