Bug 515051 - Stream listener registered in a network request channel eats JS error messages, r=bz+biesi

netwerk/base/public/nsIStreamListenerTee.idl

@@ -38,14 +38,32 @@
 #include "nsIStreamListener.idl"
 
 interface nsIOutputStream;
+interface nsIRequestObserver;
 
 /**
  * As data "flows" into a stream listener tee, it is copied to the output stream
  * and then forwarded to the real listener.
  */
-[scriptable, uuid(fb683e76-d42b-41a4-8ae6-65a6c2b146e5)]
+[scriptable, uuid(8e86c460-2f6b-4595-a6ba-d5444827cd57)]
 interface nsIStreamListenerTee : nsIStreamListener
 {
+    /** 
+     * Initalize the tee.
+     *
+     * @param listener
+     *    the original listener the tee will propagate onStartRequest,
+     *    onDataAvailable and onStopRequest notifications to, exceptions from 
+     *    the listener will be propagated back to the channel
+     * @param sink
+     *    the stream the data coming from the channel will be written to,
+     *    should be blocking
+     * @param requestObserver
+     *    optional parameter, listener that gets only onStartRequest and
+     *    onStopRequest notifications; exceptions threw within this optional
+     *    observer are also propagated to the channel, but exceptions from
+     *    the original listener (listener parameter) are privileged 
+     */
     void init(in nsIStreamListener listener,
-              in nsIOutputStream sink);
+              in nsIOutputStream sink,
+              [optional] in nsIRequestObserver requestObserver);
 };

netwerk/base/src/nsStreamListenerTee.cpp

@@ -47,7 +47,13 @@ nsStreamListenerTee::OnStartRequest(nsIRequest *request,
                                     nsISupports *context)
 {
     NS_ENSURE_TRUE(mListener, NS_ERROR_NOT_INITIALIZED);
-    return mListener->OnStartRequest(request, context);
+    nsresult rv1 = mListener->OnStartRequest(request, context);
+    nsresult rv2 = NS_OK;
+    if (mObserver)
+        rv2 = mObserver->OnStartRequest(request, context);
+  
+    // Preserve NS_SUCCESS_XXX in rv1 in case mObserver didn't throw
+    return (NS_FAILED(rv2) && NS_SUCCEEDED(rv1)) ? rv2 : rv1;
 }
 
 NS_IMETHODIMP
@@ -62,7 +68,11 @@ nsStreamListenerTee::OnStopRequest(nsIRequest *request,
         mInputTee = 0;
     }
     mSink = 0;
-    return mListener->OnStopRequest(request, context, status);
+    nsresult rv = mListener->OnStopRequest(request, context, status);
+    if (mObserver)
+        mObserver->OnStopRequest(request, context, status);
+    mObserver = 0;
+    return rv;
 }
 
 NS_IMETHODIMP
@@ -99,9 +109,11 @@ nsStreamListenerTee::OnDataAvailable(nsIRequest *request,
 
 NS_IMETHODIMP
 nsStreamListenerTee::Init(nsIStreamListener *listener,
-                          nsIOutputStream *sink)
+                          nsIOutputStream *sink,
+                          nsIRequestObserver *requestObserver)
 {
     mListener = listener;
     mSink = sink;
+    mObserver = requestObserver;
     return NS_OK;
 }

netwerk/base/src/nsStreamListenerTee.h

@@ -58,6 +58,7 @@ private:
     nsCOMPtr<nsIInputStreamTee> mInputTee;
     nsCOMPtr<nsIStreamListener> mListener;
     nsCOMPtr<nsIOutputStream>   mSink;
+    nsCOMPtr<nsIRequestObserver> mObserver;
 };
 
 #endif

netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp

@@ -1596,7 +1596,7 @@ nsFtpState::InstallCacheListener()
             do_CreateInstance(NS_STREAMLISTENERTEE_CONTRACTID);
     NS_ENSURE_STATE(tee);
 
-    nsresult rv = tee->Init(mChannel->StreamListener(), out);
+    nsresult rv = tee->Init(mChannel->StreamListener(), out, nsnull);
     NS_ENSURE_SUCCESS(rv, rv);
 
     mChannel->SetStreamListener(tee);

netwerk/protocol/http/src/nsHttpChannel.cpp

@@ -2675,7 +2675,7 @@ nsHttpChannel::InstallCacheListener(PRUint32 offset)
         do_CreateInstance(kStreamListenerTeeCID, &rv);
     if (NS_FAILED(rv)) return rv;
 
-    rv = tee->Init(mListener, out);
+    rv = tee->Init(mListener, out, nsnull);
     if (NS_FAILED(rv)) return rv;
 
     mListener = tee;
@@ -2701,7 +2701,7 @@ nsHttpChannel::InstallOfflineCacheListener()
         do_CreateInstance(kStreamListenerTeeCID, &rv);
     if (NS_FAILED(rv)) return rv;
 
-    rv = tee->Init(mListener, out);
+    rv = tee->Init(mListener, out, nsnull);
     if (NS_FAILED(rv)) return rv;
 
     mListener = tee;

netwerk/test/unit/test_traceable_channel.js

@@ -6,43 +6,19 @@
 do_load_httpd_js();
 
 var httpserver = null;
+var pipe = null;
+var streamSink = null;
+
 var originalBody = "original http response body";
-var replacedBody = "replaced http response body";
+var gotOnStartRequest = false;
 
 function TracingListener() {}
 
 TracingListener.prototype = {
-
-  // Replace received response body.
-  onDataAvailable: function(request, context, inputStream,
-                           offset, count) {
-    dump("*** tracing listener onDataAvailable\n");
-    var binaryInputStream = Cc["@mozilla.org/binaryinputstream;1"].
-      createInstance(Components.interfaces.nsIBinaryInputStream);
-    binaryInputStream.setInputStream(inputStream);
-
-    var data = binaryInputStream.readBytes(count);
-    var origBody = originalBody.substr(offset, count);
-    do_check_eq(origBody, data);
-
-    var storageStream = Cc["@mozilla.org/storagestream;1"].
-      createInstance(Components.interfaces.nsIStorageStream);
-    var binaryOutputStream = Cc["@mozilla.org/binaryoutputstream;1"].
-      createInstance(Components.interfaces.nsIBinaryOutputStream);
-
-    storageStream.init(8192, 100, null);
-    binaryOutputStream.setOutputStream(storageStream.getOutputStream(0));
-
-    var newBody = replacedBody.substr(offset, count);
-    binaryOutputStream.writeBytes(newBody, newBody.length);
-
-    this.listener.onDataAvailable(request, context,
-                                  storageStream.newInputStream(0), 0,
-                                  replacedBody.length);
-  },
-
   onStartRequest: function(request, context) {
-    this.listener.onStartRequest(request, context);
+    dump("*** tracing listener onStartRequest\n");
+
+    gotOnStartRequest = true;
 
     // Make sure listener can't be replaced after OnStartRequest was called.
     request.QueryInterface(Components.interfaces.nsITraceableChannel);
@@ -56,13 +32,28 @@ TracingListener.prototype = {
   },
 
   onStopRequest: function(request, context, statusCode) {
-    this.listener.onStopRequest(request, context, statusCode);
+    dump("*** tracing listener onStopRequest\n");
+    
+    do_check_eq(gotOnStartRequest, true);
+    
+    var sin = Components.classes["@mozilla.org/scriptableinputstream;1"].
+        createInstance(Ci.nsIScriptableInputStream);
+
+    streamSink.close();
+    var input = pipe.inputStream;
+    sin.init(input);
+    do_check_eq(sin.available(), originalBody.length);
+    
+    var result = sin.read(originalBody.length);
+    do_check_eq(result, originalBody);
+    
+    input.close();
+    
     httpserver.stop(do_test_finished);
   },
 
   QueryInterface: function(iid) {
-    if (iid.equals(Components.interfaces.nsIStreamListener) ||
-        iid.equals(Components.interfaces.nsIRequestObserver) ||
+    if (iid.equals(Components.interfaces.nsIRequestObserver) ||
         iid.equals(Components.interfaces.nsISupports)
         )
       return this;
@@ -86,14 +77,22 @@ HttpResponseExaminer.prototype = {
   observe: function(subject, topic, data) {
     try {
       subject.QueryInterface(Components.interfaces.nsITraceableChannel);
+      
+      var tee = Cc["@mozilla.org/network/stream-listener-tee;1"].
+          createInstance(Ci.nsIStreamListenerTee);
       var newListener = new TracingListener();
-      newListener.listener = subject.setNewListener(newListener);
+      pipe = Cc["@mozilla.org/pipe;1"].createInstance(Ci.nsIPipe);
+      pipe.init(false, false, 0, 0xffffffff, null);
+      streamSink = pipe.outputStream;
+      
+      var originalListener = subject.setNewListener(tee);
+      tee.init(originalListener, streamSink, newListener);
     } catch(e) {
-      do_throw("can't replace listener" + e);
+      do_throw("can't replace listener " + e);
     }
   },
 
- QueryInterface: function(iid) {
+  QueryInterface: function(iid) {
     if (iid.equals(Components.interfaces.nsIObserver) ||
         iid.equals(Components.interfaces.nsISupportsWeakReference) ||
         iid.equals(Components.interfaces.nsISupports))
@@ -116,8 +115,7 @@ function make_channel(url) {
 }
 
 // Check if received body is correctly modified.
-function get_data(request, input, ctx) {
-  do_check_eq(replacedBody, input);
+function channel_finished(request, input, ctx) {
 }
 
 function run_test() {
@@ -129,6 +127,6 @@ function run_test() {
   httpserver.start(4444);
 
   var channel = make_channel("http://localhost:4444/testdir");
-  channel.asyncOpen(new ChannelListener(get_data), null);
+  channel.asyncOpen(new ChannelListener(channel_finished), null);
   do_test_pending();
 }