Bug 835867: Use 'nsRefPtr' and 'forget()' in nsNavHistory.cpp instead of manual NS_ADDREF calls. r=mak

toolkit/components/places/nsNavHistory.cpp

@@ -1467,10 +1467,8 @@ nsNavHistory::GetNewQuery(nsINavHistoryQuery **_retval)
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG_POINTER(_retval);
 
-  *_retval = new nsNavHistoryQuery();
+  nsRefPtr<nsNavHistoryQuery> query = new nsNavHistoryQuery();
-  if (! *_retval)
+  query.forget(_retval);
-    return NS_ERROR_OUT_OF_MEMORY;
-  NS_ADDREF(*_retval);
   return NS_OK;
 }
 
@@ -1482,9 +1480,8 @@ nsNavHistory::GetNewQueryOptions(nsINavHistoryQueryOptions **_retval)
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG_POINTER(_retval);
 
-  *_retval = new nsNavHistoryQueryOptions();
+  nsRefPtr<nsNavHistoryQueryOptions> queryOptions = new nsNavHistoryQueryOptions();
-  NS_ENSURE_TRUE(*_retval, NS_ERROR_OUT_OF_MEMORY);
+  queryOptions.forget(_retval);
-  NS_ADDREF(*_retval);
   return NS_OK;
 }
 
@@ -1575,7 +1572,7 @@ nsNavHistory::ExecuteQueries(nsINavHistoryQuery** aQueries, uint32_t aQueryCount
                                             getter_AddRefs(result));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  NS_ADDREF(*_retval = result);
+  result.forget(_retval);
   return NS_OK;
 }
 
@@ -3544,7 +3541,9 @@ NS_IMETHODIMP
 nsNavHistory::GetDBConnection(mozIStorageConnection **_DBConnection)
 {
   NS_ENSURE_ARG_POINTER(_DBConnection);
-  NS_IF_ADDREF(*_DBConnection = mDB->MainConn());
+  nsRefPtr<mozIStorageConnection> connection = mDB->MainConn();
+  connection.forget(_DBConnection);
+
   return NS_OK;
 }
 
@@ -4172,7 +4171,7 @@ const int64_t UNDEFINED_URN_VALUE = -1;
 // to be used to persist the open state of this container in localstore.rdf
 nsresult
 CreatePlacesPersistURN(nsNavHistoryQueryResultNode *aResultNode, 
-                      int64_t aValue, const nsCString& aTitle, nsCString& aURN)
+                       int64_t aValue, const nsCString& aTitle, nsCString& aURN)
 {
   nsAutoCString uri;
   nsresult rv = aResultNode->GetUri(uri);
@@ -4393,7 +4392,6 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
                           nsNavHistoryResultNode** aResult)
 {
   NS_ASSERTION(aRow && aOptions && aResult, "Null pointer in RowToResult");
-  *aResult = nullptr;
 
   // URL
   nsAutoCString url;
@@ -4447,45 +4445,48 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
-    rv = QueryRowToResult(itemId, url, title, accessCount, time, favicon, aResult);
+    nsRefPtr<nsNavHistoryResultNode> resultNode;
-    NS_ENSURE_STATE(*aResult);
+    rv = QueryRowToResult(itemId, url, title, accessCount, time, favicon,
+                          getter_AddRefs(resultNode));
+    NS_ENSURE_SUCCESS(rv,rv);
+
     if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) {
       // RESULTS_AS_TAG_QUERY has date columns
-      (*aResult)->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
+      resultNode->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
-      (*aResult)->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
+      resultNode->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
     }
-    else if ((*aResult)->IsFolder()) {
+    else if (resultNode->IsFolder()) {
       // If it's a simple folder node (i.e. a shortcut to another folder), apply
       // our options for it. However, if the parent type was tag query, we do not
       // apply them, because it would not yield any results.
-      (*aResult)->GetAsContainer()->mOptions = aOptions;
+      resultNode->GetAsContainer()->mOptions = aOptions;
     }
 
+    resultNode.forget(aResult);
     return rv;
   } else if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_URI ||
              aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_CONTENTS) {
-    *aResult = new nsNavHistoryResultNode(url, title, accessCount, time,
+    nsRefPtr<nsNavHistoryResultNode> resultNode =
-                                          favicon);
+      new nsNavHistoryResultNode(url, title, accessCount, time, favicon);
-    if (!*aResult)
-      return NS_ERROR_OUT_OF_MEMORY;
 
     if (itemId != -1) {
-      (*aResult)->mItemId = itemId;
+      resultNode->mItemId = itemId;
-      (*aResult)->mFolderId = parentId;
+      resultNode->mFolderId = parentId;
-      (*aResult)->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
+      resultNode->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
-      (*aResult)->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
+      resultNode->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
     }
 
-    (*aResult)->mFrecency = aRow->AsInt32(kGetInfoIndex_Frecency);
+    resultNode->mFrecency = aRow->AsInt32(kGetInfoIndex_Frecency);
-    (*aResult)->mHidden = !!aRow->AsInt32(kGetInfoIndex_Hidden);
+    resultNode->mHidden = !!aRow->AsInt32(kGetInfoIndex_Hidden);
 
     nsAutoString tags;
     rv = aRow->GetString(kGetInfoIndex_ItemTags, tags);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (!tags.IsVoid())
+    if (!tags.IsVoid()) {
-      (*aResult)->mTags.Assign(tags);
+      resultNode->mTags.Assign(tags);
+    }
 
-    NS_ADDREF(*aResult);
+    resultNode.forget(aResult);
     return NS_OK;
   }
   // now we know the result type is some kind of visit (regular or full)
@@ -4494,17 +4495,16 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
   int64_t session = aRow->AsInt64(kGetInfoIndex_SessionId);
 
   if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_VISIT) {
-    *aResult = new nsNavHistoryVisitResultNode(url, title, accessCount, time,
+    nsRefPtr<nsNavHistoryResultNode> resultNode =
-                                               favicon, session);
+      new nsNavHistoryVisitResultNode(url, title, accessCount, time,
-    if (! *aResult)
+                                      favicon, session);
-      return NS_ERROR_OUT_OF_MEMORY;
 
     nsAutoString tags;
     rv = aRow->GetString(kGetInfoIndex_ItemTags, tags);
     if (!tags.IsVoid())
-      (*aResult)->mTags.Assign(tags);
+      resultNode->mTags.Assign(tags);
 
-    NS_ADDREF(*aResult);
+    resultNode.forget(aResult);
     return NS_OK;
   }
 
@@ -4528,6 +4528,8 @@ nsNavHistory::QueryRowToResult(int64_t itemId, const nsACString& aURI,
   nsCOMPtr<nsNavHistoryQueryOptions> options;
   nsresult rv = QueryStringToQueryArray(aURI, &queries,
                                         getter_AddRefs(options));
+
+  nsRefPtr<nsNavHistoryResultNode> resultNode;
   // If this failed the query does not parse correctly, let the error pass and
   // handle it later.
   if (NS_SUCCEEDED(rv)) {
@@ -4537,27 +4539,26 @@ nsNavHistory::QueryRowToResult(int64_t itemId, const nsACString& aURI,
       nsNavBookmarks *bookmarks = nsNavBookmarks::GetBookmarksService();
       NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
 
-      // This AddRefs for us.
+      rv = bookmarks->ResultNodeForContainer(folderId, options,
-      rv = bookmarks->ResultNodeForContainer(folderId, options, aNode);
+                                             getter_AddRefs(resultNode));
       // If this failed the shortcut is pointing to nowhere, let the error pass
       // and handle it later.
       if (NS_SUCCEEDED(rv)) {
         // This is the query itemId, and is what is exposed by node.itemId.
-        (*aNode)->GetAsFolder()->mQueryItemId = itemId;
+        resultNode->GetAsFolder()->mQueryItemId = itemId;
 
         // Use the query item title, unless it's void (in that case use the 
         // concrete folder title).
         if (!aTitle.IsVoid()) {
-          (*aNode)->mTitle = aTitle;
+          resultNode->mTitle = aTitle;
         }
       }
     }
     else {
       // This is a regular query.
-      *aNode = new nsNavHistoryQueryResultNode(aTitle, EmptyCString(), aTime,
+      resultNode = new nsNavHistoryQueryResultNode(aTitle, EmptyCString(),
-                                               queries, options);
+                                                   aTime, queries, options);
-      (*aNode)->mItemId = itemId;
+      resultNode->mItemId = itemId;
-      NS_ADDREF(*aNode);
     }
   }
 
@@ -4566,13 +4567,13 @@ nsNavHistory::QueryRowToResult(int64_t itemId, const nsACString& aURI,
     // This is a broken query, that either did not parse or points to not
     // existing data.  We don't want to return failure since that will kill the
     // whole result.  Instead make a generic empty query node.
-    *aNode = new nsNavHistoryQueryResultNode(aTitle, aFavicon, aURI);
+    resultNode = new nsNavHistoryQueryResultNode(aTitle, aFavicon, aURI);
-    (*aNode)->mItemId = itemId;
+    resultNode->mItemId = itemId;
     // This is a perf hack to generate an empty query that skips filtering.
-    (*aNode)->GetAsQuery()->Options()->SetExcludeItems(true);
+    resultNode->GetAsQuery()->Options()->SetExcludeItems(true);
-    NS_ADDREF(*aNode);
   }
 
+  resultNode.forget(aNode);
   return NS_OK;
 }