Bug 671185 - Incorrect return of NS_ERROR_* codes in functions returning PRBool, r=mak,ehsan,taras,biesi,pike,khuey,dholbert,josh,bjacob,bsmith
authorMichael Wu <mwu@mozilla.com>
Mon, 25 Jul 2011 21:57:58 -0700
changeset 73324 7a21ce9c4482b70b59b81f89641f3d586ca0e7a0
parent 73323 95eda84ea6babca4d7d43e65db1bc803252431d8
child 73325 982a5835fba1bddf72e64775a5601132bef77181
child 73330 820b809453428f161fce5cdb737a896e6a619df6
push id20846
push usermak77@bonardo.net
push dateTue, 26 Jul 2011 09:50:30 +0000
treeherdermozilla-central@982a5835fba1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, ehsan, taras, biesi, pike, khuey, dholbert, josh, bjacob, bsmith
bugs671185
milestone8.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 671185 - Incorrect return of NS_ERROR_* codes in functions returning PRBool, r=mak,ehsan,taras,biesi,pike,khuey,dholbert,josh,bjacob,bsmith
browser/components/migration/src/nsProfileMigrator.cpp
content/canvas/src/WebGLContextValidate.cpp
dom/plugins/base/nsPluginStreamListenerPeer.cpp
editor/libeditor/base/nsSelectionState.cpp
layout/forms/nsFileControlFrame.cpp
modules/libpr0n/src/VectorImage.cpp
netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
rdf/base/src/nsRDFContainerUtils.cpp
services/crypto/component/nsSyncJPAKE.cpp
toolkit/components/places/Helpers.h
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/telemetry/Telemetry.cpp
--- a/browser/components/migration/src/nsProfileMigrator.cpp
+++ b/browser/components/migration/src/nsProfileMigrator.cpp
@@ -277,21 +277,21 @@ nsProfileMigrator::GetDefaultBrowserMigr
 
 PRBool
 nsProfileMigrator::ImportRegistryProfiles(const nsACString& aAppName)
 {
   nsresult rv;
 
   nsCOMPtr<nsIToolkitProfileService> profileSvc
     (do_GetService(NS_PROFILESERVICE_CONTRACTID));
-  NS_ENSURE_TRUE(profileSvc, NS_ERROR_FAILURE);
+  NS_ENSURE_TRUE(profileSvc, PR_FALSE);
 
   nsCOMPtr<nsIProperties> dirService
     (do_GetService("@mozilla.org/file/directory_service;1"));
-  NS_ENSURE_TRUE(dirService, NS_ERROR_FAILURE);
+  NS_ENSURE_TRUE(dirService, PR_FALSE);
 
   nsCOMPtr<nsILocalFile> regFile;
 #ifdef XP_WIN
   rv = dirService->Get(NS_WIN_APPDATA_DIR, NS_GET_IID(nsILocalFile),
                        getter_AddRefs(regFile));
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
   regFile->AppendNative(aAppName);
   regFile->AppendNative(NS_LITERAL_CSTRING("registry.dat"));
--- a/content/canvas/src/WebGLContextValidate.cpp
+++ b/content/canvas/src/WebGLContextValidate.cpp
@@ -594,17 +594,17 @@ WebGLContext::InitAndValidateGL()
             // gl_PointCoord is always available in ES2 GLSL and in newer desktop GLSL versions, but apparently
             // not in OpenGL 2 and apparently not (due to a driver bug) on certain NVIDIA setups. See:
             //   http://www.opengl.org/discussion_boards/ubbthreads.php?ubb=showflat&Number=261472
             gl->fEnable(LOCAL_GL_POINT_SPRITE);
         }
     }
 
     // Check the shader validator pref
-    NS_ENSURE_TRUE(Preferences::GetRootBranch(), NS_ERROR_FAILURE);
+    NS_ENSURE_TRUE(Preferences::GetRootBranch(), PR_FALSE);
 
     mShaderValidation =
         Preferences::GetBool("webgl.shader_validator", mShaderValidation);
 
 #if defined(USE_ANGLE)
     // initialize shader translator
     if (mShaderValidation) {
         if (!ShInitialize()) {
--- a/dom/plugins/base/nsPluginStreamListenerPeer.cpp
+++ b/dom/plugins/base/nsPluginStreamListenerPeer.cpp
@@ -886,18 +886,17 @@ nsresult nsPluginStreamListenerPeer::Ser
   mPendingRequests = 0;
   
   return NS_OK;
 }
 
 PRBool
 nsPluginStreamListenerPeer::UseExistingPluginCacheFile(nsPluginStreamListenerPeer* psi)
 {
-  
-  NS_ENSURE_ARG_POINTER(psi);
+  NS_ENSURE_TRUE(psi, PR_FALSE);
   
   if (psi->mLength == mLength &&
       psi->mModified == mModified &&
       mStreamComplete &&
       mURLSpec.Equals(psi->mURLSpec))
   {
     return PR_TRUE;
   }
--- a/editor/libeditor/base/nsSelectionState.cpp
+++ b/editor/libeditor/base/nsSelectionState.cpp
@@ -143,17 +143,17 @@ nsSelectionState::IsCollapsed()
   PRBool bIsCollapsed = PR_FALSE;
   range->GetCollapsed(&bIsCollapsed);
   return bIsCollapsed;
 }
 
 PRBool
 nsSelectionState::IsEqual(nsSelectionState *aSelState)
 {
-  NS_ENSURE_TRUE(aSelState, NS_ERROR_NULL_POINTER);
+  NS_ENSURE_TRUE(aSelState, PR_FALSE);
   PRUint32 i, myCount = mArray.Length(), itsCount = aSelState->mArray.Length();
   if (myCount != itsCount) return PR_FALSE;
   if (myCount < 1) return PR_FALSE;
 
   for (i=0; i<myCount; i++)
   {
     nsCOMPtr<nsIDOMRange> myRange, itsRange;
     mArray[i].GetRange(address_of(myRange));
--- a/layout/forms/nsFileControlFrame.cpp
+++ b/layout/forms/nsFileControlFrame.cpp
@@ -184,45 +184,45 @@ PRBool CapturePickerAcceptCallback(const
   nsresult rv;
   PRBool captureEnabled;
   CaptureCallbackData* closure = (CaptureCallbackData*)aClosure;
 
   if (StringBeginsWith(aAccept,
                        NS_LITERAL_STRING("image/"))) {
     rv = closure->picker->ModeMayBeAvailable(nsICapturePicker::MODE_STILL,
                                              &captureEnabled);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, PR_TRUE);
     if (captureEnabled) {
       *closure->mode = nsICapturePicker::MODE_STILL;
       return PR_FALSE;
     }
   } else if (StringBeginsWith(aAccept,
                               NS_LITERAL_STRING("audio/"))) {
     rv = closure->picker->ModeMayBeAvailable(nsICapturePicker::MODE_AUDIO_CLIP,
                                              &captureEnabled);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, PR_TRUE);
     if (captureEnabled) {
       *closure->mode = nsICapturePicker::MODE_AUDIO_CLIP;
       return PR_FALSE;
     }
   } else if (StringBeginsWith(aAccept,
                               NS_LITERAL_STRING("video/"))) {
     rv = closure->picker->ModeMayBeAvailable(nsICapturePicker::MODE_VIDEO_CLIP,
                                              &captureEnabled);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, PR_TRUE);
     if (captureEnabled) {
       *closure->mode = nsICapturePicker::MODE_VIDEO_CLIP;
       return PR_FALSE;
     }
     rv = closure->picker->ModeMayBeAvailable(nsICapturePicker::MODE_VIDEO_NO_SOUND_CLIP,
                                              &captureEnabled);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_SUCCESS(rv, PR_TRUE);
     if (captureEnabled) {
       *closure->mode = nsICapturePicker::MODE_VIDEO_NO_SOUND_CLIP;
-      return PR_FALSE;;
+      return PR_FALSE;
     }
   }
   return PR_TRUE;
 }
 
 nsresult
 nsFileControlFrame::CreateAnonymousContent(nsTArray<ContentInfo>& aElements)
 {
--- a/modules/libpr0n/src/VectorImage.cpp
+++ b/modules/libpr0n/src/VectorImage.cpp
@@ -152,17 +152,17 @@ SVGDrawingCallback::operator()(gfxContex
                                const gfxMatrix& aTransform)
 {
   NS_ABORT_IF_FALSE(mSVGDocumentWrapper, "need an SVGDocumentWrapper");
 
   // Get (& sanity-check) the helper-doc's presShell
   nsCOMPtr<nsIPresShell> presShell;
   if (NS_FAILED(mSVGDocumentWrapper->GetPresShell(getter_AddRefs(presShell)))) {
     NS_WARNING("Unable to draw -- presShell lookup failed");
-    return NS_ERROR_FAILURE;
+    return PR_FALSE;
   }
   NS_ABORT_IF_FALSE(presShell, "GetPresShell succeeded but returned null");
 
   gfxContextAutoSaveRestore contextRestorer(aContext);
 
   // Clip to aFillRect so that we don't paint outside.
   aContext->NewPath();
   aContext->Rectangle(aFillRect);
--- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
+++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp
@@ -1113,17 +1113,18 @@ nsHttpChannelAuthProvider::ConfirmAuth(c
 {
     // skip prompting the user if
     //   1) we've already prompted the user
     //   2) we're not a toplevel channel
     //   3) the userpass length is less than the "phishy" threshold
 
     PRUint32 loadFlags;
     nsresult rv = mAuthChannel->GetLoadFlags(&loadFlags);
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv))
+        return PR_TRUE;
 
     if (mSuppressDefensiveAuth ||
         !(loadFlags & nsIChannel::LOAD_INITIAL_DOCUMENT_URI))
         return PR_TRUE;
 
     nsCAutoString userPass;
     rv = mURI->GetUserPass(userPass);
     if (NS_FAILED(rv) ||
@@ -1159,21 +1160,23 @@ nsHttpChannelAuthProvider::ConfirmAuth(c
 
     nsXPIDLString msg;
     bundle->FormatStringFromName(bundleKey.get(), strs, 2, getter_Copies(msg));
     if (!msg)
         return PR_TRUE;
 
     nsCOMPtr<nsIInterfaceRequestor> callbacks;
     rv = mAuthChannel->GetNotificationCallbacks(getter_AddRefs(callbacks));
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv))
+        return PR_TRUE;
 
     nsCOMPtr<nsILoadGroup> loadGroup;
     rv = mAuthChannel->GetLoadGroup(getter_AddRefs(loadGroup));
-    if (NS_FAILED(rv)) return rv;
+    if (NS_FAILED(rv))
+        return PR_TRUE;
 
     nsCOMPtr<nsIPrompt> prompt;
     NS_QueryNotificationCallbacks(callbacks, loadGroup, NS_GET_IID(nsIPrompt),
                                   getter_AddRefs(prompt));
     if (!prompt)
         return PR_TRUE;
 
     // do not prompt again
--- a/rdf/base/src/nsRDFContainerUtils.cpp
+++ b/rdf/base/src/nsRDFContainerUtils.cpp
@@ -457,24 +457,27 @@ RDFContainerUtilsImpl::MakeContainer(nsI
     }
 
     return NS_OK;
 }
 
 PRBool
 RDFContainerUtilsImpl::IsA(nsIRDFDataSource* aDataSource, nsIRDFResource* aResource, nsIRDFResource* aType)
 {
-    if (!aDataSource || !aResource || !aType)
-        return NS_ERROR_NULL_POINTER;
+    if (!aDataSource || !aResource || !aType) {
+        NS_WARNING("Unexpected null argument");
+        return PR_FALSE;
+    }
 
     nsresult rv;
 
     PRBool result;
     rv = aDataSource->HasAssertion(aResource, kRDF_instanceOf, aType, PR_TRUE, &result);
-    if (NS_FAILED(rv)) return PR_FALSE;
+    if (NS_FAILED(rv))
+      return PR_FALSE;
 
     return result;
 }
 
 NS_IMETHODIMP
 RDFContainerUtilsImpl::IndexOf(nsIRDFDataSource* aDataSource, nsIRDFResource* aContainer, nsIRDFNode* aElement, PRInt32* aIndex)
 {
     if (!aDataSource || !aContainer)
--- a/services/crypto/component/nsSyncJPAKE.cpp
+++ b/services/crypto/component/nsSyncJPAKE.cpp
@@ -101,17 +101,17 @@ fromHexString(const nsACString & str, un
   return NS_OK;
 }
 
 static PRBool
 toHexString(const unsigned char * str, unsigned len, nsACString & out)
 {
   static const char digits[] = "0123456789ABCDEF";
   if (!out.SetCapacity(2 * len))
-    return NS_ERROR_OUT_OF_MEMORY;
+    return PR_FALSE;
   out.SetLength(0);
   for (unsigned i = 0; i < len; ++i) {
     out.Append(digits[str[i] >> 4]);
     out.Append(digits[str[i] & 0x0f]);
   }
   return PR_TRUE;
 }
 
--- a/toolkit/components/places/Helpers.h
+++ b/toolkit/components/places/Helpers.h
@@ -105,23 +105,29 @@ protected:
     return _stmt.get();                                                        \
   }                                                                            \
   PR_END_MACRO
 #endif
 
 // Async statements don't need to be scoped, they are reset when done.
 // So use this version for statements used async, scoped version for statements
 // used sync.
-#define DECLARE_AND_ASSIGN_LAZY_STMT(_localStmt, _globalStmt)                  \
+#define DECLARE_AND_ASSIGN_LAZY_STMT_RET(_localStmt, _globalStmt, _ret)            \
   mozIStorageStatement* _localStmt = GetStatement(_globalStmt);                \
-  NS_ENSURE_STATE(_localStmt)
+  NS_ENSURE_TRUE(_localStmt, _ret)
+
+#define DECLARE_AND_ASSIGN_LAZY_STMT(_localStmt, _globalStmt)                  \
+  DECLARE_AND_ASSIGN_LAZY_STMT_RET(_localStmt, _globalStmt, NS_ERROR_UNEXPECTED)
+
+#define DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT_RET(_localStmt, _globalStmt, _ret)     \
+  DECLARE_AND_ASSIGN_LAZY_STMT_RET(_localStmt, _globalStmt, _ret);             \
+  mozStorageStatementScoper scoper(_localStmt)
 
 #define DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(_localStmt, _globalStmt)           \
-  DECLARE_AND_ASSIGN_LAZY_STMT(_localStmt, _globalStmt);                       \
-  mozStorageStatementScoper scoper(_localStmt)
+  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT_RET(_localStmt, _globalStmt, NS_ERROR_UNEXPECTED)
 
 
 /**
  * Utils to bind a specified URI (or URL) to a statement or binding params, at
  * the specified index or name.
  * @note URIs are always bound as UTF8.
  */
 class URIBinder // static
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -745,17 +745,17 @@ nsNavBookmarks::CreateRoot(const nsCStri
 
   return NS_OK;
 }
 
 
 PRBool
 nsNavBookmarks::IsRealBookmark(PRInt64 aPlaceId)
 {
-  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(stmt, mDBIsRealBookmark);
+  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT_RET(stmt, mDBIsRealBookmark, PR_FALSE);
   nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), aPlaceId);
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
   rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_type"), TYPE_BOOKMARK);
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
   rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("anno_name"),
                                   NS_LITERAL_CSTRING(LMANNO_FEEDURI));
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Binding failed");
 
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -1888,17 +1888,17 @@ nsNavHistory::InternalAddVisit(PRInt64 a
 //    This is used to compute the referring visit.
 
 PRBool
 nsNavHistory::FindLastVisit(nsIURI* aURI,
                             PRInt64* aVisitID,
                             PRTime* aTime,
                             PRInt64* aSessionID)
 {
-  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(stmt, mDBRecentVisitOfURL);
+  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT_RET(stmt, mDBRecentVisitOfURL, PR_FALSE);
   nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), aURI);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
 
   PRBool hasMore;
   rv = stmt->ExecuteStep(&hasMore);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
   if (hasMore) {
     rv = stmt->GetInt64(0, aVisitID);
@@ -1916,17 +1916,17 @@ nsNavHistory::FindLastVisit(nsIURI* aURI
 // nsNavHistory::IsURIStringVisited
 //
 //    Takes a URL as a string and returns true if we've visited it.
 //
 //    Be careful to always reset the statement since it will be reused.
 
 PRBool nsNavHistory::IsURIStringVisited(const nsACString& aURIString)
 {
-  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT(stmt, mDBIsPageVisited);
+  DECLARE_AND_ASSIGN_SCOPED_LAZY_STMT_RET(stmt, mDBIsPageVisited, PR_FALSE);
   nsresult rv = URIBinder::Bind(stmt, 0, aURIString);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
 
   PRBool hasMore = PR_FALSE;
   rv = stmt->ExecuteStep(&hasMore);
   NS_ENSURE_SUCCESS(rv, PR_FALSE);
   return hasMore;
 }
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -63,25 +63,28 @@
 #include "nsIXPCScriptable.h"
 
 #define TO_ICONTAINER(_node)                                                  \
     static_cast<nsINavHistoryContainerResultNode*>(_node)                      
 
 #define TO_CONTAINER(_node)                                                   \
     static_cast<nsNavHistoryContainerResultNode*>(_node)
 
-#define NOTIFY_RESULT_OBSERVERS(_result, _method)                             \
+#define NOTIFY_RESULT_OBSERVERS_RET(_result, _method, _ret)                   \
   PR_BEGIN_MACRO                                                              \
-  NS_ENSURE_STATE(_result);                                                   \
+  NS_ENSURE_TRUE(_result, _ret);                                              \
   if (!_result->mSuppressNotifications) {                                     \
     ENUMERATE_WEAKARRAY(_result->mObservers, nsINavHistoryResultObserver,     \
                         _method)                                              \
   }                                                                           \
   PR_END_MACRO
 
+#define NOTIFY_RESULT_OBSERVERS(_result, _method)                             \
+  NOTIFY_RESULT_OBSERVERS_RET(_result, _method, NS_ERROR_UNEXPECTED)
+
 // What we want is: NS_INTERFACE_MAP_ENTRY(self) for static IID accessors,
 // but some of our classes (like nsNavHistoryResult) have an ambiguous base
 // class of nsISupports which prevents this from working (the default macro
 // converts it to nsISupports, then addrefs it, then returns it). Therefore, we
 // expand the macro here and change it so that it works. Yuck.
 #define NS_INTERFACE_MAP_STATIC_AMBIGUOUS(_class) \
   if (aIID.Equals(NS_GET_IID(_class))) { \
     NS_ADDREF(this); \
@@ -1638,18 +1641,19 @@ nsNavHistoryContainerResultNode::EnsureI
   mChildren.RemoveObjectAt(aIndex);
 
   PRUint32 newIndex = FindInsertionPoint(
                           node, comparator,sortAnno.get(), nsnull);
   mChildren.InsertObjectAt(node.get(), newIndex);
 
   if (AreChildrenVisible()) {
     nsNavHistoryResult* result = GetResult();
-    NOTIFY_RESULT_OBSERVERS(result,
-                            NodeMoved(node, this, aIndex, this, newIndex));
+    NOTIFY_RESULT_OBSERVERS_RET(result,
+                                NodeMoved(node, this, aIndex, this, newIndex),
+                                PR_FALSE);
   }
 
   return PR_TRUE;
 }
 
 
 /**
  * This takes a list of nodes and merges them into the current result set.
@@ -3920,17 +3924,17 @@ nsNavHistoryFolderResultNode::StartIncre
       !mOptions->ExcludeReadOnlyFolders() && 
       parentAnnotationToExclude.IsEmpty()) {
 
     // easy case: we are visible, always do incremental update
     if (mExpanded || AreChildrenVisible())
       return PR_TRUE;
 
     nsNavHistoryResult* result = GetResult();
-    NS_ENSURE_STATE(result);
+    NS_ENSURE_TRUE(result, PR_FALSE);
 
     // When any observers are attached also do incremental updates if our
     // parent is visible, so that twisties are drawn correctly.
     if (mParent)
       return result->mObservers.Length() > 0;
   }
 
   // otherwise, we don't do incremental updates, invalidate and unregister
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -228,17 +228,17 @@ JSHistogram_Add(JSContext *cx, uintN arg
 
 JSBool
 JSHistogram_Snapshot(JSContext *cx, uintN argc, jsval *vp)
 {
   JSObject *obj = JS_THIS_OBJECT(cx, vp);
   Histogram *h = static_cast<Histogram*>(JS_GetPrivate(cx, obj));
   JSObject *snapshot = JS_NewObject(cx, NULL, NULL, NULL);
   if (!snapshot)
-    return NS_ERROR_FAILURE;
+    return JS_FALSE;
   JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(snapshot));
   return ReflectHistogramSnapshot(cx, snapshot, h);
 }
 
 nsresult 
 WrapAndReturnHistogram(Histogram *h, JSContext *cx, jsval *ret)
 {
   static JSClass JSHistogram_class = {