Bug 1290239, part 2 - Add assertions about inserting into NativeSetMap. r=mrbkap
If we are inserting an XPCNativeSet into a NativeSetMap, and we failed
to find it, then the insertion should not have found some other set
there already.
Additionally in this case, the hash value of a XPCNativeSetKey for the
set we are inserting should be the same as the key we used to insert
the set into the map. If this property does not hold, then we can't
use Remove() to remove the set from the map. This condition would have
caught the error that part 1 fixes.
MozReview-Commit-ID: 23v37GzA4XV
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -408,16 +408,30 @@ public:
if (!entry)
return nullptr;
if (entry->key_value)
return entry->key_value;
entry->key_value = set;
return set;
}
+ bool AddNew(const XPCNativeSetKey* key, XPCNativeSet* set)
+ {
+ XPCNativeSet* set2 = Add(key, set);
+ if (!set2) {
+ return false;
+ }
+#ifdef DEBUG
+ XPCNativeSetKey key2(set);
+ MOZ_ASSERT(key->Hash() == key2.Hash());
+ MOZ_ASSERT(set2 == set, "Should not have found an existing entry");
+#endif
+ return true;
+ }
+
inline void Remove(XPCNativeSet* set)
{
MOZ_ASSERT(set, "bad param");
XPCNativeSetKey key(set);
mTable.Remove(&key);
}
--- a/js/xpconnect/src/XPCWrappedNativeInfo.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeInfo.cpp
@@ -485,24 +485,20 @@ XPCNativeSet::GetNewOrUsed(const nsIID*
if (set)
return set;
set = NewInstance({iface.forget()});
if (!set)
return nullptr;
- XPCNativeSet* set2 = map->Add(&key, set);
- if (!set2) {
+ if (!map->AddNew(&key, set)) {
NS_ERROR("failed to add our set!");
DestroyInstance(set);
set = nullptr;
- } else if (set2 != set) {
- DestroyInstance(set);
- set = set2;
}
return set;
}
// static
XPCNativeSet*
XPCNativeSet::GetNewOrUsed(nsIClassInfo* classInfo)
@@ -570,16 +566,18 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
XPCNativeSet* set2 = map2->Add(&key, set);
if (!set2) {
NS_ERROR("failed to add our set!");
DestroyInstance(set);
set = nullptr;
goto out;
}
+ // It is okay to find an existing entry here because
+ // we did not look for one before we called Add().
if (set2 != set) {
DestroyInstance(set);
set = set2;
}
}
} else
set = GetNewOrUsed(&NS_GET_IID(nsISupports));
} else
@@ -630,24 +628,20 @@ XPCNativeSet::GetNewOrUsed(XPCNativeSetK
if (key->GetBaseSet())
set = NewInstanceMutate(key);
else
set = NewInstance({key->GetAddition()});
if (!set)
return nullptr;
- XPCNativeSet* set2 = map->Add(key, set);
- if (!set2) {
+ if (!map->AddNew(key, set)) {
NS_ERROR("failed to add our set!");
DestroyInstance(set);
set = nullptr;
- } else if (set2 != set) {
- DestroyInstance(set);
- set = set2;
}
return set;
}
// static
XPCNativeSet*
XPCNativeSet::GetNewOrUsed(XPCNativeSet* firstSet,