Failure to open a channel in a stream loader should just make init() throw.

Bug 310319, r+sr=darin

calendar/resources/content/calendarManager.js

@@ -959,7 +959,11 @@ calendarManager.prototype.getRemoteCalendarText = function calMan_getRemoteCalen
    var myInstance = Components.classes["@mozilla.org/network/stream-loader;1"].createInstance(Components.interfaces.nsIStreamLoader);
    dump( "init channel, \nChannel is "+Channel+"\nURL is "+Channel.URI.spec+"\n" );
    window.setCursor( "wait" );
-   myInstance.init( Channel, Listener, null );
+   try {
+     myInstance.init( Channel, Listener, null );
+   } catch (e) {
+     window.setCursor( "auto" );
+   }
 }
 
 calendarManager.prototype.getProfileDirectory = function calMan_getProfileDirectory()

content/xul/document/src/nsXULDocument.cpp

@@ -3237,17 +3237,15 @@ nsXULDocument::LoadScript(nsXULPrototypeScript* aScriptProto, PRBool* aBlock)
         NS_ADDREF_THIS();
     }
     else {
-        // Set mSrcLoading *before* calling NS_NewStreamLoader, in case the
-        // stream completes (probably due to an error) within the activation
-        // of NS_NewStreamLoader.
-        aScriptProto->mSrcLoading = PR_TRUE;
-
         nsCOMPtr<nsILoadGroup> group = do_QueryReferent(mDocumentLoadGroup);
 
-        // N.B., the loader will be released in OnStreamComplete
+        // Note: the loader will keep itself alive while it's loading.
-        nsIStreamLoader* loader;
+        nsCOMPtr<nsIStreamLoader> loader;
-        rv = NS_NewStreamLoader(&loader, aScriptProto->mSrcURI, this, nsnull, group);
+        rv = NS_NewStreamLoader(getter_AddRefs(loader), aScriptProto->mSrcURI,
+                                this, nsnull, group);
         if (NS_FAILED(rv)) return rv;
+
+        aScriptProto->mSrcLoading = PR_TRUE;
     }
 
     // Block until OnStreamComplete resumes us.
@@ -3382,9 +3380,6 @@ nsXULDocument::OnStreamComplete(nsIStreamLoader* aLoader,
         // ignore any evaluation errors
     }
 
-    // balance the addref we added in LoadScript()
-    NS_RELEASE(aLoader);
-
     rv = ResumeWalk();
 
     // Load a pointer to the prototype-script's list of nsXULDocuments who

netwerk/base/public/nsIStreamLoader.idl

@@ -47,6 +47,19 @@ interface nsIInterfaceRequestor;
 [scriptable, uuid(359F7990-D4E9-11d3-A1A5-0050041CAF44)]
 interface nsIStreamLoaderObserver : nsISupports
 {
+    /**
+     * Called when the entire stream has been loaded.
+     *
+     * @param loader the stream loader that loaded the stream.
+     * @param context the aContext parameter passed to the loader's init method
+     * @param status the status of the underlying channel
+     * @param resultLength the length of the data loaded
+     * @param result the data
+     *
+     * This method will always be called asynchronously by the
+     * nsIStreamLoader involved, on the thread that called the
+     * loader's init() method.
+     */
     void onStreamComplete(in nsIStreamLoader loader,
                           in nsISupports ctxt,
                           in nsresult status,
@@ -72,8 +85,10 @@ interface nsIStreamLoader : nsISupports
      * @param aContext
      *        May be null. Will be passed to the observer.
      *
-     * @note Failure to open the channel will be indicated by an async callback
+     * @note Failure to open the channel will cause init() to throw.  A
-     *       to the observer.
+     *       successful call to init() means that onStreamComplete is
+     *       guaranteed to be called on aObserver asynchronously (i.e. after
+     *       init() returns).
      */
     void init(in nsIChannel aChannel,
               in nsIStreamLoaderObserver aObserver,

netwerk/base/public/nsIUnicharStreamLoader.idl

@@ -68,6 +68,10 @@ interface nsIUnicharStreamLoaderObserver : nsISupports
    * @param aStatus the status of the underlying channel
    * @param aUnicharData the unichar input stream containing the data.  This
    *        can be null in some failure conditions.
+   *
+   * This method will always be called asynchronously by the
+   * nsUnicharIStreamLoader involved, on the thread that called the
+   * loader's init() method.
    */
   void onStreamComplete(in nsIUnicharStreamLoader aLoader,
                         in nsISupports aContext,
@@ -89,6 +93,10 @@ interface nsIUnicharStreamLoader : nsISupports
    *        the load is complete
    * @param aContext an opaque context pointer
    * @param aSegmentSize the size of the segments to use for the data, in bytes
+   *
+   * @note Failure to open the channel will cause init() to throw.  A
+   *       successful call to init() means that onStreamComplete is guaranteed
+   *       to be called on aObserver asynchronously (i.e. after init() returns).
    */
   
   void init(in nsIChannel aChannel,

netwerk/base/src/nsStreamLoader.cpp

@@ -51,24 +51,7 @@ nsStreamLoader::Init(nsIChannel *channel,
   NS_ENSURE_ARG_POINTER(observer);
 
   nsresult rv = channel->AsyncOpen(this, context);
-
+  NS_ENSURE_SUCCESS(rv, rv);
-  if (NS_FAILED(rv) && observer) {
-    // don't callback synchronously as it puts the caller
-    // in a recursive situation and breaks the asynchronous
-    // semantics of nsIStreamLoader
-    nsresult rv2 = NS_OK;
-    nsCOMPtr<nsIProxyObjectManager> pIProxyObjectManager = 
-             do_GetService(kProxyObjectManagerCID, &rv2);
-    if (NS_FAILED(rv2)) return rv2;
-
-    nsCOMPtr<nsIStreamLoaderObserver> pObserver;
-    rv2 = pIProxyObjectManager->GetProxyForObject(NS_CURRENT_EVENTQ, 
-              NS_GET_IID(nsIStreamLoaderObserver), observer, 
-              PROXY_ASYNC | PROXY_ALWAYS, getter_AddRefs(pObserver));
-    if (NS_FAILED(rv2)) return rv2;
-
-    rv = pObserver->OnStreamComplete(this, context, rv, 0, nsnull);
-  }
 
   mObserver = observer;
   mContext  = context;

netwerk/base/src/nsUnicharStreamLoader.cpp

@@ -66,29 +66,7 @@ nsUnicharStreamLoader::Init(nsIChannel *aChannel,
   }
   
   nsresult rv = aChannel->AsyncOpen(this, aContext);
-
+  NS_ENSURE_SUCCESS(rv, rv);
-  if (NS_FAILED(rv)) {
-    // don't callback synchronously as it puts the caller
-    // in a recursive situation and breaks the asynchronous
-    // semantics of nsIStreamLoader
-    nsresult rv2 = NS_OK;
-    nsCOMPtr<nsIProxyObjectManager> pIProxyObjectManager = 
-      do_GetService(kProxyObjectManagerCID, &rv2);
-    if (NS_FAILED(rv2))
-      return rv2;
-
-    nsCOMPtr<nsIUnicharStreamLoaderObserver> pObserver;
-    rv2 =
-      pIProxyObjectManager->GetProxyForObject(NS_CURRENT_EVENTQ, 
-                                              NS_GET_IID(nsIUnicharStreamLoaderObserver),
-                                              aObserver, 
-                                              PROXY_ASYNC | PROXY_ALWAYS,
-                                              getter_AddRefs(pObserver));
-    if (NS_FAILED(rv2))
-      return rv2;
-
-    rv = pObserver->OnStreamComplete(this, aContext, rv, nsnull);
-  }
 
   mObserver = aObserver;
   mContext = aContext;