Bug 984466 - change CallQueryInterface to assert in cases of trivial conversions; r=ehsan
authorNathan Froyd <froydnj@mozilla.com>
Mon, 17 Mar 2014 13:00:17 -0400
changeset 174309 e17405b3cc010d2ec5d315263a77fdff44fb03a2
parent 174308 e79d60318d620d505a226f084dea578ed46ac000
child 174310 bff60e1caa23443fa4be9e9531f5e776d671a26e
push id26451
push usercbook@mozilla.com
push dateThu, 20 Mar 2014 12:56:37 +0000
treeherdermozilla-central@358d369c8388 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs984466
milestone31.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 984466 - change CallQueryInterface to assert in cases of trivial conversions; r=ehsan
dom/plugins/base/nsPluginDirServiceProvider.cpp
embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
modules/libpref/src/Preferences.cpp
profile/dirserviceprovider/src/nsProfileDirServiceProvider.cpp
toolkit/components/downloads/nsDownloadManager.cpp
toolkit/xre/nsAppRunner.cpp
uriloader/exthandler/nsMIMEInfoImpl.cpp
widget/gtk/nsFilePicker.cpp
widget/xpwidgets/nsBaseFilePicker.cpp
xpcom/glue/nsISupportsUtils.h
xpcom/io/nsDirectoryService.cpp
xpcom/io/nsLocalFileUnix.cpp
xpcom/io/nsLocalFileWin.cpp
--- a/dom/plugins/base/nsPluginDirServiceProvider.cpp
+++ b/dom/plugins/base/nsPluginDirServiceProvider.cpp
@@ -340,20 +340,22 @@ nsPluginDirServiceProvider::GetFile(cons
 
     if (!newestPath.IsEmpty()) {
       newestPath += NS_LITERAL_STRING("\\browser");
       rv = NS_NewLocalFile(newestPath, true,
                            getter_AddRefs(localFile));
     }
   }
 
-  if (localFile && NS_SUCCEEDED(rv))
-    return CallQueryInterface(localFile, _retval);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
-  return rv;
+  localFile.forget(_retval);
+  return NS_OK;
 }
 
 nsresult
 nsPluginDirServiceProvider::GetPLIDDirectories(nsISimpleEnumerator **aEnumerator)
 {
   NS_ENSURE_ARG_POINTER(aEnumerator);
   *aEnumerator = nullptr;
 
--- a/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
+++ b/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
@@ -1111,20 +1111,22 @@ nsresult nsWebBrowserPersist::GetLocalFi
     nsresult rv;
 
     nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(aURI, &rv);
     if (NS_FAILED(rv))
         return rv;
 
     nsCOMPtr<nsIFile> file;
     rv = fileURL->GetFile(getter_AddRefs(file));
-    if (NS_SUCCEEDED(rv))
-        rv = CallQueryInterface(file, aLocalFile);
-
-    return rv;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+
+    file.forget(aLocalFile);
+    return NS_OK;
 }
 
 nsresult nsWebBrowserPersist::AppendPathToURI(nsIURI *aURI, const nsAString & aPath) const
 {
     NS_ENSURE_ARG_POINTER(aURI);
 
     nsAutoCString newPath;
     nsresult rv = aURI->GetPath(newPath);
--- a/modules/libpref/src/Preferences.cpp
+++ b/modules/libpref/src/Preferences.cpp
@@ -715,34 +715,39 @@ Preferences::GetBranch(const char *aPref
     // TODO: - cache this stuff and allow consumers to share branches (hold weak references I think)
     nsPrefBranch* prefBranch = new nsPrefBranch(aPrefRoot, false);
     if (!prefBranch)
       return NS_ERROR_OUT_OF_MEMORY;
 
     rv = CallQueryInterface(prefBranch, _retval);
   } else {
     // special case caching the default root
-    rv = CallQueryInterface(sRootBranch, _retval);
+    nsCOMPtr<nsIPrefBranch> root(sRootBranch);
+    root.forget(_retval);
+    rv = NS_OK;
   }
   return rv;
 }
 
 NS_IMETHODIMP
 Preferences::GetDefaultBranch(const char *aPrefRoot, nsIPrefBranch **_retval)
 {
   if (!aPrefRoot || !aPrefRoot[0]) {
-    return CallQueryInterface(sDefaultRootBranch, _retval);
+    nsCOMPtr<nsIPrefBranch> root(sDefaultRootBranch);
+    root.forget(_retval);
+    return NS_OK;
   }
 
   // TODO: - cache this stuff and allow consumers to share branches (hold weak references I think)
-  nsPrefBranch* prefBranch = new nsPrefBranch(aPrefRoot, true);
+  nsRefPtr<nsPrefBranch> prefBranch = new nsPrefBranch(aPrefRoot, true);
   if (!prefBranch)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  return CallQueryInterface(prefBranch, _retval);
+  prefBranch.forget(_retval);
+  return NS_OK;
 }
 
 
 nsresult
 Preferences::NotifyServiceObservers(const char *aTopic)
 {
   nsCOMPtr<nsIObserverService> observerService = 
     mozilla::services::GetObserverService();  
--- a/profile/dirserviceprovider/src/nsProfileDirServiceProvider.cpp
+++ b/profile/dirserviceprovider/src/nsProfileDirServiceProvider.cpp
@@ -227,21 +227,22 @@ nsProfileDirServiceProvider::GetFile(con
     rv = domainDir->Clone(getter_AddRefs(localFile));
     if (NS_SUCCEEDED(rv)) {
       rv = localFile->AppendNative(SEARCH_FILE_50_NAME);
       if (NS_SUCCEEDED(rv))
         rv = EnsureProfileFileExists(localFile, domainDir);
     }
   }
 
-  
-  if (localFile && NS_SUCCEEDED(rv))
-    return CallQueryInterface(localFile, _retval);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
-  return rv;
+  localFile.forget(_retval);
+  return NS_OK;
 }
 
 //*****************************************************************************
 // Protected methods
 //*****************************************************************************
 
 nsresult
 nsProfileDirServiceProvider::Initialize()
--- a/toolkit/components/downloads/nsDownloadManager.cpp
+++ b/toolkit/components/downloads/nsDownloadManager.cpp
@@ -3197,22 +3197,27 @@ nsDownload::GetMIMEInfo(nsIMIMEInfo **aM
 }
 
 NS_IMETHODIMP
 nsDownload::GetTargetFile(nsIFile **aTargetFile)
 {
   nsresult rv;
 
   nsCOMPtr<nsIFileURL> fileURL = do_QueryInterface(mTarget, &rv);
-  if (NS_FAILED(rv)) return rv;
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   nsCOMPtr<nsIFile> file;
   rv = fileURL->GetFile(getter_AddRefs(file));
-  if (NS_SUCCEEDED(rv))
-    rv = CallQueryInterface(file, aTargetFile);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  file.forget(aTargetFile);
   return rv;
 }
 
 NS_IMETHODIMP
 nsDownload::GetSpeed(double *aSpeed)
 {
   *aSpeed = mSpeed;
   return NS_OK;
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -2963,20 +2963,18 @@ XREMain::XRE_mainInit(bool* aExitFlag)
     rv = XRE_GetBinaryPath(gArgv[0], getter_AddRefs(lf));
     if (NS_FAILED(rv))
       return 2;
 
     nsCOMPtr<nsIFile> greDir;
     rv = lf->GetParent(getter_AddRefs(greDir));
     if (NS_FAILED(rv))
       return 2;
-    
-    rv = CallQueryInterface(greDir, &mAppData->xreDirectory);
-    if (NS_FAILED(rv))
-      return 2;
+
+    greDir.forget(&mAppData->xreDirectory);
   }
 
   if (!mAppData->directory) {
     NS_IF_ADDREF(mAppData->directory = mAppData->xreDirectory);
   }
 
   if (mAppData->size > offsetof(nsXREAppData, minVersion)) {
     if (!mAppData->minVersion) {
--- a/uriloader/exthandler/nsMIMEInfoImpl.cpp
+++ b/uriloader/exthandler/nsMIMEInfoImpl.cpp
@@ -260,23 +260,28 @@ nsMIMEInfoBase::SetAlwaysAskBeforeHandli
 
 /* static */
 nsresult 
 nsMIMEInfoBase::GetLocalFileFromURI(nsIURI *aURI, nsIFile **aFile)
 {
   nsresult rv;
 
   nsCOMPtr<nsIFileURL> fileUrl = do_QueryInterface(aURI, &rv);
-  if (NS_FAILED(rv)) return rv;
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   nsCOMPtr<nsIFile> file;
   rv = fileUrl->GetFile(getter_AddRefs(file));
-  if (NS_FAILED(rv)) return rv;    
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
-  return CallQueryInterface(file, aFile);
+  file.forget(aFile);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsMIMEInfoBase::LaunchWithFile(nsIFile* aFile)
 {
   nsresult rv;
 
   // it doesn't make any sense to call this on protocol handlers
--- a/widget/gtk/nsFilePicker.cpp
+++ b/widget/gtk/nsFilePicker.cpp
@@ -304,17 +304,18 @@ nsFilePicker::GetFile(nsIFile **aFile)
 
   nsCOMPtr<nsIFileURL> fileURL(do_QueryInterface(uri, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIFile> file;
   rv = fileURL->GetFile(getter_AddRefs(file));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return CallQueryInterface(file, aFile);
+  file.forget(aFile);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFilePicker::GetFileURL(nsIURI **aFileURL)
 {
   *aFileURL = nullptr;
   return NS_NewURI(aFileURL, mFileURL);
 }
--- a/widget/xpwidgets/nsBaseFilePicker.cpp
+++ b/widget/xpwidgets/nsBaseFilePicker.cpp
@@ -267,19 +267,21 @@ NS_IMETHODIMP nsBaseFilePicker::SetDispl
 // Get the display directory
 NS_IMETHODIMP nsBaseFilePicker::GetDisplayDirectory(nsIFile **aDirectory)
 {
   *aDirectory = nullptr;
   if (!mDisplayDirectory)
     return NS_OK;
   nsCOMPtr<nsIFile> directory;
   nsresult rv = mDisplayDirectory->Clone(getter_AddRefs(directory));
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return rv;
-  return CallQueryInterface(directory, aDirectory);
+  }
+  directory.forget(aDirectory);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsBaseFilePicker::GetAddToRecentDocs(bool *aFlag)
 {
   *aFlag = mAddToRecentDocs;
   return NS_OK;
 }
--- a/xpcom/glue/nsISupportsUtils.h
+++ b/xpcom/glue/nsISupportsUtils.h
@@ -6,16 +6,17 @@
 #ifndef nsISupportsUtils_h__
 #define nsISupportsUtils_h__
 
 #include "nscore.h"
 #include "nsISupportsBase.h"
 #include "nsError.h"
 #include "nsDebug.h"
 #include "nsISupportsImpl.h"
+#include "mozilla/TypeTraits.h"
 
 /**
  * Macro for adding a reference to an interface.
  * @param _ptr The interface pointer.
  */
 #define NS_ADDREF(_ptr) \
   (_ptr)->AddRef()
 
@@ -123,16 +124,22 @@ ns_if_addref( T expr )
   static_cast<nsISupports*>(static_cast<__unambiguousBase>(__expr))
 
 // a type-safe shortcut for calling the |QueryInterface()| member function
 template <class T, class DestinationType>
 inline
 nsresult
 CallQueryInterface( T* aSource, DestinationType** aDestination )
 {
+    // We permit nsISupports-to-nsISupports here so that one can still obtain
+    // the canonical nsISupports pointer with CallQueryInterface.
+    static_assert(!mozilla::IsSame<T, DestinationType>::value ||
+                  mozilla::IsSame<DestinationType, nsISupports>::value,
+                  "don't use CallQueryInterface for compile-time-determinable casts");
+
     NS_PRECONDITION(aSource, "null parameter");
     NS_PRECONDITION(aDestination, "null parameter");
-    
+
     return aSource->QueryInterface(NS_GET_TEMPLATE_IID(DestinationType),
                                    reinterpret_cast<void**>(aDestination));
 }
 
 #endif /* __nsISupportsUtils_h */
--- a/xpcom/io/nsDirectoryService.cpp
+++ b/xpcom/io/nsDirectoryService.cpp
@@ -874,17 +874,18 @@ nsDirectoryService::GetFile(const char *
 #endif
 
     if (NS_FAILED(rv))
         return rv;
 
     if (!localFile)
         return NS_ERROR_FAILURE;
 
-    return CallQueryInterface(localFile, _retval);
+    localFile.forget(_retval);
+    return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDirectoryService::GetFiles(const char *prop, nsISimpleEnumerator **_retval)
 {
     if (NS_WARN_IF(!_retval))
         return NS_ERROR_INVALID_ARG;
     *_retval = nullptr;
--- a/xpcom/io/nsLocalFileUnix.cpp
+++ b/xpcom/io/nsLocalFileUnix.cpp
@@ -1365,19 +1365,22 @@ nsLocalFile::GetParent(nsIFile **aParent
 
     nsCOMPtr<nsIFile> localFile;
     nsresult rv = NS_NewNativeLocalFile(nsDependentCString(buffer), true,
                                         getter_AddRefs(localFile));
 
     // make buffer whole again
     *slashp = c;
 
-    if (NS_SUCCEEDED(rv) && localFile)
-        rv = CallQueryInterface(localFile, aParent);
-    return rv;
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+
+    localFile.forget(aParent);
+    return NS_OK;
 }
 
 /*
  * The results of Exists, isWritable and isReadable are not cached.
  */
 
 
 NS_IMETHODIMP
--- a/xpcom/io/nsLocalFileWin.cpp
+++ b/xpcom/io/nsLocalFileWin.cpp
@@ -2679,20 +2679,22 @@ nsLocalFile::GetParent(nsIFile * *aParen
     if (offset > 0)
         parentPath.Truncate(offset);
     else
         parentPath.AssignLiteral("\\\\.");
 
     nsCOMPtr<nsIFile> localFile;
     nsresult rv = NS_NewLocalFile(parentPath, mFollowSymlinks, getter_AddRefs(localFile));
 
-    if (NS_SUCCEEDED(rv) && localFile) {
-        return CallQueryInterface(localFile, aParent);
+    if (NS_FAILED(rv)) {
+        return rv;
     }
-    return rv;
+
+    localFile.forget(aParent);
+    return NS_OK;
 }
 
 NS_IMETHODIMP
 nsLocalFile::Exists(bool *_retval)
 {
     // Check we are correctly initialized.
     CHECK_mWorkingPath();