bug 599127 - Wyciwyg channel writes end up doing synchronous disk IO. r=jduell, a=betaN
authorMichal Novotny <michal.novotny@gmail.com>
Tue, 23 Nov 2010 14:01:30 +0200
changeset 58031 fc75e982b6dd93d4a730500fec5b84233b4daaa1
parent 58030 86c3a0e9fa87a9b0967b89824c35f9b2f9355590
child 58032 b14f7ebae437a052adb2b841874a6cd8f5db38ca
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersjduell, betaN
bugs599127
milestone2.0b8pre
bug 599127 - Wyciwyg channel writes end up doing synchronous disk IO. r=jduell, a=betaN
netwerk/base/public/nsNetUtil.h
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
netwerk/protocol/wyciwyg/nsWyciwygChannel.h
netwerk/protocol/wyciwyg/nsWyciwygProtocolHandler.cpp
--- a/netwerk/base/public/nsNetUtil.h
+++ b/netwerk/base/public/nsNetUtil.h
@@ -100,16 +100,18 @@
 #include "nsServiceManagerUtils.h"
 #include "nsINestedURI.h"
 #include "nsIMutable.h"
 #include "nsIPropertyBag2.h"
 #include "nsIWritablePropertyBag2.h"
 #include "nsIIDNService.h"
 #include "nsIChannelEventSink.h"
 #include "nsIChannelPolicy.h"
+#include "nsISocketProviderService.h"
+#include "nsISocketProvider.h"
 #include "mozilla/Services.h"
 
 #ifdef MOZILLA_INTERNAL_API
 
 inline already_AddRefed<nsIIOService>
 do_GetIOService(nsresult* error = 0)
 {
     already_AddRefed<nsIIOService> ret = mozilla::services::GetIOService();
@@ -1850,9 +1852,23 @@ NS_CheckIsJavaCompatibleURLString(nsCStr
     compatible = PR_FALSE;
   }
 
   *result = compatible;
 
   return NS_OK;
 }
 
+/**
+ * Make sure Personal Security Manager is initialized
+ */
+inline void
+net_EnsurePSMInit()
+{
+    nsCOMPtr<nsISocketProviderService> spserv =
+            do_GetService(NS_SOCKETPROVIDERSERVICE_CONTRACTID);
+    if (spserv) {
+        nsCOMPtr<nsISocketProvider> provider;
+        spserv->GetSocketProvider("ssl", getter_AddRefs(provider));
+    }
+}
+
 #endif // !nsNetUtil_h__
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -1481,22 +1481,17 @@ nsHttpHandler::NewProxiedChannel(nsIURI 
         if (mPipeliningOverSSL)
             caps |= NS_HTTP_ALLOW_PIPELINING;
 
 #ifdef MOZ_IPC
         if (!IsNeckoChild()) 
 #endif
         {
             // HACK: make sure PSM gets initialized on the main thread.
-            nsCOMPtr<nsISocketProviderService> spserv =
-                    do_GetService(NS_SOCKETPROVIDERSERVICE_CONTRACTID);
-            if (spserv) {
-                nsCOMPtr<nsISocketProvider> provider;
-                spserv->GetSocketProvider("ssl", getter_AddRefs(provider));
-            }
+            net_EnsurePSMInit();
         }
     }
 
     rv = httpChannel->Init(uri, caps, proxyInfo);
     if (NS_FAILED(rv))
         return rv;
 
     httpChannel.forget(result);
--- a/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
+++ b/netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp
@@ -17,16 +17,17 @@
  *
  * The Initial Developer of the Original Code is
  * Netscape Communications Corporation.
  * Portions created by the Initial Developer are Copyright (C) 1998
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *   Radha Kulkarni(radha@netscape.com)
+ *   Michal Novotny <michal.novotny@gmail.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
  * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -44,45 +45,121 @@
 #include "nsIScriptSecurityManager.h"
 #include "nsNetUtil.h"
 #include "nsContentUtils.h"
 #include "nsICacheService.h"
 #include "nsICacheSession.h"
 #include "nsIParser.h"
 #include "nsThreadUtils.h"
 
+// Must delete nsWyciwygChannel on main thread (URI destructor not thread
+// safe), so Async IO events send event to Release channel on main thread
+class nsWyciwygAsyncEvent : public nsRunnable {
+public:
+  nsWyciwygAsyncEvent(nsWyciwygChannel *aChannel) : mChannel(aChannel) {}
+
+  ~nsWyciwygAsyncEvent() 
+  {
+    // NS_NewRunnableMethod addrefs channel and will release it on main
+    // thread--which is the goal here--so target method is a no-op
+    nsCOMPtr<nsIRunnable> event =
+      NS_NewRunnableMethod(mChannel, &nsWyciwygChannel::MainReleaseNoOp);
+
+    // ensure we free our ref before event gets released on main thread
+    mChannel = 0;
+    NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL);
+  }
+protected:
+  nsRefPtr<nsWyciwygChannel> mChannel;
+};
+
+class nsWyciwygSetCharsetandSourceEvent : public nsWyciwygAsyncEvent {
+public:
+  nsWyciwygSetCharsetandSourceEvent(nsWyciwygChannel *aChannel)
+    : nsWyciwygAsyncEvent(aChannel) {}
+
+  NS_IMETHOD Run()
+  {
+    mChannel->SetCharsetAndSourceInternal();
+    return NS_OK;
+  }
+};
+
+class nsWyciwygWriteEvent : public nsWyciwygAsyncEvent {
+public:
+  nsWyciwygWriteEvent(nsWyciwygChannel *aChannel, const nsAString &aData, 
+                      const nsACString &spec)
+    : nsWyciwygAsyncEvent(aChannel), mData(aData), mSpec(spec) {}
+
+  NS_IMETHOD Run()
+  {
+    mChannel->WriteToCacheEntryInternal(mData, mSpec);
+    return NS_OK;
+  }
+private:
+  nsString mData;
+  nsCString mSpec;
+};
+
+class nsWyciwygCloseEvent : public nsWyciwygAsyncEvent {
+public:
+  nsWyciwygCloseEvent(nsWyciwygChannel *aChannel, nsresult aReason)
+    : nsWyciwygAsyncEvent(aChannel), mReason(aReason) {}
+
+  NS_IMETHOD Run()
+  {
+    mChannel->CloseCacheEntryInternal(mReason);
+    return NS_OK;
+  }
+private:
+  nsresult mReason;
+};
+
+
 // nsWyciwygChannel methods 
 nsWyciwygChannel::nsWyciwygChannel()
   : mStatus(NS_OK),
     mIsPending(PR_FALSE),
+    mCharsetAndSourceSet(PR_FALSE),
     mNeedToWriteCharset(PR_FALSE),
     mCharsetSource(kCharsetUninitialized),
     mContentLength(-1),
     mLoadFlags(LOAD_NORMAL)
 {
 }
 
 nsWyciwygChannel::~nsWyciwygChannel() 
 {
 }
 
-NS_IMPL_ISUPPORTS6(nsWyciwygChannel,
-                   nsIChannel,
-                   nsIRequest,
-                   nsIStreamListener,
-                   nsIRequestObserver,
-                   nsICacheListener, 
-                   nsIWyciwygChannel)
+NS_IMPL_THREADSAFE_ISUPPORTS6(nsWyciwygChannel,
+                              nsIChannel,
+                              nsIRequest,
+                              nsIStreamListener,
+                              nsIRequestObserver,
+                              nsICacheListener, 
+                              nsIWyciwygChannel)
 
 nsresult
 nsWyciwygChannel::Init(nsIURI* uri)
 {
   NS_ENSURE_ARG_POINTER(uri);
+
+  nsresult rv;
+
   mURI = uri;
   mOriginalURI = uri;
+
+  nsCOMPtr<nsICacheService> serv =
+    do_GetService(NS_CACHESERVICE_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = serv->GetCacheIOTarget(getter_AddRefs(mCacheIOTarget));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   return NS_OK;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // nsIRequest methods:
 ///////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP
@@ -303,40 +380,29 @@ nsWyciwygChannel::AsyncOpen(nsIStreamLis
 
   NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
   NS_ENSURE_ARG_POINTER(listener);
 
   nsCAutoString spec;
   mURI->GetSpec(spec);
 
   // open a cache entry for this channel...
-  PRBool delayed = PR_FALSE;
-  nsresult rv = OpenCacheEntry(spec, nsICache::ACCESS_READ, &delayed);
+  nsresult rv = OpenCacheEntry(spec, nsICache::ACCESS_READ);
   if (rv == NS_ERROR_CACHE_KEY_NOT_FOUND) {
-    nsCOMPtr<nsIRunnable> ev =
-      NS_NewRunnableMethod(this, &nsWyciwygChannel::NotifyListener);
     // Overwrite rv on purpose; if event dispatch fails we'll bail, and
     // otherwise we'll wait until the event fires before calling back.
-    rv = NS_DispatchToCurrentThread(ev);
-    delayed = PR_TRUE;
+    rv = NS_DispatchToCurrentThread(
+            NS_NewRunnableMethod(this, &nsWyciwygChannel::NotifyListener));
   }
 
   if (NS_FAILED(rv)) {
     LOG(("nsWyciwygChannel::OpenCacheEntry failed [rv=%x]\n", rv));
     return rv;
   }
 
-  if (!delayed) {
-    rv = ReadFromCache();
-    if (NS_FAILED(rv)) {
-      LOG(("nsWyciwygChannel::ReadFromCache failed [rv=%x]\n", rv));
-      return rv;
-    }
-  }
-
   mIsPending = PR_TRUE;
   mListener = listener;
   mListenerContext = ctx;
 
   if (mLoadGroup)
     mLoadGroup->AddRequest(this, nsnull);
 
   return NS_OK;
@@ -344,22 +410,34 @@ nsWyciwygChannel::AsyncOpen(nsIStreamLis
 
 //////////////////////////////////////////////////////////////////////////////
 // nsIWyciwygChannel
 //////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP
 nsWyciwygChannel::WriteToCacheEntry(const nsAString &aData)
 {
+  // URIs not thread-safe, so get spec now in case we need it
+  nsCAutoString spec;
+  nsresult rv = mURI->GetAsciiSpec(spec);
+  if (NS_FAILED(rv)) 
+    return rv;
+
+  return mCacheIOTarget->Dispatch(new nsWyciwygWriteEvent(this, aData, spec),
+                                  NS_DISPATCH_NORMAL);
+}
+
+nsresult
+nsWyciwygChannel::WriteToCacheEntryInternal(const nsAString &aData, const nsACString& spec)
+{
+  NS_ASSERTION(IsOnCacheIOThread(), "wrong thread");
+
   nsresult rv;
 
   if (!mCacheEntry) {
-    nsCAutoString spec;
-    rv = mURI->GetAsciiSpec(spec);
-    if (NS_FAILED(rv)) return rv;
     rv = OpenCacheEntry(spec, nsICache::ACCESS_WRITE);
     if (NS_FAILED(rv)) return rv;
   }
 
   if (mSecurityInfo) {
     mCacheEntry->SetSecurityInfo(mSecurityInfo);
   }
 
@@ -384,18 +462,27 @@ nsWyciwygChannel::WriteToCacheEntry(cons
   return mCacheOutputStream->Write((char *)PromiseFlatString(aData).get(),
                                    aData.Length() * sizeof(PRUnichar), &out);
 }
 
 
 NS_IMETHODIMP
 nsWyciwygChannel::CloseCacheEntry(nsresult reason)
 {
+  return mCacheIOTarget->Dispatch(new nsWyciwygCloseEvent(this, reason),
+                                  NS_DISPATCH_NORMAL);
+}
+
+nsresult
+nsWyciwygChannel::CloseCacheEntryInternal(nsresult reason)
+{
+  NS_ASSERTION(IsOnCacheIOThread(), "wrong thread");
+
   if (mCacheEntry) {
-    LOG(("nsWyciwygChannel::CloseCacheEntry [this=%x ]", this));
+    LOG(("nsWyciwygChannel::CloseCacheEntryInternal [this=%x ]", this));
     mCacheOutputStream = 0;
     mCacheInputStream = 0;
 
     if (NS_FAILED(reason))
       mCacheEntry->Doom();
 
     mCacheEntry = 0;
   }
@@ -411,30 +498,45 @@ nsWyciwygChannel::SetSecurityInfo(nsISup
 }
 
 NS_IMETHODIMP
 nsWyciwygChannel::SetCharsetAndSource(PRInt32 aSource,
                                       const nsACString& aCharset)
 {
   NS_ENSURE_ARG(!aCharset.IsEmpty());
 
+  mCharsetAndSourceSet = PR_TRUE;
+  mCharset = aCharset;
+  mCharsetSource = aSource;
+
+  return mCacheIOTarget->Dispatch(new nsWyciwygSetCharsetandSourceEvent(this),
+                                  NS_DISPATCH_NORMAL);
+}
+
+void
+nsWyciwygChannel::SetCharsetAndSourceInternal()
+{
+  NS_ASSERTION(IsOnCacheIOThread(), "wrong thread");
+
   if (mCacheEntry) {
-    WriteCharsetAndSourceToCache(aSource, PromiseFlatCString(aCharset));
+    WriteCharsetAndSourceToCache(mCharsetSource, mCharset);
   } else {
     mNeedToWriteCharset = PR_TRUE;
-    mCharsetSource = aSource;
-    mCharset = aCharset;
   }
-
-  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWyciwygChannel::GetCharsetAndSource(PRInt32* aSource, nsACString& aCharset)
 {
+  if (mCharsetAndSourceSet) {
+    *aSource = mCharsetSource;
+    aCharset = mCharset;
+    return NS_OK;
+  }
+
   if (!mCacheEntry) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsXPIDLCString data;
   mCacheEntry->GetMetaDataElement("charset", getter_Copies(data));
 
   if (data.IsEmpty()) {
@@ -562,62 +664,43 @@ nsWyciwygChannel::OnStopRequest(nsIReque
 }
 
 //////////////////////////////////////////////////////////////////////////////
 // Helper functions
 //////////////////////////////////////////////////////////////////////////////
 
 nsresult
 nsWyciwygChannel::OpenCacheEntry(const nsACString & aCacheKey,
-                                 nsCacheAccessMode aAccessMode,
-                                 PRBool * aDelayFlag)
+                                 nsCacheAccessMode aAccessMode)
 {
   nsresult rv = NS_ERROR_FAILURE;
   // Get cache service
   nsCOMPtr<nsICacheService> cacheService =
     do_GetService(NS_CACHESERVICE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsXPIDLCString spec;    
-  nsAutoString newURIString;    
-  nsCOMPtr<nsICacheSession> cacheSession;
-
   // honor security settings
   nsCacheStoragePolicy storagePolicy;
   if (mLoadFlags & INHIBIT_PERSISTENT_CACHING)
     storagePolicy = nsICache::STORE_IN_MEMORY;
   else
     storagePolicy = nsICache::STORE_ANYWHERE;
- 
+
+  nsCOMPtr<nsICacheSession> cacheSession;
   // Open a stream based cache session.
   rv = cacheService->CreateSession("wyciwyg", storagePolicy, PR_TRUE,
                                    getter_AddRefs(cacheSession));
   if (!cacheSession) 
     return NS_ERROR_FAILURE;
 
-  /* we'll try to synchronously open the cache entry... however, it
-   * may be in use and not yet validated, in which case we'll try
-   * asynchronously opening the cache entry.
-   */
-    
-  rv = cacheSession->OpenCacheEntry(aCacheKey, aAccessMode, PR_FALSE,
-                                    getter_AddRefs(mCacheEntry));
-
-  if (rv == NS_ERROR_CACHE_WAIT_FOR_VALIDATION) {
-    // access to the cache entry has been denied. Let's try opening it
-    // async.
+  if (aAccessMode == nsICache::ACCESS_WRITE)
+    rv = cacheSession->OpenCacheEntry(aCacheKey, aAccessMode, PR_FALSE,
+                                      getter_AddRefs(mCacheEntry));
+  else
     rv = cacheSession->AsyncOpenCacheEntry(aCacheKey, aAccessMode, this);
-    if (NS_FAILED(rv)) 
-      return rv;
-    if (aDelayFlag)
-      *aDelayFlag = PR_TRUE;
-  }
-  else if (rv == NS_OK) {
-    LOG(("nsWyciwygChannel::OpenCacheEntry got cache entry \n"));
-  }
 
   return rv;
 }
 
 nsresult
 nsWyciwygChannel::ReadFromCache()
 {
   LOG(("nsWyciwygChannel::ReadFromCache [this=%x] ", this));
@@ -640,16 +723,17 @@ nsWyciwygChannel::ReadFromCache()
   // Pump the cache data downstream
   return mPump->AsyncRead(this, nsnull);
 }
 
 void
 nsWyciwygChannel::WriteCharsetAndSourceToCache(PRInt32 aSource,
                                                const nsCString& aCharset)
 {
+  NS_ASSERTION(IsOnCacheIOThread(), "wrong thread");
   NS_PRECONDITION(mCacheEntry, "Better have cache entry!");
   
   mCacheEntry->SetMetaDataElement("charset", aCharset.get());
 
   nsCAutoString source;
   source.AppendInt(aSource);
   mCacheEntry->SetMetaDataElement("charset-source", source.get());
 }
@@ -667,9 +751,17 @@ nsWyciwygChannel::NotifyListener()
   mIsPending = PR_FALSE;
 
   // Remove ourselves from the load group.
   if (mLoadGroup) {
     mLoadGroup->RemoveRequest(this, nsnull, mStatus);
   }
 }
 
+PRBool
+nsWyciwygChannel::IsOnCacheIOThread()
+{
+  PRBool correctThread;
+  mCacheIOTarget->IsOnCurrentThread(&correctThread);
+  return correctThread;
+}
+
 // vim: ts=2 sw=2
--- a/netwerk/protocol/wyciwyg/nsWyciwygChannel.h
+++ b/netwerk/protocol/wyciwyg/nsWyciwygChannel.h
@@ -50,16 +50,17 @@
 #include "nsIInputStream.h"
 #include "nsIInputStreamPump.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIProgressEventSink.h"
 #include "nsIStreamListener.h"
 #include "nsICacheListener.h"
 #include "nsICacheEntryDescriptor.h"
 #include "nsIURI.h"
+#include "nsIEventTarget.h"
 
 extern PRLogModuleInfo * gWyciwygLog;
 
 //-----------------------------------------------------------------------------
 
 class nsWyciwygChannel: public nsIWyciwygChannel,
                         public nsIStreamListener,
                         public nsICacheListener
@@ -68,33 +69,46 @@ public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSIREQUEST
     NS_DECL_NSICHANNEL
     NS_DECL_NSIWYCIWYGCHANNEL
     NS_DECL_NSIREQUESTOBSERVER
     NS_DECL_NSISTREAMLISTENER
     NS_DECL_NSICACHELISTENER
 
+    friend class nsWyciwygSetCharsetandSourceEvent;
+    friend class nsWyciwygWriteEvent;
+    friend class nsWyciwygCloseEvent;
+
     // nsWyciwygChannel methods:
     nsWyciwygChannel();
     virtual ~nsWyciwygChannel();
 
     nsresult Init(nsIURI *uri);
 
+    // No-op event target method for releasing on main thread
+    void MainReleaseNoOp() {}
+
 protected:
+    nsresult WriteToCacheEntryInternal(const nsAString& aData, const nsACString& spec);
+    void SetCharsetAndSourceInternal();
+    nsresult CloseCacheEntryInternal(nsresult reason);
+
     nsresult ReadFromCache();
-    nsresult OpenCacheEntry(const nsACString & aCacheKey, nsCacheAccessMode aWriteAccess, PRBool * aDelayFlag = nsnull);
+    nsresult OpenCacheEntry(const nsACString & aCacheKey, nsCacheAccessMode aWriteAccess);
 
     void WriteCharsetAndSourceToCache(PRInt32 aSource,
                                       const nsCString& aCharset);
 
     void NotifyListener();
-       
+    PRBool IsOnCacheIOThread();
+
     nsresult                            mStatus;
     PRPackedBool                        mIsPending;
+    PRPackedBool                        mCharsetAndSourceSet;
     PRPackedBool                        mNeedToWriteCharset;
     PRInt32                             mCharsetSource;
     nsCString                           mCharset;
     PRInt32                             mContentLength;
     PRUint32                            mLoadFlags;
     nsCOMPtr<nsIURI>                    mURI;
     nsCOMPtr<nsIURI>                    mOriginalURI;
     nsCOMPtr<nsISupports>               mOwner;
@@ -106,13 +120,14 @@ protected:
 
     // reuse as much of this channel implementation as we can
     nsCOMPtr<nsIInputStreamPump>        mPump;
     
     // Cache related stuff    
     nsCOMPtr<nsICacheEntryDescriptor>   mCacheEntry;
     nsCOMPtr<nsIOutputStream>           mCacheOutputStream;
     nsCOMPtr<nsIInputStream>            mCacheInputStream;
+    nsCOMPtr<nsIEventTarget>            mCacheIOTarget;
 
     nsCOMPtr<nsISupports>               mSecurityInfo;
 };
 
 #endif /* nsWyciwygChannel_h___ */
--- a/netwerk/protocol/wyciwyg/nsWyciwygProtocolHandler.cpp
+++ b/netwerk/protocol/wyciwyg/nsWyciwygProtocolHandler.cpp
@@ -38,16 +38,18 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsWyciwyg.h"
 #include "nsWyciwygChannel.h"
 #include "nsWyciwygProtocolHandler.h"
 #include "nsIURL.h"
 #include "nsIComponentManager.h"
 #include "nsNetCID.h"
+#include "nsServiceManagerUtils.h"
+#include "plstr.h"
 
 #ifdef MOZ_IPC
 #include "mozilla/net/NeckoChild.h"
 #endif
 
 #ifdef MOZ_IPC
 using namespace mozilla::net;
 #include "mozilla/net/WyciwygChannelChild.h"
@@ -140,29 +142,38 @@ nsWyciwygProtocolHandler::NewChannel(nsI
 
     channel = wcc;
     rv = wcc->Init(url);
     if (NS_FAILED(rv))
       PWyciwygChannelChild::Send__delete__(wcc);
   } else
 #endif
   {
+    // If original channel used https, make sure PSM is initialized
+    // (this may be first channel to load during a session restore)
+    nsCAutoString path;
+    rv = url->GetPath(path);
+    NS_ENSURE_SUCCESS(rv, rv);
+    PRInt32 slashIndex = path.FindChar('/', 2);
+    if (slashIndex == kNotFound)
+      return NS_ERROR_FAILURE;
+    if (path.Length() < (PRUint32)slashIndex + 1 + 5)
+      return NS_ERROR_FAILURE;
+    if (!PL_strncasecmp(path.get() + slashIndex + 1, "https", 5))
+      net_EnsurePSMInit();
+
     nsWyciwygChannel *wc = new nsWyciwygChannel();
-    if (!wc)
-      return NS_ERROR_OUT_OF_MEMORY;
     channel = wc;
     rv = wc->Init(url);
   }
 
+  if (NS_FAILED(rv))
+    return rv;
+
   *result = channel.forget().get();
-  if (NS_FAILED(rv)) {
-    NS_RELEASE(*result);
-    return rv;
-  }
-
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWyciwygProtocolHandler::GetProtocolFlags(PRUint32 *result) 
 {
   // Should this be an an nsINestedURI?  We don't really want random webpages
   // loading these URIs...