fix for bug #387203: getFolderContents() can crash [@nsNavHistoryContainerResultNode::CloseContainer] when JS GC happens nsNavHistoryContainerResultNodes will now keep a strong reference back to the nsNavHistoryResult. This introduces a cycle, so use the cycle collector. r=sicking, a=blocking-firefox3+

This commit is contained in:
sspitzer@mozilla.org 2007-10-18 23:26:34 -07:00
parent 72a8c276fd
commit c597b3a9a2
4 changed files with 49 additions and 15 deletions

View File

@ -1968,7 +1968,7 @@ nsNavBookmarks::QueryFolderChildren(PRInt64 aFolderId,
PRInt32 itemType = mDBGetChildren->AsInt32(kGetChildrenIndex_Type);
PRInt64 id = mDBGetChildren->AsInt64(nsNavHistory::kGetInfoIndex_ItemId);
nsCOMPtr<nsNavHistoryResultNode> node;
nsRefPtr<nsNavHistoryResultNode> node;
if (itemType == TYPE_FOLDER || itemType == TYPE_DYNAMIC_CONTAINER) {
if (itemType == TYPE_DYNAMIC_CONTAINER ||
(itemType == TYPE_FOLDER && options->ExcludeReadOnlyFolders())) {

View File

@ -2059,7 +2059,7 @@ nsNavHistory::ExecuteQueries(nsINavHistoryQuery** aQueries, PRUint32 aQueryCount
// folder, we can more efficiently generate results.
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
nsCOMPtr<nsNavHistoryResultNode> tempRootNode;
nsRefPtr<nsNavHistoryResultNode> tempRootNode;
rv = bookmarks->ResultNodeForContainer(folderId, options,
getter_AddRefs(tempRootNode));
NS_ENSURE_SUCCESS(rv, rv);
@ -3874,7 +3874,7 @@ nsNavHistory::ResultsAsList(mozIStorageStatement* statement,
PRBool hasMore = PR_FALSE;
while (NS_SUCCEEDED(statement->ExecuteStep(&hasMore)) && hasMore) {
nsCOMPtr<nsNavHistoryResultNode> result;
nsRefPtr<nsNavHistoryResultNode> result;
rv = RowToResult(row, aOptions, getter_AddRefs(result));
NS_ENSURE_SUCCESS(rv, rv);
aResults->AppendObject(result);

View File

@ -70,6 +70,7 @@
#include "prprf.h"
#include "mozStorageHelper.h"
#include "nsAnnotationService.h"
#include "nsCycleCollectionParticipant.h"
#define ICONURI_QUERY "chrome://browser/skin/places/query.png"
@ -102,8 +103,15 @@ inline PRInt32 CompareIntegers(PRUint32 a, PRUint32 b)
// nsNavHistoryResultNode ******************************************************
NS_IMPL_ISUPPORTS2(nsNavHistoryResultNode,
nsNavHistoryResultNode, nsINavHistoryResultNode)
NS_IMPL_CYCLE_COLLECTION_0(nsNavHistoryResultNode)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsNavHistoryResultNode)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsINavHistoryResultNode)
NS_INTERFACE_MAP_ENTRY(nsINavHistoryResultNode)
NS_INTERFACE_MAP_END
NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS(nsNavHistoryResultNode, nsINavHistoryResultNode)
NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS(nsNavHistoryResultNode, nsINavHistoryResultNode)
nsNavHistoryResultNode::nsNavHistoryResultNode(
const nsACString& aURI, const nsACString& aTitle, PRUint32 aAccessCount,
@ -259,10 +267,22 @@ nsNavHistoryFullVisitResultNode::nsNavHistoryFullVisitResultNode(
// nsNavHistoryContainerResultNode *********************************************
NS_IMPL_CYCLE_COLLECTION_CLASS(nsNavHistoryContainerResultNode)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsNavHistoryContainerResultNode, nsNavHistoryResultNode)
NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mResult)
NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMARRAY(mChildren)
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsNavHistoryContainerResultNode, nsNavHistoryResultNode)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR_AMBIGUOUS(mResult, nsINavHistoryResult)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMARRAY(mChildren)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_ADDREF_INHERITED(nsNavHistoryContainerResultNode, nsNavHistoryResultNode)
NS_IMPL_RELEASE_INHERITED(nsNavHistoryContainerResultNode, nsNavHistoryResultNode)
NS_INTERFACE_MAP_BEGIN(nsNavHistoryContainerResultNode)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(nsNavHistoryContainerResultNode)
NS_INTERFACE_MAP_STATIC_AMBIGUOUS(nsNavHistoryContainerResultNode)
NS_INTERFACE_MAP_ENTRY(nsINavHistoryContainerResultNode)
NS_INTERFACE_MAP_END_INHERITING(nsNavHistoryResultNode)
@ -1382,7 +1402,7 @@ nsNavHistoryContainerResultNode::ReplaceChildURIAt(PRUint32 aIndex,
// Hold a reference so it doesn't go away as soon as we remove it from the
// array. This needs to be passed to the view.
nsCOMPtr<nsNavHistoryResultNode> oldItem = mChildren[aIndex];
nsRefPtr<nsNavHistoryResultNode> oldItem = mChildren[aIndex];
// actually replace
if (! mChildren.ReplaceObjectAt(aNode, aIndex))
@ -1420,7 +1440,7 @@ nsNavHistoryContainerResultNode::RemoveChildAt(PRInt32 aIndex,
NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
// hold an owning reference to keep from expiring while we work with it
nsCOMPtr<nsNavHistoryResultNode> oldNode = mChildren[aIndex];
nsRefPtr<nsNavHistoryResultNode> oldNode = mChildren[aIndex];
// stats
PRUint32 oldAccessCount = 0;
@ -2385,7 +2405,7 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, PRInt64 aVisitId,
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
nsresult rv;
nsCOMPtr<nsNavHistoryResultNode> addition;
nsRefPtr<nsNavHistoryResultNode> addition;
switch(mLiveUpdate) {
case QUERYUPDATE_TIME: {
// For these simple yet common cases we can check the time ourselves
@ -3443,11 +3463,20 @@ nsNavHistorySeparatorResultNode::nsNavHistorySeparatorResultNode()
// nsNavHistoryResult **********************************************************
NS_IMPL_CYCLE_COLLECTION_CLASS(nsNavHistoryResult)
NS_IMPL_ADDREF(nsNavHistoryResult)
NS_IMPL_RELEASE(nsNavHistoryResult)
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsNavHistoryResult)
NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mRootNode)
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
NS_INTERFACE_MAP_BEGIN(nsNavHistoryResult)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsNavHistoryResult)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR_AMBIGUOUS(mRootNode, nsINavHistoryContainerResultNode)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
NS_IMPL_CYCLE_COLLECTING_ADDREF(nsNavHistoryResult)
NS_IMPL_CYCLE_COLLECTING_RELEASE(nsNavHistoryResult)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsNavHistoryResult)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsINavHistoryResult)
NS_INTERFACE_MAP_STATIC_AMBIGUOUS(nsNavHistoryResult)
NS_INTERFACE_MAP_ENTRY(nsINavHistoryResult)

View File

@ -48,6 +48,7 @@
#include "nsTArray.h"
#include "nsInterfaceHashtable.h"
#include "nsDataHashtable.h"
#include "nsCycleCollectionParticipant.h"
class nsNavHistory;
class nsIDateTimeFormat;
@ -140,9 +141,10 @@ public:
nsresult PropertyBagFor(nsISupports* aObject,
nsIWritablePropertyBag** aBag);
NS_DECL_ISUPPORTS
NS_DECL_CYCLE_COLLECTING_ISUPPORTS
NS_DECL_NSINAVHISTORYRESULT
NS_DECL_BOOKMARK_HISTORY_OBSERVER
NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsNavHistoryResult, nsINavHistoryResult)
void AddHistoryObserver(nsNavHistoryQueryResultNode* aNode);
void AddBookmarkFolderObserver(nsNavHistoryFolderResultNode* aNode, PRInt64 aFolder);
@ -264,7 +266,9 @@ public:
NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYRESULTNODE_IID)
NS_DECL_ISUPPORTS
NS_DECL_CYCLE_COLLECTING_ISUPPORTS
NS_DECL_CYCLE_COLLECTION_CLASS(nsNavHistoryResultNode)
NS_IMPLEMENT_SIMPLE_RESULTNODE
NS_IMETHOD GetIcon(nsIURI** aIcon);
NS_IMETHOD GetParent(nsINavHistoryContainerResultNode** aParent);
@ -517,6 +521,7 @@ public:
NS_DECLARE_STATIC_IID_ACCESSOR(NS_NAVHISTORYCONTAINERRESULTNODE_IID)
NS_DECL_ISUPPORTS_INHERITED
NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsNavHistoryContainerResultNode, nsNavHistoryResultNode)
NS_FORWARD_COMMON_RESULTNODE_TO_BASE
NS_IMETHOD GetType(PRUint32* type)
{ *type = mContainerType; return NS_OK; }
@ -538,7 +543,7 @@ public:
// their result pointer set so we can quickly get to the result without having
// to walk the tree. Yet, this also saves us from storing a million pointers
// for every leaf node to the result.
nsNavHistoryResult* mResult;
nsRefPtr<nsNavHistoryResult> mResult;
// for example, RESULT_TYPE_HOST. Query and Folder results override GetType
// so this is not used, but is still kept in sync.