Bug 1171305 - Remove lots of now-unnecessary null-checks involving XPCMaps. r=mrbkap.

Possible because both |new| and PLDHashTable initialization are infallible now.

I had to use NS_ABORT_OOM for a couple of the maps that use js::HashTable,
which still has fallible initialization. There were a couple of uses of those
maps that weren't protected by null-checks, so we would have got crashes anyway
if they had OOMed.
This commit is contained in:
Nicholas Nethercote 2015-05-05 18:20:33 -07:00
parent ae3bff42b7
commit 16fe493889
5 changed files with 86 additions and 139 deletions

View File

@ -1560,9 +1560,8 @@ void XPCJSRuntime::DestroyJSContextStack()
void XPCJSRuntime::SystemIsBeingShutDown()
{
if (mDetachedWrappedNativeProtoMap)
mDetachedWrappedNativeProtoMap->
Enumerate(DetachedWrappedNativeProtoShutdownMarker, nullptr);
mDetachedWrappedNativeProtoMap->
Enumerate(DetachedWrappedNativeProtoShutdownMarker, nullptr);
}
#define JS_OPTIONS_DOT_STR "javascript.options."
@ -1644,51 +1643,33 @@ XPCJSRuntime::~XPCJSRuntime()
JS_SetRuntimePrivate(Runtime(), nullptr);
// clean up and destroy maps...
if (mWrappedJSMap) {
mWrappedJSMap->ShutdownMarker();
delete mWrappedJSMap;
mWrappedJSMap = nullptr;
}
mWrappedJSMap->ShutdownMarker();
delete mWrappedJSMap;
mWrappedJSMap = nullptr;
if (mWrappedJSClassMap) {
delete mWrappedJSClassMap;
mWrappedJSClassMap = nullptr;
}
delete mWrappedJSClassMap;
mWrappedJSClassMap = nullptr;
if (mIID2NativeInterfaceMap) {
delete mIID2NativeInterfaceMap;
mIID2NativeInterfaceMap = nullptr;
}
delete mIID2NativeInterfaceMap;
mIID2NativeInterfaceMap = nullptr;
if (mClassInfo2NativeSetMap) {
delete mClassInfo2NativeSetMap;
mClassInfo2NativeSetMap = nullptr;
}
delete mClassInfo2NativeSetMap;
mClassInfo2NativeSetMap = nullptr;
if (mNativeSetMap) {
delete mNativeSetMap;
mNativeSetMap = nullptr;
}
delete mNativeSetMap;
mNativeSetMap = nullptr;
if (mThisTranslatorMap) {
delete mThisTranslatorMap;
mThisTranslatorMap = nullptr;
}
delete mThisTranslatorMap;
mThisTranslatorMap = nullptr;
if (mNativeScriptableSharedMap) {
delete mNativeScriptableSharedMap;
mNativeScriptableSharedMap = nullptr;
}
delete mNativeScriptableSharedMap;
mNativeScriptableSharedMap = nullptr;
if (mDyingWrappedNativeProtoMap) {
delete mDyingWrappedNativeProtoMap;
mDyingWrappedNativeProtoMap = nullptr;
}
delete mDyingWrappedNativeProtoMap;
mDyingWrappedNativeProtoMap = nullptr;
if (mDetachedWrappedNativeProtoMap) {
delete mDetachedWrappedNativeProtoMap;
mDetachedWrappedNativeProtoMap = nullptr;
}
delete mDetachedWrappedNativeProtoMap;
mDetachedWrappedNativeProtoMap = nullptr;
#ifdef MOZ_ENABLE_PROFILER_SPS
// Tell the profiler that the runtime is gone
@ -3674,7 +3655,7 @@ XPCJSRuntime::DebugDump(int16_t depth)
XPC_LOG_INDENT();
XPC_LOG_ALWAYS(("mJSRuntime @ %x", Runtime()));
XPC_LOG_ALWAYS(("mWrappedJSToReleaseArray @ %x with %d wrappers(s)", \
XPC_LOG_ALWAYS(("mWrappedJSToReleaseArray @ %x with %d wrappers(s)",
&mWrappedJSToReleaseArray,
mWrappedJSToReleaseArray.Length()));
@ -3692,43 +3673,39 @@ XPCJSRuntime::DebugDump(int16_t depth)
XPC_LOG_OUTDENT();
}
XPC_LOG_ALWAYS(("mWrappedJSClassMap @ %x with %d wrapperclasses(s)", \
mWrappedJSClassMap, mWrappedJSClassMap ? \
mWrappedJSClassMap->Count() : 0));
XPC_LOG_ALWAYS(("mWrappedJSClassMap @ %x with %d wrapperclasses(s)",
mWrappedJSClassMap, mWrappedJSClassMap->Count()));
// iterate wrappersclasses...
if (depth && mWrappedJSClassMap && mWrappedJSClassMap->Count()) {
if (depth && mWrappedJSClassMap->Count()) {
XPC_LOG_INDENT();
mWrappedJSClassMap->Enumerate(WrappedJSClassMapDumpEnumerator, &depth);
XPC_LOG_OUTDENT();
}
XPC_LOG_ALWAYS(("mWrappedJSMap @ %x with %d wrappers(s)", \
mWrappedJSMap, mWrappedJSMap ? \
mWrappedJSMap->Count() : 0));
XPC_LOG_ALWAYS(("mWrappedJSMap @ %x with %d wrappers(s)",
mWrappedJSMap, mWrappedJSMap->Count()));
// iterate wrappers...
if (depth && mWrappedJSMap && mWrappedJSMap->Count()) {
if (depth && mWrappedJSMap->Count()) {
XPC_LOG_INDENT();
mWrappedJSMap->Dump(depth);
XPC_LOG_OUTDENT();
}
XPC_LOG_ALWAYS(("mIID2NativeInterfaceMap @ %x with %d interface(s)", \
mIID2NativeInterfaceMap, mIID2NativeInterfaceMap ? \
mIID2NativeInterfaceMap->Count() : 0));
XPC_LOG_ALWAYS(("mIID2NativeInterfaceMap @ %x with %d interface(s)",
mIID2NativeInterfaceMap,
mIID2NativeInterfaceMap->Count()));
XPC_LOG_ALWAYS(("mClassInfo2NativeSetMap @ %x with %d sets(s)", \
mClassInfo2NativeSetMap, mClassInfo2NativeSetMap ? \
mClassInfo2NativeSetMap->Count() : 0));
XPC_LOG_ALWAYS(("mClassInfo2NativeSetMap @ %x with %d sets(s)",
mClassInfo2NativeSetMap,
mClassInfo2NativeSetMap->Count()));
XPC_LOG_ALWAYS(("mThisTranslatorMap @ %x with %d translator(s)", \
mThisTranslatorMap, mThisTranslatorMap ? \
mThisTranslatorMap->Count() : 0));
XPC_LOG_ALWAYS(("mThisTranslatorMap @ %x with %d translator(s)",
mThisTranslatorMap, mThisTranslatorMap->Count()));
XPC_LOG_ALWAYS(("mNativeSetMap @ %x with %d sets(s)", \
mNativeSetMap, mNativeSetMap ? \
mNativeSetMap->Count() : 0));
XPC_LOG_ALWAYS(("mNativeSetMap @ %x with %d sets(s)",
mNativeSetMap, mNativeSetMap->Count()));
// iterate sets...
if (depth && mNativeSetMap && mNativeSetMap->Count()) {
if (depth && mNativeSetMap->Count()) {
XPC_LOG_INDENT();
mNativeSetMap->Enumerate(NativeSetDumpEnumerator, &depth);
XPC_LOG_OUTDENT();

View File

@ -173,8 +173,8 @@ Native2WrappedNativeMap::newMap(int length)
}
Native2WrappedNativeMap::Native2WrappedNativeMap(int length)
: mTable(new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length))
{
mTable = new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length);
}
Native2WrappedNativeMap::~Native2WrappedNativeMap()
@ -187,7 +187,7 @@ Native2WrappedNativeMap::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf)
{
size_t n = 0;
n += mallocSizeOf(this);
n += mTable ? PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf) : 0;
n += PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf);
return n;
}
@ -213,16 +213,12 @@ const struct PLDHashTableOps IID2WrappedJSClassMap::Entry::sOps =
IID2WrappedJSClassMap*
IID2WrappedJSClassMap::newMap(int length)
{
IID2WrappedJSClassMap* map = new IID2WrappedJSClassMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new IID2WrappedJSClassMap(length);
}
IID2WrappedJSClassMap::IID2WrappedJSClassMap(int length)
: mTable(new PLDHashTable(&Entry::sOps, sizeof(Entry), length))
{
mTable = new PLDHashTable(&Entry::sOps, sizeof(Entry), length);
}
IID2WrappedJSClassMap::~IID2WrappedJSClassMap()
@ -230,7 +226,6 @@ IID2WrappedJSClassMap::~IID2WrappedJSClassMap()
delete mTable;
}
/***************************************************************************/
// implement IID2NativeInterfaceMap...
@ -246,16 +241,12 @@ const struct PLDHashTableOps IID2NativeInterfaceMap::Entry::sOps =
IID2NativeInterfaceMap*
IID2NativeInterfaceMap::newMap(int length)
{
IID2NativeInterfaceMap* map = new IID2NativeInterfaceMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new IID2NativeInterfaceMap(length);
}
IID2NativeInterfaceMap::IID2NativeInterfaceMap(int length)
: mTable(new PLDHashTable(&Entry::sOps, sizeof(Entry), length))
{
mTable = new PLDHashTable(&Entry::sOps, sizeof(Entry), length);
}
IID2NativeInterfaceMap::~IID2NativeInterfaceMap()
@ -268,7 +259,7 @@ IID2NativeInterfaceMap::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf)
{
size_t n = 0;
n += mallocSizeOf(this);
n += mTable ? PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf) : 0;
n += PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf);
return n;
}
@ -287,16 +278,12 @@ IID2NativeInterfaceMap::SizeOfEntryExcludingThis(PLDHashEntryHdr* hdr,
ClassInfo2NativeSetMap*
ClassInfo2NativeSetMap::newMap(int length)
{
ClassInfo2NativeSetMap* map = new ClassInfo2NativeSetMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new ClassInfo2NativeSetMap(length);
}
ClassInfo2NativeSetMap::ClassInfo2NativeSetMap(int length)
: mTable(new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length))
{
mTable = new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length);
}
ClassInfo2NativeSetMap::~ClassInfo2NativeSetMap()
@ -310,7 +297,7 @@ ClassInfo2NativeSetMap::ShallowSizeOfIncludingThis(mozilla::MallocSizeOf mallocS
size_t n = 0;
n += mallocSizeOf(this);
// The second arg is nullptr because this is a "shallow" measurement of the map.
n += mTable ? PL_DHashTableSizeOfIncludingThis(mTable, nullptr, mallocSizeOf) : 0;
n += PL_DHashTableSizeOfIncludingThis(mTable, nullptr, mallocSizeOf);
return n;
}
@ -325,8 +312,8 @@ ClassInfo2WrappedNativeProtoMap::newMap(int length)
}
ClassInfo2WrappedNativeProtoMap::ClassInfo2WrappedNativeProtoMap(int length)
: mTable(new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length))
{
mTable = new PLDHashTable(PL_DHashGetStubOps(), sizeof(Entry), length);
}
ClassInfo2WrappedNativeProtoMap::~ClassInfo2WrappedNativeProtoMap()
@ -339,7 +326,7 @@ ClassInfo2WrappedNativeProtoMap::SizeOfIncludingThis(mozilla::MallocSizeOf mallo
{
size_t n = 0;
n += mallocSizeOf(this);
n += mTable ? PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf) : 0;
n += PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf);
return n;
}
@ -437,16 +424,12 @@ const struct PLDHashTableOps NativeSetMap::Entry::sOps =
NativeSetMap*
NativeSetMap::newMap(int length)
{
NativeSetMap* map = new NativeSetMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new NativeSetMap(length);
}
NativeSetMap::NativeSetMap(int length)
: mTable(new PLDHashTable(&Entry::sOps, sizeof(Entry), length))
{
mTable = new PLDHashTable(&Entry::sOps, sizeof(Entry), length);
}
NativeSetMap::~NativeSetMap()
@ -459,7 +442,7 @@ NativeSetMap::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf)
{
size_t n = 0;
n += mallocSizeOf(this);
n += mTable ? PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf) : 0;
n += PL_DHashTableSizeOfIncludingThis(mTable, SizeOfEntryExcludingThis, mallocSizeOf);
return n;
}
@ -500,16 +483,12 @@ const struct PLDHashTableOps IID2ThisTranslatorMap::Entry::sOps =
IID2ThisTranslatorMap*
IID2ThisTranslatorMap::newMap(int length)
{
IID2ThisTranslatorMap* map = new IID2ThisTranslatorMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new IID2ThisTranslatorMap(length);
}
IID2ThisTranslatorMap::IID2ThisTranslatorMap(int length)
: mTable(new PLDHashTable(&Entry::sOps, sizeof(Entry), length))
{
mTable = new PLDHashTable(&Entry::sOps, sizeof(Entry), length);
}
IID2ThisTranslatorMap::~IID2ThisTranslatorMap()
@ -575,17 +554,12 @@ const struct PLDHashTableOps XPCNativeScriptableSharedMap::Entry::sOps =
XPCNativeScriptableSharedMap*
XPCNativeScriptableSharedMap::newMap(int length)
{
XPCNativeScriptableSharedMap* map =
new XPCNativeScriptableSharedMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new XPCNativeScriptableSharedMap(length);
}
XPCNativeScriptableSharedMap::XPCNativeScriptableSharedMap(int length)
: mTable(new PLDHashTable(&Entry::sOps, sizeof(Entry), length))
{
mTable = new PLDHashTable(&Entry::sOps, sizeof(Entry), length);
}
XPCNativeScriptableSharedMap::~XPCNativeScriptableSharedMap()
@ -627,17 +601,13 @@ XPCNativeScriptableSharedMap::GetNewOrUsed(uint32_t flags,
XPCWrappedNativeProtoMap*
XPCWrappedNativeProtoMap::newMap(int length)
{
XPCWrappedNativeProtoMap* map = new XPCWrappedNativeProtoMap(length);
if (map && map->mTable)
return map;
delete map;
return nullptr;
return new XPCWrappedNativeProtoMap(length);
}
XPCWrappedNativeProtoMap::XPCWrappedNativeProtoMap(int length)
: mTable(new PLDHashTable(PL_DHashGetStubOps(), sizeof(PLDHashEntryStub),
length))
{
mTable = new PLDHashTable(PL_DHashGetStubOps(),
sizeof(PLDHashEntryStub), length);
}
XPCWrappedNativeProtoMap::~XPCWrappedNativeProtoMap()

View File

@ -32,10 +32,13 @@ class JSObject2WrappedJSMap
public:
static JSObject2WrappedJSMap* newMap(int length) {
JSObject2WrappedJSMap* map = new JSObject2WrappedJSMap();
if (map && map->mTable.init(length))
return map;
delete map;
return nullptr;
if (!map->mTable.init(length)) {
// This is a decent estimate of the size of the hash table's
// entry storage. The |2| is because on average the capacity is
// twice the requested length.
NS_ABORT_OOM(length * 2 * sizeof(Map::Entry));
}
return map;
}
inline nsXPCWrappedJS* Find(JSObject* Obj) {
@ -599,10 +602,13 @@ class JSObject2JSObjectMap
public:
static JSObject2JSObjectMap* newMap(int length) {
JSObject2JSObjectMap* map = new JSObject2JSObjectMap();
if (map && map->mTable.init(length))
return map;
delete map;
return nullptr;
if (!map->mTable.init(length)) {
// This is a decent estimate of the size of the hash table's
// entry storage. The |2| is because on average the capacity is
// twice the requested length.
NS_ABORT_OOM(length * 2 * sizeof(Map::Entry));
}
return map;
}
inline JSObject* Find(JSObject* key) {

View File

@ -428,15 +428,11 @@ XPCWrappedNativeScope::~XPCWrappedNativeScope()
// We can do additional cleanup assertions here...
if (mWrappedNativeMap) {
MOZ_ASSERT(0 == mWrappedNativeMap->Count(), "scope has non-empty map");
delete mWrappedNativeMap;
}
MOZ_ASSERT(0 == mWrappedNativeMap->Count(), "scope has non-empty map");
delete mWrappedNativeMap;
if (mWrappedNativeProtoMap) {
MOZ_ASSERT(0 == mWrappedNativeProtoMap->Count(), "scope has non-empty map");
delete mWrappedNativeProtoMap;
}
MOZ_ASSERT(0 == mWrappedNativeProtoMap->Count(), "scope has non-empty map");
delete mWrappedNativeProtoMap;
// This should not be necessary, since the Components object should die
// with the scope but just in case.
@ -833,21 +829,20 @@ XPCWrappedNativeScope::DebugDump(int16_t depth)
XPC_LOG_ALWAYS(("mComponents @ %x", mComponents.get()));
XPC_LOG_ALWAYS(("mGlobalJSObject @ %x", mGlobalJSObject.get()));
XPC_LOG_ALWAYS(("mWrappedNativeMap @ %x with %d wrappers(s)", \
mWrappedNativeMap, \
mWrappedNativeMap ? mWrappedNativeMap->Count() : 0));
XPC_LOG_ALWAYS(("mWrappedNativeMap @ %x with %d wrappers(s)",
mWrappedNativeMap, mWrappedNativeMap->Count()));
// iterate contexts...
if (depth && mWrappedNativeMap && mWrappedNativeMap->Count()) {
if (depth && mWrappedNativeMap->Count()) {
XPC_LOG_INDENT();
mWrappedNativeMap->Enumerate(WrappedNativeMapDumpEnumerator, &depth);
XPC_LOG_OUTDENT();
}
XPC_LOG_ALWAYS(("mWrappedNativeProtoMap @ %x with %d protos(s)", \
mWrappedNativeProtoMap, \
mWrappedNativeProtoMap ? mWrappedNativeProtoMap->Count() : 0));
XPC_LOG_ALWAYS(("mWrappedNativeProtoMap @ %x with %d protos(s)",
mWrappedNativeProtoMap,
mWrappedNativeProtoMap->Count()));
// iterate contexts...
if (depth && mWrappedNativeProtoMap && mWrappedNativeProtoMap->Count()) {
if (depth && mWrappedNativeProtoMap->Count()) {
XPC_LOG_INDENT();
mWrappedNativeProtoMap->Enumerate(WrappedNativeProtoMapDumpEnumerator, &depth);
XPC_LOG_OUTDENT();

View File

@ -87,7 +87,6 @@ WrapperFactory::CreateXrayWaiver(JSContext* cx, HandleObject obj)
if (!scope->mWaiverWrapperMap) {
scope->mWaiverWrapperMap =
JSObject2JSObjectMap::newMap(XPC_WRAPPER_MAP_LENGTH);
MOZ_ASSERT(scope->mWaiverWrapperMap);
}
if (!scope->mWaiverWrapperMap->Add(cx, obj, waiver))
return nullptr;