Bug 703882 - crash nsExternalAppHandler::nsExternalAppHandler. r=bz
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 16 Jan 2012 12:55:13 +0900
changeset 85799 fb92bdd1ab2db1596af2e60f66af2fd00af4db29
parent 85798 0fe6641a2076a582ce040a83e1672f81c3f49a39
child 85800 4facc14043a68c8eb5a041fb69e0f9774a2d0faf
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs703882
milestone12.0a1
Bug 703882 - crash nsExternalAppHandler::nsExternalAppHandler. r=bz
uriloader/exthandler/nsExternalHelperAppService.cpp
uriloader/exthandler/nsExternalHelperAppService.h
uriloader/exthandler/nsExternalProtocolHandler.cpp
uriloader/exthandler/nsExternalProtocolHandler.h
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -178,21 +178,16 @@ PRLogModuleInfo* nsExternalHelperAppServ
 #define LOG(args) PR_LOG(mLog, 3, args)
 #define LOG_ENABLED() PR_LOG_TEST(mLog, 3)
 
 static const char NEVER_ASK_FOR_SAVE_TO_DISK_PREF[] =
   "browser.helperApps.neverAsk.saveToDisk";
 static const char NEVER_ASK_FOR_OPEN_FILE_PREF[] =
   "browser.helperApps.neverAsk.openFile";
 
-/**
- * Contains a pointer to the helper app service, set in its constructor
- */
-nsExternalHelperAppService* gExtProtSvc;
-
 // Helper functions for Content-Disposition headers
 
 /**
  * Given a URI fragment, unescape it
  * @param aFragment The string to unescape
  * @param aURI The URI from which this fragment is taken. Only its character set
  *             will be used.
  * @param aResult [out] Unescaped string.
@@ -537,17 +532,16 @@ NS_IMPL_ISUPPORTS6(
   nsIExternalProtocolService,
   nsIMIMEService,
   nsIObserver,
   nsISupportsWeakReference)
 
 nsExternalHelperAppService::nsExternalHelperAppService() :
   mInPrivateBrowsing(false)
 {
-  gExtProtSvc = this;
 }
 nsresult nsExternalHelperAppService::Init()
 {
   nsCOMPtr<nsIPrivateBrowsingService> pbs =
     do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID);
   if (pbs) {
     pbs->GetPrivateBrowsingEnabled(&mInPrivateBrowsing);
   }
@@ -567,17 +561,16 @@ nsresult nsExternalHelperAppService::Ini
 
   nsresult rv = obs->AddObserver(this, "profile-before-change", true);
   NS_ENSURE_SUCCESS(rv, rv);
   return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true);
 }
 
 nsExternalHelperAppService::~nsExternalHelperAppService()
 {
-  gExtProtSvc = nsnull;
 }
 
 static PRInt64 GetContentLengthAsInt64(nsIRequest *request)
 {
   PRInt64 contentLength = -1;
   nsresult rv;
   nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(request, &rv));
   if (props)
@@ -642,17 +635,18 @@ NS_IMETHODIMP nsExternalHelperAppService
                                                   disp,
                                                   aForceSave, contentLength,
                                                   IPC::URI(referrer));
     ExternalHelperAppChild *childListener = static_cast<ExternalHelperAppChild *>(pc);
 
     NS_ADDREF(*aStreamListener = childListener);
 
     nsRefPtr<nsExternalAppHandler> handler =
-      new nsExternalAppHandler(nsnull, EmptyCString(), aWindowContext, fileName,
+      new nsExternalAppHandler(nsnull, EmptyCString(), aWindowContext, this,
+                               fileName,
                                reason, aForceSave);
     if (!handler)
       return NS_ERROR_OUT_OF_MEMORY;
     
     childListener->SetHandler(handler);
 
     return NS_OK;
   }
@@ -755,16 +749,17 @@ NS_IMETHODIMP nsExternalHelperAppService
   // We want the mimeInfo's primary extension to pass it to
   // nsExternalAppHandler
   nsCAutoString buf;
   mimeInfo->GetPrimaryExtension(buf);
 
   nsExternalAppHandler * handler = new nsExternalAppHandler(mimeInfo,
                                                             buf,
                                                             aWindowContext,
+                                                            this,
                                                             fileName,
                                                             reason,
                                                             aForceSave);
   if (!handler)
     return NS_ERROR_OUT_OF_MEMORY;
   NS_ADDREF(*aStreamListener = handler);
   
   return NS_OK;
@@ -1115,16 +1110,17 @@ NS_INTERFACE_MAP_BEGIN(nsExternalAppHand
    NS_INTERFACE_MAP_ENTRY(nsIHelperAppLauncher)   
    NS_INTERFACE_MAP_ENTRY(nsICancelable)
    NS_INTERFACE_MAP_ENTRY(nsITimerCallback)
 NS_INTERFACE_MAP_END_THREADSAFE
 
 nsExternalAppHandler::nsExternalAppHandler(nsIMIMEInfo * aMIMEInfo,
                                            const nsCSubstring& aTempFileExtension,
                                            nsIInterfaceRequestor* aWindowContext,
+                                           nsExternalHelperAppService *aExtProtSvc,
                                            const nsAString& aSuggestedFilename,
                                            PRUint32 aReason, bool aForceSave)
 : mMimeInfo(aMIMEInfo)
 , mWindowContext(aWindowContext)
 , mWindowToClose(nsnull)
 , mSuggestedFileName(aSuggestedFilename)
 , mForceSave(aForceSave)
 , mCanceled(false)
@@ -1133,16 +1129,17 @@ nsExternalAppHandler::nsExternalAppHandl
 , mStopRequestIssued(false)
 , mProgressListenerInitialized(false)
 , mReason(aReason)
 , mContentLength(-1)
 , mProgress(0)
 , mDataBuffer(nsnull)
 , mKeepRequestAlive(false)
 , mRequest(nsnull)
+, mExtProtSvc(aExtProtSvc)
 {
 
   // make sure the extention includes the '.'
   if (!aTempFileExtension.IsEmpty() && aTempFileExtension.First() != '.')
     mTempFileExtension = PRUnichar('.');
   AppendUTF8toUTF16(aTempFileExtension, mTempFileExtension);
 
   // replace platform specific path separator and illegal characters to avoid any confusion
@@ -1160,29 +1157,24 @@ nsExternalAppHandler::nsExternalAppHandl
   for (PRUint32 i = 0; i < ArrayLength(unsafeBidiCharacters); ++i) {
     mSuggestedFileName.ReplaceChar(unsafeBidiCharacters[i], '_');
     mTempFileExtension.ReplaceChar(unsafeBidiCharacters[i], '_');
   }
   
   // Make sure extension is correct.
   EnsureSuggestedFileName();
 
-  gExtProtSvc->AddRef();
-
   mBufferSize = Preferences::GetUint("network.buffer.cache.size", 4096);
   mDataBuffer = (char*) malloc(mBufferSize);
   if (!mDataBuffer)
     return;
 }
 
 nsExternalAppHandler::~nsExternalAppHandler()
 {
-  // Not using NS_RELEASE, since we don't want to set gExtProtSvc to NULL
-  gExtProtSvc->Release();
-
   if (mDataBuffer)
     free(mDataBuffer);
 }
 
 NS_IMETHODIMP nsExternalAppHandler::SetWebProgressListener(nsIWebProgressListener2 * aWebProgressListener)
 { 
   // this call back means we've successfully brought up the 
   // progress window so set the appropriate flag, even though
@@ -1508,18 +1500,17 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
           bool hasMore;
           rv = encEnum->HasMore(&hasMore);
           if (NS_SUCCEEDED(rv) && hasMore)
           {
             nsCAutoString encType;
             rv = encEnum->GetNext(encType);
             if (NS_SUCCEEDED(rv) && !encType.IsEmpty())
             {
-              NS_ASSERTION(gExtProtSvc, "Where did the service go?");
-              gExtProtSvc->ApplyDecodingForExtension(extension, encType,
+              mExtProtSvc->ApplyDecodingForExtension(extension, encType,
                                                      &applyConversion);
             }
           }
         }
       }    
     }
 
     encChannel->SetApplyConversion( applyConversion );
@@ -1565,17 +1556,16 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
   mMimeInfo->GetAlwaysAskBeforeHandling(&alwaysAsk);
   if (alwaysAsk)
   {
     // But we *don't* ask if this mimeInfo didn't come from
     // our user configuration datastore and the user has said
     // at some point in the distant past that they don't
     // want to be asked.  The latter fact would have been
     // stored in pref strings back in the old days.
-    NS_ASSERTION(gExtProtSvc, "Service gone away!?");
 
     bool mimeTypeIsInDatastore = false;
     nsCOMPtr<nsIHandlerService> handlerSvc = do_GetService(NS_HANDLERSERVICE_CONTRACTID);
     if (handlerSvc)
       handlerSvc->Exists(mMimeInfo, &mimeTypeIsInDatastore);
     if (!handlerSvc || !mimeTypeIsInDatastore)
     {
       nsCAutoString MIMEType;
@@ -1910,17 +1900,17 @@ nsresult nsExternalAppHandler::ExecuteDe
       if (action == nsIMIMEInfo::useHelperApp ||
           action == nsIMIMEInfo::useSystemDefault)
       {
         rv = OpenWithApplication();
       }
       else if(action == nsIMIMEInfo::saveToDisk)
       {
         nsCOMPtr<nsILocalFile> destfile(do_QueryInterface(mFinalFileDestination));
-        gExtProtSvc->FixFilePermissions(destfile);
+        mExtProtSvc->FixFilePermissions(destfile);
       }
     }
 
     // Notify dialog that download is complete.
     // By waiting till this point, it ensures that the progress dialog doesn't indicate
     // success until we're really done.
     if(mWebProgressListener)
     {
@@ -2224,33 +2214,32 @@ nsresult nsExternalAppHandler::OpenWithA
 #if !defined(XP_MACOSX)
                            true);
 #else
                            false);
 #endif
 
     // make the tmp file readonly so users won't edit it and lose the changes
     // only if we're going to delete the file
-    if (deleteTempFileOnExit || gExtProtSvc->InPrivateBrowsing())
+    if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing())
       mFinalFileDestination->SetPermissions(0400);
 
     rv = mMimeInfo->LaunchWithFile(mFinalFileDestination);
     if (NS_FAILED(rv))
     {
       // Send error notification.
       nsAutoString path;
       mFinalFileDestination->GetPath(path);
       SendStatusChange(kLaunchError, rv, nsnull, path);
       Cancel(rv); // Cancel, and clean up temp file.
     }
     // Always schedule files to be deleted at the end of the private browsing
     // mode, regardless of the value of the pref.
-    else if (deleteTempFileOnExit || gExtProtSvc->InPrivateBrowsing()) {
-      NS_ASSERTION(gExtProtSvc, "Service gone away!?");
-      gExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination);
+    else if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) {
+      mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination);
     }
   }
 
   return rv;
 }
 
 // LaunchWithApplication should only be called by the helper app dialog which allows
 // the user to say launch with application or save to disk. It doesn't actually 
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -68,16 +68,17 @@
 #include "nsITimer.h"
 
 #include "nsIHandlerService.h"
 #include "nsCOMPtr.h"
 #include "nsIObserver.h"
 #include "nsCOMArray.h"
 #include "nsWeakReference.h"
 #include "nsIPrompt.h"
+#include "nsAutoPtr.h"
 
 class nsExternalAppHandler;
 class nsIMIMEInfo;
 class nsITransfer;
 class nsIDOMWindow;
 
 /**
  * The helper app service. Responsible for handling content that Mozilla
@@ -255,22 +256,24 @@ public:
   NS_DECL_NSITIMERCALLBACK
 
   /**
    * @param aMIMEInfo      MIMEInfo object, representing the type of the
    *                       content that should be handled
    * @param aFileExtension The extension we need to append to our temp file,
    *                       INCLUDING the ".". e.g. .mp3
    * @param aWindowContext Window context, as passed to DoContent
+   * @param mExtProtSvc    nsExternalHelperAppService on creation
    * @param aFileName      The filename to use
    * @param aReason        A constant from nsIHelperAppLauncherDialog indicating
    *                       why the request is handled by a helper app.
    */
   nsExternalAppHandler(nsIMIMEInfo * aMIMEInfo, const nsCSubstring& aFileExtension,
                        nsIInterfaceRequestor * aWindowContext,
+                       nsExternalHelperAppService * aExtProtSvc,
                        const nsAString& aFilename,
                        PRUint32 aReason, bool aForceSave);
 
   ~nsExternalAppHandler();
 
 protected:
   nsCOMPtr<nsIFile> mTempFile;
   nsCOMPtr<nsIURI> mSourceUrl;
@@ -442,13 +445,13 @@ protected:
   bool mKeepRequestAlive;
 
   /**
    * The request that's being loaded. Initialized in OnStartRequest.
    * Nulled out in OnStopRequest or once we know what we're doing
    * with the data, whichever happens later.
    */
   nsCOMPtr<nsIRequest> mRequest;
+
+  nsRefPtr<nsExternalHelperAppService> mExtProtSvc;
 };
 
-extern NS_HIDDEN_(nsExternalHelperAppService*) gExtProtSvc;
-
 #endif // nsExternalHelperAppService_h__
--- a/uriloader/exthandler/nsExternalProtocolHandler.cpp
+++ b/uriloader/exthandler/nsExternalProtocolHandler.cpp
@@ -371,18 +371,19 @@ nsExternalProtocolHandler::AllowPort(PRI
 // returns TRUE if the OS can handle this protocol scheme and false otherwise.
 bool nsExternalProtocolHandler::HaveExternalProtocolHandler(nsIURI * aURI)
 {
   bool haveHandler = false;
   if (aURI)
   {
     nsCAutoString scheme;
     aURI->GetScheme(scheme);
-    if (gExtProtSvc)
-      gExtProtSvc->ExternalProtocolHandlerExists(scheme.get(), &haveHandler);
+    nsCOMPtr<nsIExternalProtocolService> extProtSvc(do_GetService(NS_EXTERNALPROTOCOLSERVICE_CONTRACTID));
+    if (extProtSvc)
+      extProtSvc->ExternalProtocolHandlerExists(scheme.get(), &haveHandler);
   }
 
   return haveHandler;
 }
 
 NS_IMETHODIMP nsExternalProtocolHandler::GetProtocolFlags(PRUint32 *aUritype)
 {
     // Make it norelative since it is a simple uri
@@ -433,16 +434,17 @@ NS_IMETHODIMP nsExternalProtocolHandler:
   return NS_ERROR_UNKNOWN_PROTOCOL;
 }
 
 ///////////////////////////////////////////////////////////////////////
 // External protocol handler interface implementation
 //////////////////////////////////////////////////////////////////////
 NS_IMETHODIMP nsExternalProtocolHandler::ExternalAppExistsForScheme(const nsACString& aScheme, bool *_retval)
 {
-  if (gExtProtSvc)
-    return gExtProtSvc->ExternalProtocolHandlerExists(
+  nsCOMPtr<nsIExternalProtocolService> extProtSvc(do_GetService(NS_EXTERNALPROTOCOLSERVICE_CONTRACTID));
+  if (extProtSvc)
+    return extProtSvc->ExternalProtocolHandlerExists(
       PromiseFlatCString(aScheme).get(), _retval);
 
   // In case we don't have external protocol service.
   *_retval = false;
   return NS_OK;
 }
--- a/uriloader/exthandler/nsExternalProtocolHandler.h
+++ b/uriloader/exthandler/nsExternalProtocolHandler.h
@@ -59,13 +59,12 @@ public:
 
 	nsExternalProtocolHandler();
 	~nsExternalProtocolHandler();
 
 protected:
   // helper function
   bool HaveExternalProtocolHandler(nsIURI * aURI);
 	nsCString	m_schemeName;
-  nsCOMPtr<nsIExternalProtocolService> m_extProtService;
 };
 
 #endif // nsExternalProtocolHandler_h___